Skip to content

Add option to run tests without builtins stub fixtures - #3268

Closed
pkch wants to merge 5 commits into
python:masterfrom
pkch:cleantests
Closed

pkch wants to merge 5 commits into
python:masterfrom
pkch:cleantests

Conversation

@pkch

@pkch pkch commented Apr 27, 2017

Copy link
Copy Markdown
Contributor

This PR is a cleaned up and rebased version of #3189.

If the environment does not contain env. var. NO_BUILTINS_FIXTURES, this PR does nothing at all.

If env. var. NO_BUILTINS_FIXTURES is defined, this PR affects the tests as follows:

  • Builtins stub fixtures are disabled; that is, all tests are run using the normal builtins stub.
  • When comparing error output, only line numbers matter, not the contents; if the expected and actual output have identical line numbers, the test passes (note: multiple errors in the same line are treated as one). This is to help reduce the effort of analyzing failing tests, since minor changes in message wording is not the highest priority.
  • When a test fails, the actual output printed to stderr is limited to 25 lines per test. This is to avoid killing the terminal window when a few dozen tests fail with hundreds of lines actual output each.
  • mergetest.py skips not only builtins module but all modules other than __main__ and target. This is to make existing expected output work in the no-builtins-fixtures environment.

I chose the environment variable instead of a command-line option for runttests because this feature impacts code in locations where it's somewhat troublesome to obtain the command line options.

Right now, a few tests have no hope of working without builtins fixtures. To exclude them, use this command line:

NO_BUILTINS_FIXTURES=1 python runtests.py -x lint -x "unit-test mypy.test.testtypegen" -x "unit-test mypy.test.testtransform" -x "unit-test mypy.test.testsemanal" -p -k -p "not mypy/test/testfinegrained.py"

This run would take 5-6 min, and would produce 10 errors (all from pytest) on the current master. Over time, we can gradually fix the failing tests or mark them with "xfail", but this can be done separately from this PR.

@gvanrossum

Copy link
Copy Markdown
Member

Let's hold off on this until after the release; it touches more files than I like and it doesn't fix any important bugs.

@ilinum ilinum self-assigned this Jul 14, 2017
@ilinum

ilinum commented Jul 14, 2017

Copy link
Copy Markdown
Collaborator

@pkch Thanks for the PR!

Honestly, I would love to get rid of fixtures and thank you for working on it!

However, I think we should either have fixtures or none at all -- I am not a fan of the idea to have the two coexist. In particular, since you say there's only 10 tests failing, perhaps, we could just fix them and get rid of fixtures altogether.

@pkch

pkch commented Jul 15, 2017

Copy link
Copy Markdown
Contributor Author

The problem with getting rid of fixtures is that the tests would take a few min longer. I've been playing with some ways to reduce the runtime without fixtures, but didn't make much progress.

I'm guessing, it might be possible to make tests faster by avoiding the complete reset of mypy between tests, but xdist doesn't allow to specify that certain tests need to run on the same thread to preserve state.

@ilinum

ilinum commented Jul 19, 2017

Copy link
Copy Markdown
Collaborator

I see.

I think currently this is an incomplete feature and it would only make sense to merge it once it is complete.
Once it is complete, we can make all tests run without fixtures.

@ddfisher

Copy link
Copy Markdown
Collaborator

I think we may not want this change. I don't see much of a benefit from it -- we're not planning on moving to a fixture-less world any time soon, and I'm not sure what it gains us in the interim. It also touches a decent number of files, and the line-number-only matching feels over-complex.

@gvanrossum

Copy link
Copy Markdown
Member

I think we may not want this change. [...]

I agree. Unless @JukkaL disagrees let's not do this.

@JukkaL

JukkaL commented Jul 21, 2017

Copy link
Copy Markdown
Collaborator

I agree that this is not worth the extra complexity.

A potentially better way to improve the fixture situation would be to make the fixtures closer to the real stubs (e.g. make tuple always generic), to reduce the number of different fixtures, and to make the different fixtures more consistent with each other. However, each change needs to be individually evaluated, and significant regressions in pytest runtime probably aren't worth it.

Currently mypy often gives a hint if you forget to use the correct fixture, so the usability aspects should be less of a problem now.

@JukkaL JukkaL closed this Jul 21, 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