Prototype of common test suite format and runner for Mypy #436

vlasovskikh · 2017-05-24T07:19:17Z

Here is a prototype for the common test suite suggested in #434 that uses YAML as the test format. I've created a simple test runner for Mypy and added a couple of tests.

the-knights-who-say-ni · 2017-05-24T07:19:19Z

Hello, and thanks for your contribution!

I'm a bot set up to make sure that the project can legally accept your contribution by verifying you have signed the PSF contributor agreement (CLA).

Unfortunately we couldn't find an account corresponding to your GitHub username on bugs.python.org (b.p.o) to verify you have signed the CLA (this might be simply due to a missing "GitHub Name" entry in your b.p.o account settings). This is necessary for legal reasons before we can look at your contribution. Please follow the steps outlined in the CPython devguide to rectify this issue.

Thanks again to your contribution and we look forward to looking at it!

ilevkivskyi

This looks interesting. Is there a possibility/need to add (per file) flags common for all type checkers, like Python version against which to typecheck?

ilevkivskyi · 2017-05-28T12:31:08Z

test-data/check-modules.yml

+# Type checker test cases dealing with modules and imports.
+
+- case: Access imported definitions
+  files:


Is it possible to omit files: line? It looks redundant, but I have never worked with YAML.

@ilevkivskyi No, it's not possible to omit it here. In a sense, YAML is just a simpler way to write JSON objects, which are, from the Python's point of view, just dictionaries.

So, this YAML file:

# Type checker test cases dealing with modules and imports. - case: Access imported definitions files: - content: | ... - path: m.py content: | ...

is equivalent to the following Python dict:

[{'case': 'Access imported definitions', 'files': [{'content': '...\n'}, {'content': '...', 'path': 'm.py'}]}]

(You can convert a YAML string to a Python dict / list using yaml.load(yaml_string).)

So we need the files attribute, as it becomes the 'files' key in our test case dict.

ilevkivskyi · 2017-05-28T12:32:51Z

test-data/check-modules.yml

+        m.f(m.A())
+        m.x = m.A()
+    - path: m.py
+      content: |


Is it possible to somehow make this less verbose? Mypy tests just use single line [file m.py].

@ilevkivskyi I'm afraid not. path and content become the keys of the resulting dict: {'path': 'm.py', 'content': '...'}. I've made the file path main.py default so you don't have to type path for every test. But when you need a custom path, you have to provide it as an attribute.

OK, I see. Still, YAML looks quite flexible, let's see what others will say.

vlasovskikh · 2017-05-30T22:06:48Z

@ilevkivskyi Yes, YAML is extensible so we can add new attributes in case we need them.

For Python versions it could look like this:

- case: Assignment and var def
  python_versions:
    - "2.7"
    - "3.6"
  files:
    - content: |
        ...

or, equivalently, like this (similar to Python lists of strings):

- case: Assignment and var def
  python_versions: ["2.7", "3.6"]
  files:
    - content: |
        ...

gvanrossum

I've kicked the tires a bit... Came up with a few nits/suggestions.

gvanrossum · 2017-06-01T00:09:51Z

tests/mypy_test.py

+    output_lines = output.decode(ENCODING).strip().splitlines()
+    actual_files = inline_errors(output_lines, files)
+    for expected, actual in zip(files, actual_files):
+        assert expected.get('content', '') == actual.get('content', '')


While pytest produces nice diff-style output when an assert fails, there's one problem: if the expected output for several files doesn't match the actual output, pytest only shows the diff for the first file with a mismatch. This seems a problem (since sometimes unexpected errors in the second file provide a clue for the unexpected errors in the first file).

IMO it would look nice if the output for each file were concatenated with headers, e.g.:

-----x.py----- output -----y.py---- output

Then you could see it all at once.

gvanrossum · 2017-06-01T00:17:34Z

tests/mypy_test.py

+
+
+def strip_errors(s: Text) -> Text:
+    return re.sub(r'#\s*E:.*', r'', s)


Can we split on ' # ' (i.e. requiring at least one space on each side of '#')? See python/mypy#3417 -- the reason is that we want to allow error messages containing URLs of the form http://host/path#fragment to deep-link directly to a specific section of the docs (like https://github.com/python/mypy/pull/3411/files#diff-7739b07c7ae4561e51cbb36f31984c90R1016 -- still WIP).

gvanrossum · 2017-06-01T00:26:04Z

tests/mypy_test.py

+    return index, message, path
+
+
+@pytest.mark.parametrize('filename,case,files', load_test_cases('test-data'))


This appears to be calling load_test_cases() even if this test were not selected. Upon further inspection load_test_cases() is a generator function so the call doesn't actually do any work, but it still creeped me out. (Maybe load_test_cases() deserves a comment explaining why it should be a generator.)

carljm · 2017-06-01T20:00:54Z

tests/mypy_test.py

+
+
+@pytest.mark.parametrize('filename,case,files', load_test_cases('test-data'))
+def test_yaml_case(filename: Text, case: Text, files: List[dict],


Pytest allows easily writing a plugin to directly parse non-Python files as pytest-native test files containing individual tests, rather than using test parametrization like this. I've done this before, and it works very well and has a number of nice properties, e.g.:

output when running tests has a line of dots per actual test file, not just a single line of dots referring uselessly to this file

test names in failure output can be exactly the name listed in the yaml file, with no repeated test_yaml_case cruft

you can use the obvious pytest path/to/test-basic.yaml natively, to run the tests in test-basic.yaml only, just like you would with a file containing Python tests

Sample code for this approach can be found at https://docs.pytest.org/en/latest/example/nonpython.html

vlasovskikh · 2018-05-22T12:31:16Z

Since there is not a lot of interest in the common test suite at the moment among the developers of type checkers, I'm closing this PR. Maybe someday :)

Prototype of common test suite format and runner for Mypy

13234da

the-knights-who-say-ni added the CLA not signed label May 24, 2017

JelleZijlstra mentioned this pull request May 24, 2017

Tests specifically for tricky, white-lie signatures python/typeshed#1339

Closed

ilevkivskyi removed the CLA not signed label May 28, 2017

the-knights-who-say-ni added the CLA signed label May 28, 2017

ilevkivskyi reviewed May 28, 2017

View reviewed changes

gvanrossum requested changes Jun 1, 2017

View reviewed changes

carljm reviewed Jun 1, 2017

View reviewed changes

vlasovskikh closed this May 22, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Prototype of common test suite format and runner for Mypy #436

Prototype of common test suite format and runner for Mypy #436

vlasovskikh commented May 24, 2017

the-knights-who-say-ni commented May 24, 2017

ilevkivskyi left a comment

ilevkivskyi May 28, 2017

vlasovskikh May 30, 2017 •

edited

Loading

ilevkivskyi May 28, 2017

vlasovskikh May 30, 2017

ilevkivskyi May 31, 2017

vlasovskikh commented May 30, 2017

gvanrossum left a comment

gvanrossum Jun 1, 2017

refi64 Jun 1, 2017

gvanrossum Jun 1, 2017

gvanrossum Jun 1, 2017

carljm Jun 1, 2017

vlasovskikh commented May 22, 2018



		def strip_errors(s: Text) -> Text:
		return re.sub(r'#\sE:.', r'', s)

		return index, message, path


		@pytest.mark.parametrize('filename,case,files', load_test_cases('test-data'))



		@pytest.mark.parametrize('filename,case,files', load_test_cases('test-data'))
		def test_yaml_case(filename: Text, case: Text, files: List[dict],

Prototype of common test suite format and runner for Mypy #436

Prototype of common test suite format and runner for Mypy #436

Conversation

vlasovskikh commented May 24, 2017

the-knights-who-say-ni commented May 24, 2017

ilevkivskyi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vlasovskikh May 30, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vlasovskikh commented May 30, 2017

gvanrossum left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vlasovskikh commented May 22, 2018

vlasovskikh May 30, 2017 •

edited

Loading