Repository navigation
Tests behave differently without builtins fixtures #3111
Description
I was somewhat concerned that test builtins fixtures might not perfectly represent the production version of mypy, so I ran the tests without the fixtures.
The good news is that, as expected, fixtures make tests at least twice faster (pytest alone takes 212 sec without the fixtures, and 12 sec with).
The bad news is that 60 out of 2309 tests failed. Many of the failures were trivial (a slight change in the error message since the test was written, or not defining a type variables because it's already defined in the fixture). But a few were more serious.
For example this code (from check-class-namedtuple.test: testNewNamedTupleJoinTuple), instead of reporting the two revealed types, crashed mypy (under python 3.6):
class X(NamedTuple):
x: int
y: str
reveal_type([(3, 'b'), X(1, 'a')]) # E: Revealed type is 'builtins.list[Tuple[builtins.int, builtins.str]]'
reveal_type([X(1, 'a'), (3, 'b')]) # E: Revealed type is 'builtins.list[Tuple[builtins.int, builtins.str]]'
This code (from check-namedtuple.test: testNamedTupleMake) didn't report the second expected error message:
X = NamedTuple('X', [('x', int), ('y', str)])
reveal_type(X._make([5, 'a'])) # E: Revealed type is 'Tuple[builtins.int, builtins.str, fallback=__main__.X]'
X._make('a b') # E: Argument 1 to X._make has incompatible type "str"; expected Iterable[Any]
Not sure what the best solution is. Maybe it's worth having a daily run of all tests without fixtures using some special commit.
In any case, I made a branch that allows to easily run tests without fixtures because I couldn't find a command line parameter that would do it. I'll push a PR for it, although it's probably more appropriate for testing and discussion, not for merging.
Activity
pkch commentedon Apr 2, 2017
Edit: output updated now that I disabled
[file builtins.py]in addition to[builtins ...].And this is the output of
pytestwithout fixtures:JukkaL commentedon Apr 3, 2017
Thanks for looking into this! Would you like to create issues just for the crashes, as we should definitely investigate them?
ilevkivskyi commentedon Apr 3, 2017
I could immediately guess the cause for the crash in
normalize_type_alias:lib-stub/typing.pyihas all ofList,Set,Dict, but some fixtures for builtins don't havelist, orset, ordict. Then there is a crash, sincenormalize_type_aliasdoes not check whetherlistwas defined, it just translatesListtolistetc. This is why the crash happens on unknown node.pkch commentedon Apr 3, 2017
@JukkaL I did; luckily, only one real crash (well 2 tests crash for the same reason; and 4 tests crash for the reason @ilevkivskyi said)
@ilevkivskyi Actually, I just didn't notice that
[file builtins.py]is another way to effectively make a fixture. I've disabled it now, so no such crashes occur. I updated my comment with the test output log with the new output.pkch commentedon Apr 4, 2017
So there are 4 reasons tests may fail without builtins fixture:
Callableand other types (although perhapsOverloadedshould be treated the same asCallable). And since custom stub hastypedefined just once, it's seen asCallable, unlike wiht the realbuiltins.pyiwhere it's defined several times and hence isOverloaded.TypeVar('T')defined inside stub, instead of inside the test).My suggestions for each type of test:
mypy.myunit.AssertionFailure: Invalid type checker output, but inCaptured stderr callhave the same line numbers of expected and actual error messagespkch commentedon Apr 4, 2017
I forgot the rest of my proposal:
To make sure that without custom builtins stubs the tests would actually fail in cases 1), 3), 4), and pass in case 2) , we can:
runtests.pystill use custom builtins stubs (for faster local tests), but changetravis.yml(notappveyor.yml) to disable custom builtins stubs (it adds ~3 min to the tests)I can modify my #3112 to do that, if this approach seems desirable.
Alternatively, if we really want Travis CI to finish fast, we can create a new branch
release, and whenevermasteris updated, automatically run slow tests and automaticallypush --forcethe tip frommasterintoreleaseif those tests pass.JukkaL commentedon Oct 5, 2020
Closing due to lack of activity.