Merge lp://staging/~gary/launchpad/bug724609 into lp://staging/launchpad

Proposed by Gary Poster
Status: Merged
Approved by: Gary Poster
Approved revision: no longer in the source branch.
Merged at revision: 14351
Proposed branch: lp://staging/~gary/launchpad/bug724609
Merge into: lp://staging/launchpad
Diff against target: 709 lines (+539/-19)
8 files modified
lib/lp/app/javascript/client.js (+18/-9)
lib/lp/app/javascript/server_fixture.js (+67/-1)
lib/lp/app/javascript/tests/test_lp_client.js (+48/-0)
lib/lp/app/javascript/tests/test_lp_client_integration.js (+333/-0)
lib/lp/app/javascript/tests/test_lp_client_integration.py (+62/-0)
lib/lp/app/templates/base-layout-macros.pt (+6/-2)
lib/lp/registry/browser/tests/test_subscription_links.py (+2/-6)
lib/lp/testing/__init__.py (+3/-1)
To merge this branch: bzr merge lp://staging/~gary/launchpad/bug724609
Reviewer Review Type Date Requested Status
Brad Crittenden (community) Approve
Review via email: mp+82689@code.staging.launchpad.net

Commit message

[r=bac][bug=724609] re-add integration tests for the lp.client library, replacing the old Windmill tests with yuixhr tests.

Description of the change

This branch addresses critical bug 724609 by re-adding integration tests for the lp.client library. These had been written for Windmill, and were removed earlier this year because we discarded Windmill.

If you would like to look at the original, you can find them here. http://bazaar.launchpad.net/~launchpad-pqm/launchpad/devel/view/13343/lib/canonical/launchpad/windmill/jstests/launchpad_ajax.js

The match is usually roughly one-to-one, though I did take some editorial liberties, and a few of the tests were moved to pure JS unit tests when I felt I could do that.

The original tests had tests for the hosted file support. Much of them were commented out because we weren't actually using enough of it in Launchpad to test the integration. I tried to port the existing tests, but I discovered that Chrome makes our hosted files even more problematic. We redirect our hosted files to another domain for the librarian, and Chrome disallows getting files from another domain for security reasons. We could fix this by having librarian files in the same domain, but when I brought this up to him, Francis told me to just remove the support in lp.client for hosted files, because we have not used it for years. Therefore, in a second and separate branch, I will do this.

I made a few changes to help with testing.

- The lp.client itself gained a new argument so that you can instantiate it so that requests are performed synchronously. This makes it easier to write tests, because you don't have to try and figure out how long to wait for the results using the YUI async test API.

- The yuixhr module gained two abilities.

  * First, you can add arbitrary cleanup functions to be called on teardown.

  * The cleanup function ability is then used by the second ability: you can test a page in an iframe. This is good for integration tests that just need to verify that library code is actually hooked up on a page. You should of course not test the library itself with the iframe.

Lint is happy.

Thank you!

To post a comment you must log in.
Revision history for this message
Brad Crittenden (bac) wrote :

Hi Gary,

Thanks for this branch and helping to kill of windmill.

* As discussed on IRC, I think waitForIFrame is a bit misleading for a name but I've not got a reasonable suggestion for fixing it.

* When calling waitForIFrame it would be helpful to comment the arguments being passed. I needed to go back and forth to understand what was being passed. Trivial point.

* fix: """{Describe your test suite here}."""

Very nice branch.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.