Bazaar

test_http load_tests is overly complex and hard to understand

Bug #597791 reported by John A Meinel
8
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Bazaar
Fix Released
Medium
Martin Pool
Bazaar 2.3b2

Bug Description

Currently bzrlib/tests/test_http.py is a fairly large file, which is testing a lot of HTTP conformance. It does some multiplexing against multiple transport implementations, multiple HTTP protocol versions, and Authentication schemes.

As such the 'load_tests' function does a fair amount of gyrations to make sure that any given tests gets multiplied against the right permutations.

It would probably be easier to follow if the code was split out into separate "per_http_transport, per_http_protocol, per_http_authentication" sections

Related branches

lp:~mbp/bzr/597791-http-tests
Robert Collins (community): Approve
Vincent Ladeuil: Approve
Diff: 689 lines (+326/-100)
6 files modified
NEWS (+6/-0)
bzrlib/tests/__init__.py (+14/-1)
bzrlib/tests/scenarios.py (+61/-0)
bzrlib/tests/test_http.py (+115/-89)
bzrlib/tests/test_scenarios.py (+110/-0)
doc/developers/testing.txt (+20/-10)
Revision history for this message
Vincent Ladeuil (vila) wrote :

Also note that TestActivity and TestNoReportActivity differs only for a parametrization issue (the later
is parametrized in the class itself and doesn't need help from load_tests()).

Revision history for this message
Martin Pool (mbp) wrote :

I agree, it is pretty complex! I wanted to add a test for https://bugs.edge.launchpad.net/bzr/+bug/656667 and while the parameterization is quite thorough it's not obvious where to put it.

My inclination would be to split by per_* but this case does perhaps show that naming things that way might get unwieldy if there are multiple variables.

Another approach would be to say, in each class

 variations = [ByHttpClientImplementation, ByHttpProtocolVersion]

and expand from that. It would get reduce the action-at-a-distance effect, and perhaps make later manipulations easier.

Martin Pool (mbp)
Changed in bzr:
assignee: nobody → Martin Pool (mbp)
importance: Wishlist → Medium
status: Confirmed → In Progress
Revision history for this message
Martin Pool (mbp) wrote :

https://code.launchpad.net/~mbp/bzr/597791-http-tests/+merge/37941 cleans it up. I'm quite satisfied with the results.

Vincent Ladeuil (vila)
Changed in bzr:
milestone: none → 2.3b2
status: In Progress → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.
Subscribing...

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.