Skip to content

Stop faking builtins for tests #890

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
o11c opened this issue Oct 3, 2015 · 4 comments
Closed

Stop faking builtins for tests #890

o11c opened this issue Oct 3, 2015 · 4 comments

Comments

@o11c
Copy link
Contributor

o11c commented Oct 3, 2015

Recently, I wrote a patch that worked perfectly in mypy, but (after fixing one buggy test) caused several unit tests to fail because the unit tests don't use the real builtins, rather some file under mypy/test/data/fixtures/

It does not matter how fast your tests execute if they are not representative of what mypy will actually use. You should look for performance improvements elsewhere - perhaps by only ever using the parts of a module that are used from the main file.

@o11c
Copy link
Contributor Author

o11c commented Oct 3, 2015

Actually it looks like there are more buggy tests, but line 8 here is not a bug in the test: https://travis-ci.org/o11c/mypy/jobs/83400574#L1225

It should not be an error to assign a Tuple[T, ...] expression to a variable of type Sequence[T], and indeed this does work in mypy itself, just not in the unit tests.

@o11c
Copy link
Contributor Author

o11c commented Oct 3, 2015

I've managed to figure out what change was needed in the fixture (PR coming as soon as I fix the rest of the buggy tests), but this is still a horrible idea.

@JukkaL
Copy link
Collaborator

JukkaL commented Oct 7, 2015

Mypy mostly has unit tests, not integration tests (the pythoneval suite is a separate integration suite that uses full stubs). There are some drawbacks, but I rarely have had any issues that haven't been trivial to work around, and making tests run about 10x faster or so is definitely a huge win for productivity.

Benefits of having unit tests:

  • Tests run much faster. Fast tests is about the most important thing to speed up productivity on a typical project (maybe even more important than static typing). I can't afford to have a cluster of 10 machines running my tests to give a reasonable turnaround.
  • Test failures are much easier to debug, as there isn't a ton of cruft (all builtins) always in memory. It's actually possible to add debug prints to almost anywhere and get useful output without much noise.
  • Tests are isolated and only involve functionality related to the tests (more or less). With full stubs, every test would hit a large fraction of the code paths in the implementation and thus they wouldn't be very modular. This is much less important than the first 2 benefits, though. In parcitular, changes to real stubs don't break unit tests, which has been super handy during the development of mypy, but it will become less important as the builtins stabilize.

The main drawback is that you have to maintain the fixtures, but that's generally simple (just a little tedious, but less tedious than slow iteration speed due to not having them).

And yeah, and the fixtures are intended to be not 100% representative to get better test isolation. That can be tweaked on a case by case basis.

@JukkaL JukkaL added the wontfix label Oct 7, 2015
@JukkaL JukkaL closed this as completed Oct 7, 2015
@JukkaL
Copy link
Collaborator

JukkaL commented Oct 7, 2015

One additional point: parallelizing tests is also very useful because that also speeds up tests. Currently running mypy unit tests locally is kind of slow (as there are many tests), so the new parallel test driver will be very useful.

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

No branches or pull requests

2 participants