Add unit tests to twine.repository

[deveshks]

Towards #7

Add more unit tests to twine.repository to bring the coverage up to 99%

py run-test: commands[1] | coverage report -m
Name                         Stmts   Miss Branch BrPart  Cover   Missing
------------------------------------------------------------------------
twine/__init__.py               14      1      2      1    88%   29->32, 32
twine/_installed.py             40      3     18      7    83%   19->56, 21->22, 22, 26->56, 45->56, 46->51, 48->52, 51, 52->45, 56
twine/auth.py                   55      0      4      1    98%   55->62
twine/cli.py                    25      0      4      0   100%
twine/commands/__init__.py      21      0     11      0   100%
twine/commands/check.py         72      0     24      1    99%   90->97
twine/commands/register.py      23      0      4      0   100%
twine/commands/upload.py        55      2     22      2    95%   77->78, 78, 79->80, 80
twine/exceptions.py             27      0      0      0   100%
twine/package.py               128      8     35      8    90%   83->88, 88, 101->102, 102-103, 106->107, 107, 163->164, 164, 167, 236->exit, 240->242, 242, 245->exit, 249->251, 251
twine/repository.py            120      1     38      0    99%   251
twine/settings.py               68      0      4      0   100%
twine/utils.py                  98      0     40      1    99%   172->177
twine/wheel.py                  46      0     16      0   100%
twine/wininst.py                37     26     19      0    20%   14-16, 20-24, 27-58
------------------------------------------------------------------------
TOTAL                          829     41    241     21    92%

[bhrutledge]

Thanks for adding the coverage! As a bit of hopefully constructive feedback, I probably would have started with a PR that only added coverage, and asked questions in the PR description about the refactoring and docstrings, before making those changes. That would have made the additions easier for the maintainers to review and merge, and possibly saved you time spent on making changes, and all of us time spent on comment threads.

[deveshks]

Thanks for adding the coverage! As a bit of hopefully constructive feedback, I probably would have started with a PR that only added coverage, and asked questions in the PR description about the refactoring and docstrings, before making those changes. That would have made the additions easier for the maintainers to review and merge, and possibly saved you time spent on making changes, and all of us time spent on comment threads.

I think what you mean is that first start with adding code for coverage, and then ask if the already added code can be refactored in terms of changing the logic, or adding docstring which will cause existing code to be changed, and make the changes only if the permission is granted.

Also in terms of refactoring, can I go ahead and add commits which explicitly states the changes made, but maybe for others, ask questions about the possible approach on how to refactor the code and then do it?

[deveshks]

I have gone ahead and made the suggested changes. I will start with only making incremental additions in newer PRs, and any potential refactoring of existing code on a case-to-case basis

[bhrutledge]

Thanks for sticking with this, @deveshks. You seem to have a good understanding of what to test and how to test it, and that's appreciated. My comments are intended to build on that, add a bit more polish to your work, and hopefully streamline future contributions.

Re: my earlier comments on refactoring, I think you understood what I was getting at. In general, and within reason, I think it's wise to keep the scope of PR narrow, e.g. keeping refactoring distinct from adding/changing behavior. Of course, I ignored my own advice as recently as this morning: #608 (review).

Finally, my thinking on testing has evolved a lot over the last few years, most recently via Why Good Developers Write Bad Tests at PyGotham 2019.

[bhrutledge]

Thanks again! I'm going to hold on merging this until after #608, because that's got a spurious coverage failure, and this should give master a clean result.