Skip to content

MarkGenerator: Report unknown mark warning at caller level #5929

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

Closed
wants to merge 1 commit into from
Closed

MarkGenerator: Report unknown mark warning at caller level #5929

wants to merge 1 commit into from

Conversation

allanlewis
Copy link

@allanlewis allanlewis commented Oct 8, 2019

Currently, PytestUnknownMarkWarning is reported at the level of MarkGenerator, which is unhelpful to callers as the warning is shown as being "caused" by pytest. This commit therefore increases the stack level passed to the warning call to 2, which will make the warning log as being "caused" by the caller, which is much more helpful.

  • Target the master branch for bug fixes, documentation updates and trivial changes.
  • Target the features branch for new features, improvements, and removals/deprecations.
  • Include documentation when adding new features.
  • Include new tests or update existing tests when applicable.

Unless your change is trivial or a small documentation fix (e.g., a typo or reword of a small section) please:

  • Create a new changelog file in the changelog folder, with a name like <ISSUE NUMBER>.<TYPE>.rst. See changelog/README.rst for details.
  • Add yourself to AUTHORS in alphabetical order;

Closes #5928.

@allanlewis
Copy link
Author

Should I add a test for this?

@nicoddemus
Copy link
Member

Hi @allanlewis, thanks a lot for this!

Yes, a test would be appreciated, thanks.

@blueyed
Copy link
Contributor

blueyed commented Oct 9, 2019

Test might be nice, but for this kind of things it is often not tested already (and not trivial).
+0 for a test from me.

@nicoddemus
Copy link
Member

Test might be nice, but for this kind of things it is often not tested already (and not trivial).

Indeed, but we should have tests for warning locations because those things can regress (a refactoring might change the call stack depth).

But @allanlewis let us know if you have trouble coming up with a test, we would be glad to help!

@blueyed
Copy link
Contributor

blueyed commented Oct 15, 2019

Related: #4445

Currently, `PytestUnknownMarkWarning` is reported at the level of
`MarkGenerator`, which is unhelpful to callers as the warning is shown as being
"caused" by pytest. This commit therefore increases the stack level passed to
the warning call to 2, which will make the warning log as being "caused" by the
caller, which is much more helpful.

Resolves #5928.
@allanlewis
Copy link
Author

Rebased on master.
Is there anything else to do here, or are we just waiting for the tests from #5984?

@blueyed
Copy link
Contributor

blueyed commented Oct 29, 2019

@allanlewis
Maybe pull it into #5984, so that it gets tested there actually?

@allanlewis
Copy link
Author

@allanlewis
Maybe pull it into #5984, so that it gets tested there actually?

I don't think I have permissions to do that - @nicoddemus ?

@blueyed
Copy link
Contributor

blueyed commented Oct 29, 2019

@xibalba01 could do it.

@blueyed
Copy link
Contributor

blueyed commented Oct 29, 2019

Just for the record, hypothesis failed the build here again (osx):

=================================== FAILURES ===================================
______________________ TestMetafunc.test_idval_hypothesis ______________________
[gw0] darwin -- Python 3.7.0 /Users/travis/build/pytest-dev/pytest/.tox/py37-xdist/bin/python
self = <metafunc.TestMetafunc object at 0x1066d0400>
    @hypothesis.given(strategies.text() | strategies.binary())
>   @hypothesis.settings(
        deadline=400.0
    )  # very close to std deadline and CI boxes are not reliable in CPU power
    def test_idval_hypothesis(self, value):
E   hypothesis.errors.FailedHealthCheck: Data generation is extremely slow: Only produced 7 valid examples in 1.12 seconds (0 invalid ones and 2 exceeded maximum size). Try decreasing size of the data you're generating (with e.g.max_size or max_leaves parameters).
E   See https://hypothesis.readthedocs.io/en/latest/healthchecks.html for more information about this. If you want to disable just this health check, add HealthCheck.too_slow to the suppress_health_check settings for this test.
testing/python/metafunc.py:195: FailedHealthCheck
---------------------------------- Hypothesis ----------------------------------
You can add @seed(35364130883208350154606057602597663902) to this test or run pytest with --hypothesis-seed=35364130883208350154606057602597663902 to reproduce this failure.
=============================== warnings summary ===============================

@phloose
Copy link

phloose commented Oct 29, 2019

@xibalba01 could do it.

Sure i can try! I would have to add @allanlewis's fork as remote, right?! Never did this kind of "merging" of pull requests :)

@blueyed
Copy link
Contributor

blueyed commented Oct 29, 2019

Yes. with https://github.com/github/hub you can do hub remote add -p allanlewis, then git fetch it.
But works more manually of course, too. I can recommend using / looking at hub though in general.

@phloose
Copy link

phloose commented Oct 29, 2019

Ok just did it manually. Will have a look at hub

@blueyed
Copy link
Contributor

blueyed commented Oct 29, 2019

Closing in favor of #5984.
See #5984 (comment).
Thanks for having started on this!

@blueyed blueyed closed this Oct 29, 2019
@allanlewis
Copy link
Author

Thanks, @blueyed - I'm happy to defer to #5984.

@allanlewis allanlewis deleted the fix-unknown-mark-warning-stack-level branch October 29, 2019 14:34
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.

PytestUnknownMarkWarning should reference user code, not pytest
4 participants