Merge ~racb/git-ubuntu:prepare-upload-adjustments into git-ubuntu:master

PASSED: Continuous integration, rev:7f25e4872b2d134925d3c6768be7ece34e24f112
https://jenkins.ubuntu.com/server/job/git-ubuntu-ci/63/
Executed test runs:
    SUCCESS: VM Setup
    SUCCESS: Build
    SUCCESS: VM Reset
    SUCCESS: Unit Tests
    IN_PROGRESS: Declarative: Post Actions

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/git-ubuntu-ci/63//rebuild

LGTM!

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" (even though that is being done through calls to "assert" and therefore we do rely on __debug__ being set to True).

> 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!

> 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.

I was referring to the assert calls in `gitubuntu/prepare_upload.py`, as you mentioned later in your reply. I was just wondering why they are not explicitly raising exceptions instead. The comments in that file do justify them though:

> # However, we don't know of any actual case when this might happen, so
  # these are assertions rather than fully UX-compliant error paths.

> If there's something you spotted that you think isn't being tested from there, I'd like to add it!

The only thing that came to mind was the regular expression to parse the git URL. Although it is based on the one present in `https://git.launchpad.net/launchpad/tree/lib/lp/app/validators/name.py`, it is slightly different since it allows the username to start with one of the allowed special characters. Parametrizing that test to include other valid username examples could prevent mistakes in future changes to that regex. However, this looks simple enough and perhaps the effort is just not worth here.

This LGTM :)