Add unit tests to twine.commands.upload (take 2)

[bhrutledge]

Building on #622 from @deveshks. Happy to receive feedback and answer questions.

Changes:

• Rebased on master

• 5e33607: Add more information to comments and make test_successful_upload_sign_package follow the same pattern as test_successful_upload_add_gpg_signature

• 2530852: Reduce patching complexity by making assertions on the package passed to repository.upload()

TODO:

• Extract a fixture for stub_repository to make test_successful_upload* easier to read

[deveshks]

Hi @bhrutledge

Thank you for creating this PR to better apply the suggestions you provided me in the last PR. Having said that, I had a few questions.

I thought that it was almost at the finish line, and the back-and-forth there is part of a standard in the PR review process. I understand that there was some struggle from my end to clearly put across and understand what was needed in this PR, but I thought that is part of the learning process of being a contributor.

It would be helpful to know how can I avoid such a situation in my future contributions to twine, and make the experience enjoyable for all of us, where the communication parameters/frequency are moderated such that this doesn't happen in future, and my PRs can see the finish line.

[deveshks]

So we are going to extract this part into a fixture which will live within test_upload and will make this more readable, or is this something which can be moved to conftest.py is this pattern is repeated in other tests as well?

[bhrutledge]

@deveshks Thanks for your thoughts, and for your contributions. I mean no offense. I'm also learning to be a maintainer, and trying to find the right balance between time spent nurturing contributions vs. time spent on improving twine. So, this second PR was a bit of an experiment, which I thought would still highlight your contribution via the commit author, without force-pushing to your branch. That said, if I were in your shoes, I'd rather have my original PR merged.

That said, if you restore your deleted branch (which I think the GitHub UI enables), and don't mind me force-pushing to it, I can reopen #622, and close this.

[bhrutledge]

Also, @deveshks, one more bit of feedback: you posted similar comments here and in #622. In the interest of streamlining conversation, I would try to keep it all in one place (preferably an open issue/PR).

[deveshks]

@deveshks Thanks for your thoughts, and for your contributions. I mean no offense. I'm also learning to be a maintainer, and trying to find the right balance between time spent nurturing contributions vs. time spent on improving twine. So, this second PR was a bit of an experiment, which I thought would still highlight your contribution via the commit author, without force-pushing to your branch. That said, if I were in your shoes, I'd rather have my original PR merged.

Thanks for your response, and I apologise if my response on this and the other PR offended you in any way as well.

That said, if you restore your deleted branch (which I think the GitHub UI enables), and don't mind me force-pushing to it, I can reopen #622, and close this.

I really appreciate your thoughtfulness for allowing me to reopen the old PR. I have gone ahead and restored the branch in #622 , you can force-push to it.

[deveshks]

Also, @deveshks, one more bit of feedback: you posted similar comments here and in #622. In the interest of streamlining conversation, I would try to keep it all in one place (preferably an open issue/PR).

I will keep this in mind for future, thanks for the pointer.

[deveshks]

Please do also let me know if you face difficulties force-pushing to the branch.

I remember checking the Allow edits by Maintainers checkbox for #622 as I do with all other PRs, but for some reason, now it shows to me that the box is unchecked.

Edit: If you don't mine opening the PR, I can also pull the commits here and force-push them there. I tried checking the checkbox again, but there seems to be a UI bug where if I refresh the page, it goes away. I even tried it via https://developer.github.com/v3/pulls/#update-a-pull-request , but that seems to not work as well.

[bhrutledge]

Closing in favor of #622.