Launchpad itself

lib/lp/app/javascript/server_fixture.js (+17/-0)
lib/lp/testing/__init__.py (+29/-4)
lib/lp/testing/yuixhr.py (+7/-0)
setup.py (+1/-0)
versions.cfg (+1/-0)

To merge this branch:

bzr merge lp://staging/~gary/launchpad/bug875697

High

Fix Released

Link a bug report

Reviewer	Review Type	Date Requested	Status
Ian Booth (community)	code	2011-11-03	Approve on 2011-11-04
Review via email: mp+81219@code.staging.launchpad.net

Commit message

[r=wallyworld][bug=875697] Give yuixhr tests per-test timeouts.

Description of the change

This branch incorporates the changes I made to html5browser, allowing for per-test times, into the yuixhr tests. This means that each test will have 6 seconds to run. The test suite itself has a limit that should never be encountered of 5 minutes. The first test has a very generous 20 seconds to pass or fail, because it incorporates the time needed to fire up the test browser.

I use buildout for html5browser to make it easier to update safely and gradually across our environment. It still makes sense to include it as a deb dependency so we get its OS-level dependencies.

Lint is happy except for some pre-existing and pre-considered lines in yuixhr.py.

This depends on another branch because I can't land that one without this one--the tests time out on buildout.

Revision history for this message

Ian Booth (wallyworld) wrote on 2011-11-04:

Hi Gary

This looks like a nice change. I'll take it as a given that when the code calls page = client.load_page(), the page.content can be parsed onto a dict containing keys testCase, testName, type. If any one of those keys are missing, then:

76 + try:
77 + msg += (' The last test that ran to '
78 + 'completion before timing out was '
79 + '%(testCase)s:%(testName)s. The test %(type)sed.'
80 + % self._last_test_info)
81 + except KeyError:
82 + pass

will not append anything to msg. Do we need to allow for that and still append a useful message fragment? Or is the test report dict always guaranteed to have all the required keys?

Should there be any tests for this stuff? I would like to see some but I'll leave it to you.

review: Approve (code)

Revision history for this message

Gary Poster (gary) wrote on 2011-11-04:

On Nov 3, 2011, at 8:48 PM, Ian Booth wrote:

> Review: Approve code
>
> Hi Gary

Hi Ian. Thank you very much for your review.

> This looks like a nice change. I'll take it as a given that when the code calls page = client.load_page(), the page.content can be parsed onto a dict containing keys testCase, testName, type.

Right, if there is an incremental timeout. You can see the data that is used for this generated in the JS changes in this branch (handle_pass and handle_fail).

> If any one of those keys are missing, then:
>
> 76 + try:
> 77 + msg += (' The last test that ran to '
> 78 + 'completion before timing out was '
> 79 + '%(testCase)s:%(testName)s. The test %(type)sed.'
> 80 + % self._last_test_info)
> 81 + except KeyError:
> 82 + pass
>
> will not append anything to msg. Do we need to allow for that and still append a useful message fragment? Or is the test report dict always guaranteed to have all the required keys?

A good question. I added a snippet to alert a reader of the problem, and give a repr of the received data.

> Should there be any tests for this stuff? I would like to see some but I'll leave it to you.

I didn't see a reasonable way of making an integration test of our integration test runner stuff. Perhaps I should have shaved some more yaks to make it possible to do so, but I already have other yak shaving in progress, and I was (IMO) following an existing precedent for this level of code.

Thanks again.

Gary