Replace unittest with pytest in cli tests and document endstate

[cidrblock]

This is follow on work from #872 which guarded against the use of unittest.

• Made minimal changes necessary to switch to pytest-mock from unittest-mock
• Updated docstrings as encountered
• Relevant docstrings exceptions removed from flake8 config for candidate linters pydocstyle and darglint
• Added pytest-mock as a dependency

[webknjaz]

I don't seem to see any of the updated places needing mocker at all. Everything can be implemented with the built-in monkeypatch.setattr() and monkeypatch.setenv().

[cidrblock]

I don't seem to see any of the updated places needing mocker at all. Everything can be implemented with the built-in monkeypatch.setattr() and monkeypatch.setenv().

I was hoping you might bring attention to this.

In #872 , we suggest the use of "unittest.mock:pytest-mock" and I suggested others help clean these up. Today, I thought, If I'm asking someone else to do this, I should t least fix a few myself.

Based on the #872 I went directly for pytest-mock as a replacement, which I believe others may have done as well.

I think this finding might merit documentation somewhere, that we prefer the use of monkeypatch over pytest-mock where it can be used, which I should have suggested in #872 but didn't think of it at the time.

I will add an issue related to that so it can be added to the upcoming developer guide when it arrives.

I will also make the changes, because I agree, no need to pull in pytest-mock until it is needed.

[webknjaz]

I will also make the changes, because I agree, no need to pull in pytest-mock until it is needed.

I think it may be useful when replacing the decorators or needing to automatically and implicitly use magic mocks or spy on things. But none of the diff changes I see included right now use that.

[webknjaz]

I think this finding might merit documentation somewhere, that we prefer the use of monkeypatch over pytest-mock where it can be used, which I should have suggested in #872 but didn't think of it at the time.

Totally agreed. None of https://docs.pytest.org/en/latest/how-to/monkeypatch.html / https://pypi.org/p/pytest-mock have this comparison in their documentation.

Having some sort of dos and don'ts could be useful.

[ssbarnea]

Please try to follow the famous 72/50 rule, that PR title spreading on 3 lines in browser is not easy to read, on git log would even be worse.

From personal experience be sure that the editor you use to write commit message is able to display 72/50 markers, as this helps. I use Fork.app on macos, but also code is able to display guidelines for these.

[webknjaz]

that PR title spreading on 3 lines in browser is not easy to read

+1 I've also made this screenshot to make the problem more visually accessible:

[cidrblock]

Good stuff here about that for me, since I have chosen to use vscode.... https://robertcooper.me/post/git-commit-messages#using-vscode-to-write-commit-messages

[webknjaz]

Good stuff here about that for me, since I have chosen to use vscode.... robertcooper.me/post/git-commit-messages#using-vscode-to-write-commit-messages

Yep, I know some people use set VS Code as their default Git editor. You may also like the possibility to configure a commit template — https://codeinthehole.com/tips/a-useful-template-for-commit-messages/.

P.S. Here's some inspiration on how to craft great commit messages: https://twitter.com/nedbat/status/1283147492616556544.

[webknjaz]

@cidrblock #896 this may be helpful to keep yourself in check. Note that it only checks the last commit. Try it out, though.

[cidrblock]

@webknjaz @ssbarnea anything else here? I've got other changes to make to the same files and would love to avoid the conflict resolution.

[ssbarnea]

Approved but I will take the opportunity to mention that I have mixed feelings about thin kind of move. I remember people on openstack being actively opposed to pytest being a walled garden for testing and use of its own implementation limiting the options.

Is not really a problem for us as we use only pytest everywhere, but I would avoid bothering about switching stuff unless they are broken. If it still works with older approach, why bother? (no need to answer).

[cidrblock]

Approved but I will take the opportunity to mention that I have mixed feelings about thin kind of move. I remember people on openstack being actively opposed to pytest being a walled garden for testing and use of its own implementation limiting the options.

Is not really a problem for us as we use only pytest everywhere, but I would avoid bothering about switching stuff unless they are broken. If it still works with older approach, why bother? (no need to answer).

No need, but I am compelled.

I want to explain how this played out, so future contributors don't get put in a similar situation.

• In Disable anchors and aliases when serializing to yaml #864 an owner of the repository suggested that unittest be avoided, and pytest be used.
• In Guard against using unittest ecosystem within pylint #872 pylint's preferred module was used to warn contributors about the use of unittest.
  • This also created some technical debt in the repository by adding pylint disable statements to current tests that use unittest
  • In that same PR I suggested those get converted

Because many developers look at the current code base to reuse pieces of code when adding to it, I wanted to make sure these got converted to pytest tests so they can serve as valid examples in the future.

I also wanted the experience of doing the conversion and the learning about pytest that came with it.

When mixed messages from 2 repository owners fall on the shoulders of a contributor to sort out and weigh, they might find it frustrating and be led to believe the team does not have one voice.

Please help me make sure this doesn't happen again, let's all try to stay in sync better, including me with both of you.