Testing fixes for test_results_push.py #3374
Assignees
Comments
ndokos
added a commit
that referenced
this issue
Apr 10, 2023
* Do not use the file logger at all in python commands. The logger only uses the default Streamhandler now which outputs to stderr. * - copy_result_tb: return the response to the PUT request copy_result_tb returns the response from the server. The callers are responsible for interpreting it. However, it can still raise an exception (e.g. on connection error). - push.py: construct a reasonable message out of the response and check if the HTTP status is 201: we exit with 0 if so. Otherwise, if the status is OK (i.e. < 400), then we exit with 0 but print a message with a reasonably self-explanatory status on stderr. We expect that we will only ever get two OK responses: a 201 which indicates a successful PUT with a newly created dataset and 200 which indicates a duplicate dataset. In all non-OK cases, we output the message on stderr and exit with 1. - move.py: check the response - if not OK (>= 400) raise exception. * Fix the monkeypatching in test_copy_result_tb.py Monkeypatching pathlib components (.exists() and .open()) causes pytest to blow up. Limit the scope by using `monkeypatch.context()' so that it is only applied to the objects under test (and not e.g. to what is used in checking assertions). * Use symbolic constant instead of explicit 201 * Parametrize the "normal" test This is just the first step - see issue #3374 for some more work along these lines.
ndokos
added a commit
to ndokos/pbench
that referenced
this issue
Apr 11, 2023
…b0.72 * Do not use the file logger at all in python commands. The logger only uses the default Streamhandler now which outputs to stderr. * - copy_result_tb: return the response to the PUT request copy_result_tb returns the response from the server. The callers are responsible for interpreting it. However, it can still raise an exception (e.g. on connection error). - push.py: construct a reasonable message out of the response and check if the HTTP status is 201: we exit with 0 if so. Otherwise, if the status is OK (i.e. < 400), then we exit with 0 but print a message with a reasonably self-explanatory status on stderr. We expect that we will only ever get two OK responses: a 201 which indicates a successful PUT with a newly created dataset and 200 which indicates a duplicate dataset. In all non-OK cases, we output the message on stderr and exit with 1. - move.py: check the response - if not OK (>= 400) raise exception. * Fix the monkeypatching in test_copy_result_tb.py Monkeypatching pathlib components (.exists() and .open()) causes pytest to blow up. Limit the scope by using `monkeypatch.context()' so that it is only applied to the objects under test (and not e.g. to what is used in checking assertions). * Use symbolic constant instead of explicit 201 * Parametrize the "normal" test This is just the first step - see issue distributed-system-analysis#3374 for some more work along these lines.
|
ndokos
added a commit
to ndokos/pbench
that referenced
this issue
Apr 12, 2023
Fixes distributed-system-analysis#3374 This is a continuation of distributed-system-analysis#3348, implementing fixes to the pbench_results_push tests. The main one is to subsume some exception tests under the parametrized "normal" case and eliminate redundant tests.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
There are some existing issues and some arising from PR #3348 that should be addressed in the
pbench-results-pushtests (see test_resuls_push.py):Some tests (e.g. the tests that test exceptions) can be subsumed under the parametrization of
add_http_mock_responsewith suitable changes so that the latter can accept an exception as well as a string for themessage). That will eliminateadd_connectionerr_mock_responseand the individual tests that use it.It might be better to specify the numeric value (400) rather than a symbolic name that happens to have the same value when deciding how to format the error message.
See also this suggestion for modifying the parametrization of the tests.
The text was updated successfully, but these errors were encountered: