Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tests: deprecate test_tools; replace with tests.pytest_plugin #2003

Merged
merged 3 commits into from
Jan 11, 2021

Conversation

Exirel
Copy link
Contributor

@Exirel Exirel commented Dec 14, 2020

Description

Since the last refactoring, the only purpose left for sopel.test_tools was to provide utility functions for sopel.plugin.example. Since this feature always requires pytest, I figured that it would be better to nuke sopel.test_tools in Sopel 8 and to move everything still relevant to that plugin.

A second round of refactoring in Sopel 8 might be done at some point to rework how pytest is used to discover example test, but that's for another time and certainly not something to do in this PR.

Checklist

  • I have read CONTRIBUTING.md
  • I can and do license this contribution under the EFLv2
  • No issues are reported by make qa (runs make quality and make test)
  • I have tested the functionality of the things this change touches

Copy link
Member

@dgw dgw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not exactly going in order, but this is the PR that caught my attention and then I got distracted by plumbing some of the weird shit that's left in the example-test code. See line notes. 😁

sopel/test_tools.py Outdated Show resolved Hide resolved
sopel/tests/pytest_plugin.py Outdated Show resolved Hide resolved
sopel/tests/pytest_plugin.py Outdated Show resolved Hide resolved
sopel/tests/pytest_plugin.py Outdated Show resolved Hide resolved
@Exirel Exirel force-pushed the tests-deprecate-test-tools branch 2 times, most recently from 54ca39f to d87f998 Compare December 14, 2020 18:11
@Exirel
Copy link
Contributor Author

Exirel commented Dec 14, 2020

I did some force-push because really its just a few lines. Damn, I hate py.

Copy link
Member

@dgw dgw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hooray for fixing what I mentioned before! 🥳 Though I'm not sure what happened with the module docstring for sopel.test_tools. I swear it wasn't like this before.

sopel/test_tools.py Outdated Show resolved Hide resolved
@Exirel Exirel requested a review from dgw December 19, 2020 10:39
@dgw
Copy link
Member

dgw commented Dec 25, 2020

Go ahead and do any history cleanup you were planning. I'd guess 3f10b01 will get fixup'd into fe5e29c?

@Exirel
Copy link
Contributor Author

Exirel commented Dec 26, 2020

Already did.

@dgw
Copy link
Member

dgw commented Dec 26, 2020

Did you? Without 3f10b01, that first commit introduces mixed indentation (3 & 4 spaces) into the module docstring for test_tools.

@Exirel
Copy link
Contributor Author

Exirel commented Dec 26, 2020

Pretty sure it's fixed... did I do something wrong? I don't see what you are talking about.

I remember the problem, but I think I did fixed and rebased properly.

@dgw
Copy link
Member

dgw commented Dec 26, 2020

I can even see it on mobile. First commit:

Screenshot_20201226-145944.png

Fix commit, replacing mixed indentation and redundant verbiage with a consistent and concise docstring:

Screenshot_20201226-150006.png

@Exirel
Copy link
Contributor Author

Exirel commented Dec 26, 2020

Oh, OK. Weird.

@Exirel Exirel force-pushed the tests-deprecate-test-tools branch from 3f10b01 to b8d5144 Compare December 29, 2020 16:01
@Exirel
Copy link
Contributor Author

Exirel commented Dec 29, 2020

OK, now it should be good!

Copy link
Member

@dgw dgw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was putting off re-reviewing this because of the line count, but I forgot that it was only waiting for history cleanup. 😅

@dgw dgw merged commit 3a66002 into sopel-irc:master Jan 11, 2021
@Exirel Exirel deleted the tests-deprecate-test-tools branch May 1, 2021 17:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants