Skip to content

Start moving to pytest #1723

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 20 commits into from
Closed
Show file tree
Hide file tree
Changes from 16 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ python:
# - "pypy3"

install:
- pip install -r test-requirements.txt
- pip install -r requirements-testing.txt
- python setup.py install

script:
Expand Down
7 changes: 7 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,13 @@ pass inferior arguments via `-a`:
$ PYTHONPATH=$PWD scripts/myunit -m mypy.test.testlex -v '*backslash*'
$ ./runtests.py mypy.test.testlex -a -v -a '*backslash*'

Mypy is currently in the process of converting its tests from myunit to pytest.
Some of the tests, such as `test_check`, have already been converted. To run
these, you must use pytest instead, and use `-k` instead of `-a`:

$ ./runtests.py pytest -t -k -t NestedListAssignment
$ py.test -k NestedListAssignment

Copy link
Collaborator

Choose a reason for hiding this comment

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

"To run these, you must use pytest instead", sounds like these tests might not run if you just say ./runtests.py. They do, right?

Maybe a clearer description here would be something like "To run these unit tests individually, you can run py.test directly, or pass inferior arguments with -t." (I think you mean -t rather than -k -- is that right or am I misreading the example?)

Copy link
Collaborator

Choose a reason for hiding this comment

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

👍

You can also run the type checker for manual testing without
installing anything by setting up the Python module search path
suitably (the lib-typing/3.2 path entry is not needed for Python 3.5
Expand Down
22 changes: 22 additions & 0 deletions conftest.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
import pytest

def pytest_addoption(parser):
parser.addoption('--update-testcases', action='store_true',
dest='UPDATE_TESTCASES')

# mypy.test.helpers defines several top-level utility functions that
# pytest will pick up as tests. This removes them.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Hmm. How about just renaming those functions? Is it just testfile_pyversion and testcase_pyversion? This logic is complex enough that I worry about it being fragile.

Copy link
Collaborator

Choose a reason for hiding this comment

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

👍

def pytest_collection_modifyitems(items):
to_remove = []

for i, item in enumerate(items):
if (isinstance(item, pytest.Function) and
item.function.__module__ == 'mypy.test.helpers' and
# This is to prevent removing data-driven tests, which are
# defined by mypy.test.helpers.PytestSuite.
not getattr(item.function, 'is_test_attr', False)):
to_remove.append(i)

# reversed is to prevent changing indexes that haven't been removed yet.
for index in reversed(to_remove):
items.pop(index)
Copy link
Collaborator

Choose a reason for hiding this comment

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

This gives the algorithmist in me a bit of the jitters because it's a quadratic-time algorithm -- each pop in the middle of the list takes linear time to shift over everything that was to the right of the deleted element. See http://accidentallyquadratic.tumblr.com/ for entertaining examples of such algorithms coming up to bite people in real life.

Something like

items[:] = [item for item in items
            if isinstance(item, pytest.Function) and ... ]

would avoid this problem, and probably actually be clearer to boot.

Copy link
Collaborator

Choose a reason for hiding this comment

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

(moot thanks to the other edit that cuts this function)

31 changes: 23 additions & 8 deletions mypy/test/data.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,14 @@
from os import remove, rmdir
import shutil

from typing import Callable, List, Tuple
from typing import Callable, List, Tuple, Any

from mypy.myunit import TestCase, SkipTestCaseException


def parse_test_cases(
path: str,
perform: Callable[['DataDrivenTestCase'], None],
perform: Callable[[Any, 'DataDrivenTestCase'], None],
Copy link
Collaborator

Choose a reason for hiding this comment

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

Hmm. What's up with this Any type? Always preferable to avoid those where possible -- where it isn't, it's good to explain in a comment somewhere what the obstacle is.

base_path: str = '.',
optional_out: bool = False,
include_path: str = None,
Expand Down Expand Up @@ -89,29 +89,34 @@ def parse_test_cases(


class DataDrivenTestCase(TestCase):
name = None # type: str
input = None # type: List[str]
output = None # type: List[str]

file = ''
line = 0

perform = None # type: Callable[['DataDrivenTestCase'], None]
perform = None # type: Callable[..., None]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this meant to be the same signature as the perform parameter to parse_test_cases above?


# (file path, file content) tuples
files = None # type: List[Tuple[str, str]]

clean_up = None # type: List[Tuple[bool, str]]

is_skip = False
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not quite sure how to read this name. "Is skip" doesn't sound right to me -- I don't generally think of "skip" or "a skip" as a kind of thing that a test or a test case can be. Maybe is_skippable or should_skip or to_skip?

Maybe even better would be "marked_skip" -- I see in your PytestSuite logic below that there's actually another condition under which you also end up telling pytest to skip the resulting test item. So the tests that "we should skip" or that "are to be skipped" are a little broader than the tests that this attribute is true for.

Copy link
Collaborator

Choose a reason for hiding this comment

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

👍


def __init__(self, name, input, output, file, line, lastline,
perform, files):
super().__init__(name)
self.name = name
self.input = input
self.output = output
self.lastline = lastline
self.file = file
self.line = line
self.perform = perform
self.files = files
self.is_skip = self.name.endswith('-skip')

def set_up(self) -> None:
super().set_up()
Expand All @@ -137,19 +142,29 @@ def add_dirs(self, dir: str) -> List[str]:
os.mkdir(dir)
return dirs

def run(self):
if self.name.endswith('-skip'):
raise SkipTestCaseException()
def run(self, obj: Any = None):
if obj is None:
Copy link
Collaborator

Choose a reason for hiding this comment

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

What is this obj parameter? For understanding this logic it'd be really helpful to have a description of it somewhere.

Perhaps even a type. :-)

# XXX: The unit tests are being converted over to pytest. Due to
# modifications requires to make BOTH run at the moment, this branch
# is necessary. It should be removed once all the tests relying on
# DataDrivenTestCase are converted to pytest.
if self.is_skip:
raise SkipTestCaseException()
else:
self.perform(self)
else:
self.perform(self)
assert not self.is_skip
self.perform(obj, self)

def tear_down(self) -> None:
# First remove files.
for is_dir, path in reversed(self.clean_up):
if not is_dir:
if not is_dir and os.path.exists(path):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Huh. Why does this condition become necessary where it wasn't before?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That isn't actually related to pytest. It's just because, if the test case were killed before it finished, the files wouldn't always exist, which ended up throwing an exception, which was annoying (to say the least).

Copy link
Collaborator

Choose a reason for hiding this comment

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

I see. Sounds like a good bug to fix!

Would you send this as a separate, tiny PR? That way it can have its own commit message that explains it's fixing this bug, and also it doesn't sit here in this unrelated PR (and ultimately this commit in the final history) causing the reader to wonder what purpose it serves and what mysterious interaction with pytest brings it into this change.

I expect that separate PR can be merged very quickly, too. :-)

remove(path)
# Then remove directories.
for is_dir, path in reversed(self.clean_up):
if not os.path.exists(path):
continue
if is_dir:
pycache = os.path.join(path, '__pycache__')
if os.path.isdir(pycache):
Expand Down
41 changes: 41 additions & 0 deletions mypy/test/helpers.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import sys
import re
import os
import pytest # type: ignore
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why the type: ignore?

Ah, we don't have pytest in typeshed. This could be OK for now -- particularly as pytest is awfully metaprogramming-heavy and the stubs may end up with a fair number of Anys in them -- but we should really make some kind of stub, even if it's full of Any, rather than have a type: ignore here for long.

And any time we write a type: ignore we should always comment it with the reason why it's necessary.


from typing import List, Dict, Tuple

Expand Down Expand Up @@ -284,3 +285,43 @@ def normalize_error_messages(messages: List[str]) -> List[str]:
for m in messages:
a.append(m.replace(os.sep, '/'))
return a


@pytest.fixture(scope='function')
def test(request):
test = request.function.test
test.set_up()
request.addfinalizer(test.tear_down)
return test


class PytestSuite:
"""Assists in setting up data-driven test cases for pytest."""
@classmethod
def setup_tests(cls):
"""
Sets up the child class's test case. The child must have a method
`cases` that returns a list of `DataDrivenTestCase`s. This method will
load the data-driven test cases and use setattr to assign it to the
class, which will allow pytest to recognize the test.

Note that this method **must** be run after the definition of any
functions used by PytestSuite.cases.
"""
c = cls.cases() # type: List[DataDrivenTestCase]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's add an implementation of cases on this base class that just returns an empty list. That would let us do two or three good things:

  • As I remarked over on conftest.py, it would mean that collect_tests could be invoked uniformly on the base class just like on its subclasses, cutting a special case.
  • It would make this function well typed! As it is, we're trying to invoke a method cases on a value (cls) whose type is Type[PytestSuite] -- but PytestSuite has no method by that name, so that method call won't type-check. You state in the docstring a requirement that derived classes possess such a method, but this is a kind of requirement that we can express quite well in the type system rather than in English; and when we can do that, we get all the benefits of the type checker in making it a requirement that we can really depend on in thinking about related code. (Which is what makes this type checker so interesting in the first place!) So let's do that.
  • Let's also give both collect_tests and that cases method type annotations, so that the type-checker does its full work on them.

for test in c:
def func(self, test):
test.run(self)
func.is_test_attr = True
if test.is_skip:
func = pytest.mark.skip(reason='Test ends with -skip')(func)
if 'FastParse' in test.name and not test.is_skip:
try:
import mypy.fastparse
except SystemExit:
func = pytest.mark.skip(
reason='You must install the typed_ast package in '
'order to run this test')(func)
func.test = test
setattr(cls, test.name, func)
# setattr(cls, test.name.replace('test', 'test_', 1), func)
29 changes: 14 additions & 15 deletions mypy/test/testcheck.py → mypy/test/test_check.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,18 +4,18 @@
import re
import shutil
import sys
import mypy
import pytest # type: ignore

from typing import Tuple, List, Dict, Set

from mypy import build
import mypy.myunit # for mutable globals (ick!)
from mypy.build import BuildSource, find_module_clear_caches
from mypy.myunit import Suite, AssertionFailure
from mypy.test.config import test_temp_dir, test_data_prefix
from mypy.test.data import parse_test_cases, DataDrivenTestCase
from mypy.test.helpers import (
assert_string_arrays_equal, normalize_error_messages,
testcase_pyversion, update_testcase_output,
normalize_error_messages, testcase_pyversion, update_testcase_output,
PytestSuite, test
)
from mypy.errors import CompileError

Expand Down Expand Up @@ -61,13 +61,13 @@
]


class TypeCheckSuite(Suite):

def cases(self) -> List[DataDrivenTestCase]:
class TestTypeCheck(PytestSuite):
@classmethod
def cases(cls):
c = [] # type: List[DataDrivenTestCase]
for f in files:
c += parse_test_cases(os.path.join(test_data_prefix, f),
self.run_test, test_temp_dir, True)
cls.run_test, test_temp_dir, True)
return c

def run_test(self, testcase: DataDrivenTestCase) -> None:
Expand Down Expand Up @@ -122,13 +122,12 @@ def run_test_once(self, testcase: DataDrivenTestCase, incremental=0) -> None:
a = e.messages
a = normalize_error_messages(a)

if output != a and mypy.myunit.UPDATE_TESTCASES:
if output != a and pytest.config.getoption('UPDATE_TESTCASES'):
update_testcase_output(testcase, a, mypy.myunit.APPEND_TESTCASES)

assert_string_arrays_equal(
output, a,
assert output == a, \
'Invalid type checker output ({}, line {})'.format(
testcase.file, testcase.line))
testcase.file, testcase.line)
Copy link
Collaborator

Choose a reason for hiding this comment

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

How does the output from this turn out when there's a mismatch?

The assert_string_arrays_equal helper is pretty nice for comparing things that look like the error output from a typechecker -- I'd want to make sure we get something equally nice after the transition. The most essential feature might be just the fact that it prints both actual and expected as sequences of lines.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@gnprice Given the following toy test case:

def test_xyz():
    l1 = [
        'abc',
        'def',
        'ghi'
    ]

    l2 = [
        'abc',
        'dfg',
        'ghi',
        'jkl'
    ]

    assert l1 == l2

Pytest prints:

====================================== test session starts =======================================
platform linux -- Python 3.4.4, pytest-2.9.2, py-1.4.31, pluggy-0.3.1
rootdir: /media/ryan/stuff/mypy, inifile: pytest.ini
plugins: xdist-1.15.dev17+ng4fc9cb6.d20160618
collected 1 items 

test_xyz.py F

============================================ FAILURES ============================================
____________________________________________ test_xyz ____________________________________________

    def test_xyz():
        l1 = [
            'abc',
            'def',
            'ghi'
        ]

        l2 = [
            'abc',
            'dfg',
            'ghi',
            'jkl'
        ]

>       assert l1 == l2
E       assert ['abc', 'def', 'ghi'] == ['abc', 'dfg', 'ghi', 'jkl']
E         At index 1 diff: 'def' != 'dfg'
E         Right contains more items, first extra item: 'jkl'
E         Use -v to get the full diff

test_xyz.py:15: AssertionError
==================================== 1 failed in 0.01 seconds ====================================

Copy link
Collaborator

@gnprice gnprice Jul 3, 2016

Choose a reason for hiding this comment

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

Yeah, I've used pytest in other projects, and I appreciate the fancy output on asserts. The thing is that assert_string_arrays_equal is really kind of a misnomer for the purpose that that helper serves for us in our myunit tests. It's not about just arrays of strings -- it's really optimized for lists of lines. And it's quite good at that, in a way that the generic pytest handling of lists isn't for this case.

For example, say I mess up one of the check tests, testListAssignmentFromTuple, like so:

 [a, c], c = t, c  # E: Incompatible types in assignment (expression has type "B", variable has type "C")
-[a, a, a], c = t, c  # E: Need more than 2 values to unpack (3 expected)
+[a, a, a], c = t, c  # E: Need more than 2 values to unpack (4 expected)
 [a], c = t, c  # E: Too many values to unpack (1 expected, 2 provided)

Our existing myunit framework will print this output:

$ PYTHONPATH=$PWD scripts/myunit -m mypy.test.testcheck '*ListAssignmentFromTuple'
Expected:
  main:6: error: Incompatible types in assignment (expression has type "B", v...
  main:7: error: Need more than 2 values to unpack (4 expected) (diff)
  main:8: error: Too many values to unpack (1 expected, 2 provided)
Actual:
  main:6: error: Incompatible types in assignment (expression has type "B", v...
  main:7: error: Need more than 2 values to unpack (3 expected) (diff)
  main:8: error: Too many values to unpack (1 expected, 2 provided)

Alignment of first line difference:
  E: ...2 values to unpack (4 expected)
  A: ...2 values to unpack (3 expected)
                            ^
Traceback (most recent call last):
  File "/home/greg/w/mypy/mypy/test/data.py", line 144, in run
    self.perform(self)
  File "/home/greg/w/mypy/mypy/test/testcheck.py", line 93, in run_test
    self.run_test_once(testcase)
  File "/home/greg/w/mypy/mypy/test/testcheck.py", line 144, in run_test_once
    testcase.file, testcase.line))
  File "/home/greg/w/mypy/mypy/test/helpers.py", line 85, in assert_string_arrays_equal
    raise AssertionFailure(msg)
AssertionFailure: Invalid type checker output (/home/greg/w/mypy/test-data/unit/check-lists.test, line 34)

test_testcheck_TypeCheckSuite.testListAssignmentFromTuple failed

1/1 test cases failed.
*** FAILURE ***

It prints both the actual and expected arrays as a full line for each line -- so that they look nearly just like the actual way these error messages would come out in normal operation. (They're slightly indented, which is probably helpful, and also abbreviated, which maybe we actually shouldn't do.) That's extremely useful when debugging a failure.

Then it also highlights the mismatched lines with the suffix " (diff)", and it even drills into the first pair of different lines and highlights the column where they first differ.

Your current version will print this:

$ time py.test -k ListAssignmentFromTuple mypy/test/test_check.py ============================ test session starts =============================
platform linux -- Python 3.5.1, pytest-2.9.2, py-1.4.31, pluggy-0.3.1
rootdir: /home/greg/w/mypy, inifile: pytest.ini
collected 1386 items 

mypy/test/test_check.py F

================================== FAILURES ==================================
_________________ TestTypeCheck.testListAssignmentFromTuple __________________

self = <mypy.test.test_check.TestTypeCheck object at 0xb64874cc>
test = <mypy.test.data.DataDrivenTestCase object at 0xb668c4ac>

    def func(self, test):
>       test.run(self)

../../mypy/test/helpers.py:314: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
../../mypy/test/data.py:163: in run
    self.perform(obj, self)
../../mypy/test/test_check.py:82: in run_test
    self.run_test_once(testcase)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = <mypy.test.test_check.TestTypeCheck object at 0xb64874cc>
testcase = <mypy.test.data.DataDrivenTestCase object at 0xb668c4ac>
incremental = 0

    def run_test_once(self, testcase: DataDrivenTestCase, incremental=0) -> None:
        find_module_clear_caches()
        pyversion = pyversion_testcase(testcase.file, testcase.name)
        program_text = '\n'.join(testcase.input)
        module_name, program_name, program_text = self.parse_options(program_text)
        flags = self.parse_flags(program_text)
        output = testcase.output
        if incremental:
            flags.append(build.INCREMENTAL)
            if incremental == 1:
                # In run 1, copy program text to program file.
                output = []
                with open(program_name, 'w') as f:
                    f.write(program_text)
                    program_text = None
            elif incremental == 2:
                # In run 2, copy *.py.next files to *.py files.
                for dn, dirs, files in os.walk(os.curdir):
                    for file in files:
                        if file.endswith('.py.next'):
                            full = os.path.join(dn, file)
                            target = full[:-5]
                            shutil.copy(full, target)
        source = BuildSource(program_name, module_name, program_text)
        try:
            res = build.build(target=build.TYPE_CHECK,
                              sources=[source],
                              pyversion=pyversion,
                              flags=flags + [build.TEST_BUILTINS],
                              alt_lib_path=test_temp_dir)
            a = res.errors
        except CompileError as e:
            res = None
            a = e.messages
        a = normalize_error_messages(a)

        if output != a and pytest.config.getoption('UPDATE_TESTCASES'):
            update_testcase_output(testcase, a, mypy.myunit.APPEND_TESTCASES)

>       assert output == a, \
            'Invalid type checker output ({}, line {})'.format(
                testcase.file, testcase.line)
E       AssertionError: Invalid type checker output (mypy/test/data/check-lists.test, line 34)
E       assert ['main:6: err... 2 provided)'] == ['main:6: erro... 2 provided)']
E         At index 1 diff: 'main:7: error: Need more than 2 values to unpack (4 expected)' != 'main:7: error: Need more than 2 values to unpack (3 expected)'
E         Use -v to get the full diff

../../mypy/test/test_check.py:128: AssertionError
============ 1385 tests deselected by '-kListAssignmentFromTuple' ============
================= 1 failed, 1385 deselected in 2.56 seconds ==================

It's a lot harder here to tell what's going on. Partly that's because there's a lot of goo which pytest is printing out that's from the test framework and from the test_check test driver -- none of which will be at issue for >95% of the times that someone is looking at a test failure -- rather than from this particular test case. That's annoying but I think it's OK to have in an initial version of this; we can try later to cut it down.

More critically, there just isn't some of the useful information that's there in the myunit version. The differing pair of lines themselves are printed in full, but stuck together on one giant line with extra stuff around them, because the format is intended for shorter strings or other values. And the context lines aren't shown at all.

I can add -v as the message says, and then I get this at the end:

>       assert output == a, \
            'Invalid type checker output ({}, line {})'.format(
                testcase.file, testcase.line)
E       AssertionError: Invalid type checker output (mypy/test/data/check-lists.test, line 34)
E       assert ['main:6: err... 2 provided)'] == ['main:6: erro... 2 provided)']
E         At index 1 diff: 'main:7: error: Need more than 2 values to unpack (4 expected)' != 'main:7: error: Need more than 2 values to unpack (3 expected)'
E         Full diff:
E         ['main:6: error: Incompatible types in assignment (expression has type "B", '
E         'variable has type "C")',
E         -  'main:7: error: Need more than 2 values to unpack (4 expected)',
E         ?                                                     ^
E         +  'main:7: error: Need more than 2 values to unpack (3 expected)',
E         ?                                                     ^
E         'main:8: error: Too many values to unpack (1 expected, 2 provided)']

That's better but still I think a good deal harder to read than the myunit version. And it's an extra step.

So I'd like to see this PR leave assert_string_arrays_equal in the loop and continue to show the kind of output it produces, one way or another. There are surely improvements to make to that helper, and perhaps even ways to integrate it usefully with pytest fanciness, but those can be separate PRs to keep the scope of this one from growing.

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah, a unified context diff (like diff -u and the git diff default) would be great.

I've actually never tried the -i and -u options. They don't seem to particularly be documented, either, though I can infer what I think they're supposed to do from this:

        elif a == '-u':
            APPEND_TESTCASES = '.new'
            UPDATE_TESTCASES = True
        elif a == '-i':
            APPEND_TESTCASES = ''
            UPDATE_TESTCASES = True

(and from a vague recollection of hearing about that kind of functionality in myunit.)

Copy link
Collaborator

Choose a reason for hiding this comment

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

To clarify when it comes to the scope of this PR: I'd leave any improvements to the diff output (like switching to a unified diff) to a separate PR -- which could even come sooner! should be easy to do, and independent of this change -- and in this one just keep invoking the same assert_string_arrays_equal function to keep the change as simple as possible.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@gnprice @gvanrossum If I change the assertion to assert '\n'.join(output) == '\n'.join(a), the diff is actually really nice:

============================================ FAILURES ============================================
___________________________ TestTypeCheck.testListAssignmentFromTuple ____________________________

self = <mypy.test.test_check.TestTypeCheck object at 0x7f7145179588>
test = <mypy.test.data.DataDrivenTestCase object at 0x7f714557d780>

    def func(self, test):
>       test.run(self)

../../mypy/test/helpers.py:314: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
../../mypy/test/data.py:163: in run
    self.perform(obj, self)
../../mypy/test/test_check.py:82: in run_test
    self.run_test_once(testcase)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = <mypy.test.test_check.TestTypeCheck object at 0x7f7145179588>
testcase = <mypy.test.data.DataDrivenTestCase object at 0x7f714557d780>, incremental = 0

    def run_test_once(self, testcase: DataDrivenTestCase, incremental=0) -> None:
        find_module_clear_caches()
        pyversion = pyversion_testcase(testcase.file, testcase.name)
        program_text = '\n'.join(testcase.input)
        module_name, program_name, program_text = self.parse_options(program_text)
        flags = self.parse_flags(program_text)
        output = testcase.output
        if incremental:
            flags.append(build.INCREMENTAL)
            if incremental == 1:
                # In run 1, copy program text to program file.
                output = []
                with open(program_name, 'w') as f:
                    f.write(program_text)
                    program_text = None
            elif incremental == 2:
                # In run 2, copy *.py.next files to *.py files.
                for dn, dirs, files in os.walk(os.curdir):
                    for file in files:
                        if file.endswith('.py.next'):
                            full = os.path.join(dn, file)
                            target = full[:-5]
                            shutil.copy(full, target)
        source = BuildSource(program_name, module_name, program_text)
        try:
            res = build.build(target=build.TYPE_CHECK,
                              sources=[source],
                              pyversion=pyversion,
                              flags=flags + [build.TEST_BUILTINS],
                              alt_lib_path=test_temp_dir)
            a = res.errors
        except CompileError as e:
            res = None
            a = e.messages
        a = normalize_error_messages(a)

        if output != a and pytest.config.getoption('UPDATE_TESTCASES'):
            update_testcase_output(testcase, a, mypy.myunit.APPEND_TESTCASES)

>       assert '\n'.join(output) == '\n'.join(a), \
            'Invalid type checker output ({}, line {})'.format(
                testcase.file, testcase.line)
E       AssertionError: Invalid type checker output (mypy/test/data/check-lists.test, line 34)
E       assert 'main:6: erro..., 2 provided)' == 'main:6: error..., 2 provided)'
E         Skipping 137 identical leading characters in diff, use -v to show
E         Skipping 67 identical trailing characters in diff, use -v to show
E         - o unpack (4 expected
E         ?           ^
E         + o unpack (3 expected
E         ?           ^

../../mypy/test/test_check.py:128: AssertionError
====================== 1860 tests deselected by '-kListAssignmentFromTuple' ======================

Even if assert_string_arrays_equal is used, the function body will be dumped; this is more of a side-effect of pytest than of the assertion.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

With -v:

E       AssertionError: Invalid type checker output (mypy/test/data/check-lists.test, line 34)
E       assert 'main:6: erro..., 2 provided)' == 'main:6: error..., 2 provided)'
E           main:6: error: Incompatible types in assignment (expression has type "B", variable has type "C")
E         - main:7: error: Need more than 2 values to unpack (4 expected)
E         ?                                                   ^
E         + main:7: error: Need more than 2 values to unpack (3 expected)
E         ?                                                   ^
E           main:8: error: Too many values to unpack (1 expected, 2 provided)


if incremental and res:
self.verify_cache(module_name, program_name, a, res.manager)
Expand All @@ -145,9 +144,7 @@ def verify_cache(self, module_name: str, program_name: str, a: List[str],
modules = self.find_module_files()
modules.update({module_name: program_name})
missing_paths = self.find_missing_cache_files(modules, manager)
if missing_paths != error_paths:
raise AssertionFailure("cache data discrepancy %s != %s" %
(missing_paths, error_paths))
assert missing_paths == error_paths, 'cache data discrepancy'

def find_error_paths(self, a: List[str]) -> Set[str]:
hits = set()
Expand Down Expand Up @@ -209,3 +206,5 @@ def parse_flags(self, program_text: str) -> List[str]:
return m.group(1).split()
else:
return []

TestTypeCheck.setup_tests()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Hmm, typically the magic of pytest makes it so you don't need to make this kind of call up at toplevel. Is there another way we can prepare these?

Also, "setup" in the pytest context usually refers to something you do just before running some tests in order to set up some state that the test itself, or the application code it's testing, expect in order for the test to run. It's not normally something you'd want to do at toplevel like this. Is "collect" or "test collection" more what this step is about?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Nice!

19 changes: 10 additions & 9 deletions mypy/test/testcmdline.py → mypy/test/test_cmdline.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
from mypy.myunit import Suite, SkipTestCaseException
from mypy.test.config import test_data_prefix, test_temp_dir
from mypy.test.data import parse_test_cases, DataDrivenTestCase
from mypy.test.helpers import assert_string_arrays_equal
from mypy.test.helpers import PytestSuite, test

# Path to Python 3 interpreter
python3_path = sys.executable
Expand All @@ -23,20 +23,20 @@
cmdline_files = ['cmdline.test']


class PythonEvaluationSuite(Suite):

def cases(self) -> List[DataDrivenTestCase]:
class TestPythonEvaluation(PytestSuite):
@classmethod
def cases(cls) -> List[DataDrivenTestCase]:
c = [] # type: List[DataDrivenTestCase]
for f in cmdline_files:
c += parse_test_cases(os.path.join(test_data_prefix, f),
test_python_evaluation,
python_evaluation_test,
base_path=test_temp_dir,
optional_out=True,
native_sep=True)
return c


def test_python_evaluation(testcase: DataDrivenTestCase) -> None:
def python_evaluation_test(obj: None, testcase: DataDrivenTestCase) -> None:
# Write the program to a file.
program = '_program.py'
program_path = os.path.join(test_temp_dir, program)
Expand All @@ -58,9 +58,8 @@ def test_python_evaluation(testcase: DataDrivenTestCase) -> None:
# Remove temp file.
os.remove(program_path)
# Compare actual output to expected.
assert_string_arrays_equal(testcase.output, out,
'Invalid output ({}, line {})'.format(
testcase.file, testcase.line))
assert testcase.output == out, 'Invalid output ({}, line {})'.format(
testcase.file, testcase.line)


def parse_args(line: str) -> List[str]:
Expand All @@ -78,3 +77,5 @@ def parse_args(line: str) -> List[str]:
if not m:
return [] # No args; mypy will spit out an error.
return m.group(1).split()

TestPythonEvaluation.setup_tests()
Loading