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

Separate plugin tests from the core tests #4951

Merged
merged 3 commits into from
Oct 17, 2023

Conversation

Serene-Arc
Copy link
Contributor

As discussed on Zulip, this refactors the plugin tests into a specific folder. This is a minor but important first step in increasing the test base and tidying everything up, and allows developers to exclude the plugin tests with ease.

There are no changes to actual tests, beyond anything relating to paths, and all tests succeed as they did before.

@Serene-Arc Serene-Arc requested a review from JOJ0 October 16, 2023 03:07
@JOJ0
Copy link
Member

JOJ0 commented Oct 16, 2023

Would it be a bad idea to shorten the path a little? ie test/plugin/... instead of test/plugin_tests/...

(plugin singular!)

I'm thinking of firing one particular test is a rather long path already

currently we have

python -m unittest test.test_blabla.BlaBla.test_something

we would then get something like

python -m unittest test.plugin.test_blabla.BlaBla.test_something

@Serene-Arc
Copy link
Contributor Author

We should really be encouraging people to use pytest, not the unittest module. Then it's easier to ignore.

I'm not wedded to the directory name. In general I think more descriptive names are better, but if you feel strongly about it, I can change it.

@JOJ0
Copy link
Member

JOJ0 commented Oct 17, 2023

We should really be encouraging people to use pytest, not the unittest module. Then it's easier to ignore.

I'm not wedded to the directory name. In general I think more descriptive names are better, but if you feel strongly about it, I can change it.

For now I'm still up to changing it, yes. I think it's descriptive that a directory plugin/ within a directory named test/ is referring to unittests :-)

Question on the side: Please open an issue about updating the docs regarding pytest. I'll try to take care of it, but since I'm a noob around pytest still probably it would be good if you provide some basic info about it. Just some quick basiscs on how to run tests in quick words, I'll try to do the calligraphy then :-))

First question: How to run a single test with e.g. my example command above with pytest, but let's continue discussion about testing-docs in a separate issue and let's try to merge this one quickly.

@Serene-Arc
Copy link
Contributor Author

You literally just write the command pytest in the root of the project and it will collect and run all the tests. I will write specific instructions in a different PR, though. Pytest is better for a variety of reasons: there are a bunch of plugins we can make use of, and going forward there I intend to simplify and expand the tests wherever possible, which will probably make use of pytest features.

@Serene-Arc
Copy link
Contributor Author

@JOJ0 I just pushed the rename to just plugin. If you give your approval, we can merge.

@JOJ0
Copy link
Member

JOJ0 commented Oct 17, 2023

You literally just write the command pytest in the root of the project and it will collect and run all the tests. I will write specific instructions in a different PR, though. Pytest is better for a variety of reasons: there are a bunch of plugins we can make use of, and going forward there I intend to simplify and expand the tests wherever possible, which will probably make use of pytest features.

I was referring to: How to run single tests :-)

Copy link
Member

@JOJ0 JOJ0 left a comment

Choose a reason for hiding this comment

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

You forgot to change the imports to also use that new module name :-) Please go ahead and merge once that's done. Thanks!

@Serene-Arc Serene-Arc merged commit 0ccd70d into beetbox:master Oct 17, 2023
13 of 14 checks passed
@Serene-Arc Serene-Arc deleted the test_separation branch October 17, 2023 11:12
@Serene-Arc
Copy link
Contributor Author

Always something you miss lol

@sampsyo
Copy link
Member

sampsyo commented Oct 20, 2023

Just wanted to express gratitude for doing this reorganization! Seems like a great idea.

sampsyo added a commit that referenced this pull request Oct 20, 2023
These references were out-of-date as of #4951. Not sure why we were
listing these individually in the first place; it's not like we want to
be strict about docstrings in tests...
@Serene-Arc
Copy link
Contributor Author

Seemed like a good first step for improving our testing

Serene-Arc pushed a commit to Serene-Arc/beets that referenced this pull request Oct 21, 2023
These references were out-of-date as of beetbox#4951. Not sure why we were
listing these individually in the first place; it's not like we want to
be strict about docstrings in tests...
Serene-Arc pushed a commit to Serene-Arc/beets that referenced this pull request Oct 21, 2023
These references were out-of-date as of beetbox#4951. Not sure why we were
listing these individually in the first place; it's not like we want to
be strict about docstrings in tests...
Serene-Arc pushed a commit to Serene-Arc/beets that referenced this pull request Oct 21, 2023
These references were out-of-date as of beetbox#4951. Not sure why we were
listing these individually in the first place; it's not like we want to
be strict about docstrings in tests...
Serene-Arc pushed a commit to Serene-Arc/beets that referenced this pull request Oct 21, 2023
These references were out-of-date as of beetbox#4951. Not sure why we were
listing these individually in the first place; it's not like we want to
be strict about docstrings in tests...
Serene-Arc pushed a commit to Serene-Arc/beets that referenced this pull request Oct 21, 2023
These references were out-of-date as of beetbox#4951. Not sure why we were
listing these individually in the first place; it's not like we want to
be strict about docstrings in tests...
Serene-Arc pushed a commit to Serene-Arc/beets that referenced this pull request Oct 21, 2023
These references were out-of-date as of beetbox#4951. Not sure why we were
listing these individually in the first place; it's not like we want to
be strict about docstrings in tests...
Serene-Arc pushed a commit to Serene-Arc/beets that referenced this pull request Oct 21, 2023
These references were out-of-date as of beetbox#4951. Not sure why we were
listing these individually in the first place; it's not like we want to
be strict about docstrings in tests...
Serene-Arc pushed a commit to Serene-Arc/beets that referenced this pull request Oct 21, 2023
These references were out-of-date as of beetbox#4951. Not sure why we were
listing these individually in the first place; it's not like we want to
be strict about docstrings in tests...
Serene-Arc pushed a commit to Serene-Arc/beets that referenced this pull request Oct 21, 2023
These references were out-of-date as of beetbox#4951. Not sure why we were
listing these individually in the first place; it's not like we want to
be strict about docstrings in tests...
michaeldiazh pushed a commit to michaeldiazh/beets that referenced this pull request Feb 19, 2024
These references were out-of-date as of beetbox#4951. Not sure why we were
listing these individually in the first place; it's not like we want to
be strict about docstrings in tests...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants