Add tests that specify checksumtype in drpm upload

[kdelee]

Includes tests in both API and CLI

Closes #585

@Ichimonji10
Let me know what you think.

While re-creating this issue, Pulp issue #2871 was discovered. The consequence of 2871 is that if a checksumtype is specified that does not match a specified checksum, no error is thrown, which is a problem.

Another consequence seems to be that, no matter what "checksumtype" is specified, "sha256" is chosen.

I have not filed a pulp-smash issue yet because it seems there is still some debate about what the problem is -- i.e. should all references be "checksumtype" or is it valid for some to be "checksum_type".

[coveralls]

Coverage remained the same at 62.204% when pulling 84aa309 on kdelee:test_upload_with_checksumtype into ef3cbb5 on PulpQE:master.

[Ichimonji10]

The API test targets the right Pulp functionality, but there's a few tweaks that can be made to the implementation of the test.

[Ichimonji10]

The default response handler is api.json_handler. If the default is used, then the calls to .json() can be omitted from the code below.

[Ichimonji10]

When this assertion fails, the user will be faced with an error message like "assertion failed: 2 not equal to 1." The user won't know what units.json() is, though. This assertion gives the user a little extra information about what went wrong then the test fails:

self.assertEqual(len(units.json()), 1, units.json())

[Ichimonji10]

What if len(units.json()) == 0? It's a pretty reasonable outcome. In that case, the assertion on the line below will fail... and the test will continue on to the next assertion, which will blow up on the units.json()[0] line. In this case, it's probably easier to just avoid subTest context managers:

self.assertEqual(len(units.json()), 1, units.json())
self.assertEqual(units.json()[0]['metadata']['filename'], DRPM)

[Ichimonji10]

This is funky. How about this?

'pulp-admin rpm repo uploads drpm --repo-id {} --file {} '
'--checksum-type sha256'
.format(repo_id, drpm_file).split()

[kdelee]

@Ichimonji10
Great feedback, thanks. I'll work on those changes.

[coveralls]

Coverage remained the same at 62.204% when pulling e236ea2 on kdelee:test_upload_with_checksumtype into ef3cbb5 on PulpQE:master.

[Ichimonji10]

Nitpick: These parentheses are redundant. Python will join these two fragments into a string before applying .format().

EDIT: This is something the other test method does too, right? My bad. Worry about it later if you like.

[kdelee]

@Ichimonji10
Made those changes you suggested.

I have not done it, but I could refactor the API test that I based mine off of to be more like this one.

pulp_smash.tests.rpm.api_v2.UploadDrpmTestCase tests the return code (default code_handler does this, right?), and does the same thing with the units = utils.search_units(self.cfg, repo, {}, api.safe_handler) that I had done.

Does that sound like something that should be done in this PR or should I stick to the tests I was writing to close this issue?

[Ichimonji10]

pulp_smash.tests.rpm.api_v2.test_upload_publish.UploadDrpmTestCase makes use of an anachronistic coding style. I'd love to see it refactored to look more like the new test case that you've written. Assertions on exact HTTP status codes aren't usually necessary. They just don't prove much. It's easier to let methods like code_handler or safe_handler assert than the status code is less than 400, and then make assertions on other behaviour, like whether a correct checksum was used.

Does that sound like something that should be done in this PR or should I stick to the tests I was writing to close this issue?

If I had my druthers, I'd refactor that existing test case in a separate commit. A refactor would be great regardless of how it lands.

[kdelee]

@Ichimonji10

Ready to merge now. Thanks for the feedback.
I will do the refactor in a different PR.

[coveralls]

Coverage remained the same at 62.204% when pulling c1281cb on kdelee:test_upload_with_checksumtype into ef3cbb5 on PulpQE:master.