Merge lp:~benji/launchpad/wadl-refactoring into lp:launchpad
Proposed by
Benji York
Status: | Merged |
---|---|
Approved by: | Brad Crittenden |
Approved revision: | no longer in the source branch. |
Merged at revision: | 11755 |
Proposed branch: | lp:~benji/launchpad/wadl-refactoring |
Merge into: | lp:launchpad |
Diff against target: |
365 lines (+223/-53) 6 files modified
Makefile (+1/-1) lib/canonical/launchpad/ftests/test_wadl_generation.py (+35/-0) lib/canonical/launchpad/rest/wadl.py (+62/-0) lib/lp/testing/matchers.py (+35/-0) lib/lp/testing/tests/test_matchers.py (+35/-0) utilities/create-lp-wadl-and-apidoc.py (+55/-52) |
To merge this branch: | bzr merge lp:~benji/launchpad/wadl-refactoring |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Gavin Panella (community) | Approve | ||
Brad Crittenden (community) | code | Approve | |
Review via email: mp+36452@code.launchpad.net |
Description of the change
This branch extracts the refactorings and tests from an earlier branch
that we've decided to scrap. All of the code in this branch has been
previously reviewed and approved.
The theme of the refactorings is to provide APIs to generate the WADL
and the web service documentation. The branch also includes smoke tests
to be sure that something resembling HTML and WADL are generated and no
exceptions are raised.
Some whitespace lint was fixed in this branch.
To run the added tests: bin/test -c test_wadl_
To post a comment you must log in.
Nice clean up :)
I have a few fairly minor comments. [4] needs fixing before this
branch is safe to land.
[1]
+ self.assert_ (wadl.startswit h('<?xml '))
I think this might be better as:
from lp.testing.matchers import Startswith assertThat( wadl, StartsWith('<?xml '))
self.
You'll need to inherit from lp.testing.TestCase (or, more accurately,
testtools.TestCase) to get assertThat().
[2]
+ self.assert_('<html ' in html)
Same as [1].
[3]
+ return subprocess.Popen( subprocess. PIPE, subprocess. PIPE).communica te()[0]
+ ['xsltproc', stylesheet, wadl_filename],
+ stdout=
+ stderr=
Unless it's useless and/or noisy, I think stderr should be left alone,
so:
return subprocess.Popen(
['xsltproc' , stylesheet, wadl_filename],
stdout= subprocess. PIPE).communica te()[0]
I also think the exit code should be checked, so:
args = ['xsltproc', stylesheet, wadl_filename] Popen(args, stdout= subprocess. PIPE) communicate( ) CalledProcessEr ror(
process. returncode, args)
process = subprocess.
out, err = process.
if process.returncode == 0:
return out
else
raise subprocess.
This is similar to how subprocess. check_call( ) behaves.
[4]
-import _pythonpath
-
It's ugly, looks like lint, but you'll need this.
It may be worth adding a test to show that the script can run
successfully, even if it's only to show the help.