Skip to content

In tests use -m instead of -k for running tests by marker expressions #287

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

Merged
merged 2 commits into from
Jan 13, 2019

Conversation

sliwinski-milosz
Copy link
Contributor

@sliwinski-milosz sliwinski-milosz commented Jan 10, 2019

In tests we use -k for running tests by markers (tags). That approach works for pytest<4.1.0. Pytest documentation says nothing about ability to use -k for selecting markers.
https://docs.pytest.org/en/latest/usage.html#specifying-tests-selecting-tests

Correct approach is to use -m for markers.

Latest pytest==4.1.0 removes some legacy code related to markers. My guess is that it also removed ability to use -k for selecting markers:
(removal of store_legacy_markinfo function)
pytest-dev/pytest@9f9f6ee#diff-b1e6724759432cb1b166981e45a4a25cL238

Please note that some tests are failing because of the warnings that break regex checks of tests results. Those warnings are fixed in #282

This and mentioned above pull request together make our tests green again.

Copy link
Contributor

@youtux youtux left a comment

Choose a reason for hiding this comment

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

So do we need #282 merged before this PR will pass the test suite?

@sliwinski-milosz
Copy link
Contributor Author

I would do that other way round. I would merge this pull request (as it is only tests update and is quite obvious) and then I would update #282 to check if tests pass :)

@youtux
Copy link
Contributor

youtux commented Jan 12, 2019

Then we need the help of @pytest-dev/pytest-bdd-admin, as I don't have superpowers to merge this PR.

@sliwinski-milosz
Copy link
Contributor Author

I thought you have :D Could you please approve this PR to let them know that you are fine with changes?

@sliwinski-milosz
Copy link
Contributor Author

Other option is to merge this PR to #282 . What do you think?

@youtux
Copy link
Contributor

youtux commented Jan 12, 2019

Also fine to me, since we are effectively trying to fix CI builds due to pytest changes.

@sliwinski-milosz
Copy link
Contributor Author

Merged to #282 in the forked repository

@sliwinski-milosz
Copy link
Contributor Author

@youtux as you have now superpowers, could you please merge this to master? It would make #282 easier to review ;-)

@youtux
Copy link
Contributor

youtux commented Jan 13, 2019

I still don't have superpowers, @sliwinski-milosz

@The-Compiler
Copy link
Member

Superpowers, I hear? 🙈

@The-Compiler The-Compiler merged commit 6db3496 into pytest-dev:master Jan 13, 2019
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