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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pre-PR for warnings on unknown markers #5015

Merged
merged 5 commits into from Mar 31, 2019

Conversation

Zac-HD
Copy link
Member

@Zac-HD Zac-HD commented Mar 31, 2019

This pull request is designed to make #4935 much smaller and easier to work on (or review). The four commits each make a helpful change that is also worth doing independently.

  • Typo fixes should be clear.
  • pytester was using an unregistered marker - adding it to the help is both useful for users and will avoid making usage a warning later.
  • We needed to document how a mark could be registered, so I've done that and linked to it from the section on --strict and marks.
  • Our test suite already uses both mark.issue and mark.issueNNN; standardising on the former makes selecting issue-relevant tests uniform - and means we won't need to register every number.

I'm targeting features so that I can rebase #4935 as soon as this is merged 馃槃

@Zac-HD Zac-HD requested a review from nicoddemus March 31, 2019 03:12
@codecov
Copy link

codecov bot commented Mar 31, 2019

Codecov Report

Merging #5015 into features will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff              @@
##           features    #5015      +/-   ##
============================================
+ Coverage     96.06%   96.06%   +<.01%     
============================================
  Files           114      114              
  Lines         25749    25750       +1     
  Branches       2548     2548              
============================================
+ Hits          24735    24736       +1     
  Misses          704      704              
  Partials        310      310
Impacted Files Coverage 螖
src/_pytest/mark/structures.py 92.61% <酶> (酶) 猬嗭笍
testing/test_capture.py 99.24% <100%> (酶) 猬嗭笍
testing/test_mark.py 99.37% <100%> (酶) 猬嗭笍
testing/python/integration.py 91.48% <100%> (酶) 猬嗭笍
src/_pytest/pytester.py 91.15% <100%> (+0.01%) 猬嗭笍
testing/python/fixtures.py 99.08% <100%> (酶) 猬嗭笍
testing/test_conftest.py 99.62% <100%> (酶) 猬嗭笍
testing/python/metafunc.py 95.23% <100%> (酶) 猬嗭笍

Continue to review full report at Codecov.

Legend - Click here to learn more
螖 = absolute <relative> (impact), 酶 = not affected, ? = missing data
Powered by Codecov. Last update 278b289...deade37. Read the comment docs.

doc/en/mark.rst Outdated Show resolved Hide resolved
@@ -166,6 +166,8 @@ filterwarnings =
# Do not cause SyntaxError for invalid escape sequences in py37.
default:invalid escape sequence:DeprecationWarning
pytester_example_dir = testing/example_scripts
markers =
issue
Copy link
Contributor

Choose a reason for hiding this comment

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

s/issue/gh_issue/ maybe?

Copy link
Member Author

Choose a reason for hiding this comment

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

We're already using issue, so I'd rather leave any changes to a future PR and just standardise here.

Copy link
Contributor

Choose a reason for hiding this comment

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

Well, a) I do not know how this is used actually, but b) you're touching all those lines already (and therefore it would be good to do it right away).

Copy link
Member

Choose a reason for hiding this comment

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

We're already using issue, so I'd rather leave any changes to a future PR and just standardise here.

I never used it myself. I think we can actually remove them in the future, unless someone steps up and mentions he/she uses it on their workflow.

Co-Authored-By: Zac-HD <Zac-HD@users.noreply.github.com>
@@ -42,8 +40,10 @@ Marks can be registered in ``pytest.ini`` like this:
slow
serial

This can be used to prevent users mistyping mark names by accident. Test suites that want to enforce this
should add ``--strict`` to ``addopts``:
When the ``--strict`` command-line flag is passed, any unknown marks applied
Copy link
Member

Choose a reason for hiding this comment

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

@Zac-HD how about using this PR and add --strict-marks as an alias to --strict, while also updating the docs to use the former?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'd prefer to leave functional changes to the other PR - will discuss there.

Copy link
Member

Choose a reason for hiding this comment

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

Sure, thanks!

@Zac-HD Zac-HD merged commit 407d74b into pytest-dev:features Mar 31, 2019
@Zac-HD Zac-HD deleted the mark-warning-prep branch March 31, 2019 23:11
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.

None yet

3 participants