Code review comment for ~racb/git-ubuntu:prepare-upload-adjustments

Revision history for this message
Robie Basak (racb) wrote :

> At first I wondered if it would be a good idea to verify the contents of the "headers" dict in "cli_printargs", but then I realized it is already being done in "push"

Right - it's more tedious to test from something closer to the CLI interface, so I tend to test the inner bits more directly.

> (even though that is being done through calls to "assert" and therefore we do rely on __debug__ being set to True).

I'm not sure we're on the same page here. What I mean above is that I'm testing the contents of the "headers" dict in prepare_upload_test.py using tests that test the behaviour of the prepare_upload.py::push(). assert statements from there are the usual pattern for pytest-based test suites and test suites are expected to always run with asserts enabled.

There are separate assert statements in the code itself in prepare_upload.py::push(), but these are there to state (and runtime verify when asserts are enabled) invariants that would help fail earlier and more helpfully if there is a bug somewhere. But I intend to test every actual case from the test suite in prepare_upload_test.py. If there's something you spotted that you think isn't being tested from there, I'd like to add it!

« Back to merge proposal