Skip to content
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

Move pythoneval and cmdline to pytest #3870

Merged
merged 1 commit into from
Aug 25, 2017
Merged

Conversation

elazarg
Copy link
Contributor

@elazarg elazarg commented Aug 24, 2017

(EDITED)
Another item from #1673, following #3780, #3788, #3861 and #3866.

After this, StubgenPythonSuite is the only data-driven test left in myunit.

As in previous PRs, I have checked that a test may fail by changing tests and running

./runtests.py eval -a -n0 -a -k -a testHello
./runtests.py eval -a -n0 -a -k -a testSimpleCoroutineSleep
./runtests.py cmd -a -n0 -a -k -a testCmdlinePackageContainingSubdir

I am somewhat less confident about this PR, since I am not entirely sure what special considerations are in play here (which aren't there for the proper unit tests).

@ambv @gvanrossum: The splitting of eval tests, introduced in PR #2635, is removed. IIUC this was the intention - please correct me if I'm wrong. (I did not time the difference).

Executing multiple pytest instances does not work well with appveyor.

@elazarg elazarg force-pushed the pytest-eval branch 2 times, most recently from 8aa7f43 to d4a2ac6 Compare August 24, 2017 23:49
@elazarg elazarg changed the title Move pythoneval and cmdline to pytest [WIP] Move pythoneval and cmdline to pytest Aug 25, 2017
'-m', 'mypy.test.testcmdline', *driver.arglist,
coverage=True)
driver.add_python_mod('unit-test %s' % mod, 'mypy.myunit', '-m', mod,
*driver.arglist, coverage=True)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The diff is somewhat confusing - this part belongs to add_myunit()

@elazarg elazarg changed the title [WIP] Move pythoneval and cmdline to pytest Move pythoneval and cmdline to pytest Aug 25, 2017
both suites execute in the same pytest instance as any other.
Copy link
Member

@gvanrossum gvanrossum left a comment

Choose a reason for hiding this comment

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

This all looks fine to me. @ambv if you have something to say, say it soon or I'll just merge this.

Copy link
Contributor

@ambv ambv left a comment

Choose a reason for hiding this comment

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

LGTM.

@gvanrossum
Copy link
Member

Sadly there's a problem with this. When I run the tests as follows:

pytest -n0 mypy/test/testcmdline.py

every test in that file fails. Apparently the pytest-xdist extension which runs tests in parallel does something that keeps the tests from failing. The error is essentially that every subprocess invoking scripts/mypy fails with the following traceback:

Traceback (most recent call last):
  File "/Users/guido/src/mypy/scripts/mypy", line 4, in <module>
    from mypy.main import main
ModuleNotFoundError: No module named 'mypy.main'

This makes sense as the current directory is set to the temporary directory where the test files are written, so the repo root is not on sys.path. I'm guessing that the xdist extension somehow retains the current directory?

The pythoneval tests fail in the same way.

@gvanrossum
Copy link
Member

(And yes, I spent 2 hours trying to debug unrelated code because I didn't realize -n0 always made these tests fail.)

@elazarg
Copy link
Contributor Author

elazarg commented Oct 17, 2017

Ouch. Sorry.

I recall checking for that before opening this PR, and it did not fail on my machine :\ I use -n0 pretty often. But maybe I didn't use it for cmdline specifically; for some reason, exactly one test (testCoberturaParser) fails for me now:

Expected:
  <coverage timestamp="$TIMESTAMP" version="$VERSION" line-rate="0.8000" branch-rate="0"> (diff)
  ...
Actual:
  <coverage timestamp="$TIMESTAMP" version="0.530" line-rate="0.8000" branch-rate="0"> (diff)
  ...

@elazarg
Copy link
Contributor Author

elazarg commented Oct 17, 2017

That test indeed does not fail without -n0.

@gvanrossum
Copy link
Member

I wonder if you have mypy installed? That version looks like it comes from the installed version. If you pip3 uninstall it, do more tests fail?

@elazarg
Copy link
Contributor Author

elazarg commented Oct 17, 2017

Yes they are! Or actually, they didn't fail - until I've commented out the patch :)

@elazarg
Copy link
Contributor Author

elazarg commented Oct 17, 2017

But actually this means my patch is the wrong solution. We need to make sure the folder is the right one, and simply adding it to PYTHONPATH does not guarantee it, I assume.

@gvanrossum
Copy link
Member

Use mypy.api.run()? (See other issue.)

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.

3 participants