Skip to content

Turn warnings into errors WIP, waiting #2598 #2599

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

Conversation

nicoddemus
Copy link
Member

This fixes #2588

When I started working on this I noticed I needed to control warnings on a per-test level, hence #2598 in which this branch is built on. We should review and consider merging this only if #2598 is.

As a test, I reverted 62556ba locally to see if this change would catch the regression, and indeed 25 tests failed so this seems to do the trick.

@@ -170,32 +170,6 @@ def f():
with pytest.deprecated_call():
f()

def test_deprecated_function_already_called(self, testdir):
Copy link
Member Author

Choose a reason for hiding this comment

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

Realized that this test is already covered by test_deprecated_call_modes so it was redundant.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.004%) to 90.808% when pulling 696ca61 on nicoddemus:turn-warnings-into-errors into abb5d20 on pytest-dev:features.

@@ -705,6 +711,7 @@ def test_xfail(x):
result = testdir.runpytest()
result.stdout.fnmatch_lines('* 2 passed, 1 xpassed in *')

@pytest.mark.filterwarnings('ignore:Applying marks directly to parameters')
Copy link
Member

Choose a reason for hiding this comment

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

those mark applications are all the same, can we assign a speaking name to that?

Copy link
Member

Choose a reason for hiding this comment

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

I wonder if those tests should just be using pytest.param instead?

Copy link
Member Author

Choose a reason for hiding this comment

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

those mark applications are all the same, can we assign a speaking name to that?

Good idea, will do.

I wonder if those tests should just be using pytest.param instead?

I think we should keep them in to ensure the old method still works, so we should remove them only when we remove the functionality.

@The-Compiler
Copy link
Member

Looks like the numpy comparison is broken in some way - a perfect example why I think deprecation warnings should be shown in all testsuites by default, even pytest itself would've benefitted 😉

cc @kalekundert

@The-Compiler
Copy link
Member

Hmm, and there are ResourceWarnings on Windows with trial.

@nicoddemus
Copy link
Member Author

Looks like the numpy comparison is broken in some way - a perfect example why I think deprecation warnings should be shown in all testsuites by default, even pytest itself would've benefitted

Heh. Not really broken, but definitely deprecated. I will take a look later, but if someone knows how to do it properly please let me know.

I will take a look at the trial errors later as well. 👍

@nicoddemus
Copy link
Member Author

Heh. Not really broken, but definitely deprecated. I will take a look later, but if someone knows how to do it properly please let me know.

I gave it a shot but had no luck, @kalekundert could you take a look and see what's the best way to preserve this functionality? I'm not sure it is possible, it seems they want to deprecate the == operator itself, but I might be wrong. We might consider ignoring that deprecation locally.

@kalekundert
Copy link
Contributor

I'll try to give it a look this weekend.

@nicoddemus
Copy link
Member Author

I'll try to give it a look this weekend.

Awesome, thanks!

@nicoddemus
Copy link
Member Author

The trial warning seems to be a problem in Twisted, reported a new issue: https://twistedmatrix.com/trac/ticket/9227

@coveralls
Copy link

Coverage Status

Coverage remained the same at 91.917% when pulling 3eb4b16 on nicoddemus:turn-warnings-into-errors into 1b732fe on pytest-dev:features.

@nicoddemus nicoddemus force-pushed the turn-warnings-into-errors branch from 3eb4b16 to bda07d8 Compare July 23, 2017 00:44
@nicoddemus
Copy link
Member Author

Rebased and applied the requested changes.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 91.917% when pulling bda07d8 on nicoddemus:turn-warnings-into-errors into 1b732fe on pytest-dev:features.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage remained the same at 91.917% when pulling bda07d8 on nicoddemus:turn-warnings-into-errors into 1b732fe on pytest-dev:features.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 91.917% when pulling 499df4a on nicoddemus:turn-warnings-into-errors into 1b732fe on pytest-dev:features.

Travis recently has changed its dist from "precise" to "trusty", so
some Python versions are no longer installed by default
@nicoddemus nicoddemus force-pushed the turn-warnings-into-errors branch 2 times, most recently from 20f607d to d5bb200 Compare July 23, 2017 03:52
@coveralls
Copy link

Coverage Status

Coverage remained the same at 91.917% when pulling d5bb200 on nicoddemus:turn-warnings-into-errors into 1b732fe on pytest-dev:features.

@@ -32,6 +29,12 @@ env:

matrix:
include:
- env: TOXENV=py26
Copy link
Member Author

@nicoddemus nicoddemus Jul 23, 2017

Choose a reason for hiding this comment

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

These changes are required with the migration of Travis images default from Precise to Trusty.

@The-Compiler
Copy link
Member

Thanks!

@The-Compiler The-Compiler merged commit 309152d into pytest-dev:features Jul 23, 2017
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.

5 participants