search

m-lab / ndt-e2e-clientworker

Code for the client worker of the NDT end-to-end test framework
Apache License 2.0
1 stars 4 forks source link

Adding event time recording to Banjo driver #48

Closed mtlynch closed 8 years ago

mtlynch commented 8 years ago

Adds functionality to the Banjo driver to allow it to record the relevant event times of the test.

Also fixes a small bug in results.NdtResult where the constructor was assigning default values that got re-used between calls (breaking tests).

This change is Reviewable

coveralls commented 8 years ago

Coverage Status

Changes Unknown when pulling ad4ed7e464cb46ead7585de5eb9521ea44f47379 on mtlynch:banjo2 into \ on m-lab:master**.

fernandalavalle commented 8 years ago

Few suggestions, but :lgtm:

Review status: 0 of 3 files reviewed at latest revision, 3 unresolved discussions.

_client_wrapper/banjo_driver.py, line 107 [r1] (raw file):_ Maybe try clarifying that the end of an NDT test is also the end of the c2s test. Minor point, but its just kind of confusing to have latency becoming visible, end of c2s and end of NDT all map to the same thing.

_tests/test_banjo_driver.py, line 117 [r1] (raw file):_ The test name is unclear to me. Is test in progress timeout a specific thing?

_tests/test_banjo_driver.py, line 136 [r1] (raw file):_ extra just? "If waiting for download start times out, expect just one error."

Comments from Reviewable

mtlynch commented 8 years ago

Review status: 0 of 3 files reviewed at latest revision, 3 unresolved discussions.

_tests/test_banjo_driver.py, line 117 [r1] (raw file):_ Yeah, this was a bad name. Fixed (I think).

_tests/test_banjo_driver.py, line 136 [r1] (raw file):_ Done.

Comments from Reviewable

coveralls commented 8 years ago

Coverage Status

Changes Unknown when pulling a7485f8d535582adfb39909c2aa7b10a7f7c775e on mtlynch:banjo2 into \ on m-lab:master**.

mtlynch commented 8 years ago

Review status: 0 of 3 files reviewed at latest revision, 3 unresolved discussions.

_client_wrapper/banjo_driver.py, line 107 [r1] (raw file):_ Oh, specifying C2S test makes sense. I think I've addressed this. I didn't say that it's the end of the NDT test because result.end_time is when the test process finishes, so it includes things like shutting down the browser and parsing the results page. Let me know if more detail would be helpful.

Comments from Reviewable