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

Automating PyPI Wheel Pytest #440

Merged
merged 34 commits into from Aug 8, 2019

Conversation

@tommyli3318
Copy link
Member

commented Jun 23, 2019

What does this PR do?

Creates a script automating PyPI Wheel tests that can be extended to test multiple PyPI packages.
Normal pytest results of each package will be compared to its nuitka-built .whl counterpart.
Testing is done to ensure that nuitka is building the wheel correctly for each PyPI package. If the pytests pass/fail in the same way, that means Nuitka built the wheel properly. Else if the tests differ, then something is wrong. Virtualenv is used to create a clean environment with no outside pollution.

Why was it initiated? Any relevant Issues?

GSoC2019

PR Checklist

  • Correct base branch selected? develop for new features and bug fixes too. If it's
    part of a hotfix, it will be moved to master during it.
  • All tests still pass. Check the developer manual about Running the Tests. There
    are Travis tests that cover the most important things however, and you are
    welcome to rely on those, but they might not cover enough.
  • Ideally new features or fixed regressions ought to be covered via new tests.
  • Ideally new or changed features have documentation updates.
@tommyli3318

This comment has been minimized.

Copy link
Member Author

commented Jun 30, 2019

@kayhayen Just updated with a new commit, the code now gets stdout and stderr from both the uncompiled pytest and wheel pytest. I left some comments, please review

tests/PyPI-pytest/run_all.py Outdated Show resolved Hide resolved
tests/PyPI-pytest/run_all.py Outdated Show resolved Hide resolved

@kayhayen kayhayen added this to In progress in Planning Jul 1, 2019

@kayhayen kayhayen moved this from In progress to Review in progress in Planning Jul 1, 2019

@tommyli3318 tommyli3318 force-pushed the tommyli3318:develop-pypi branch 2 times, most recently from 6ae27c6 to 1c25c27 Jul 7, 2019

@tommyli3318

This comment has been minimized.

Copy link
Member Author

commented Jul 7, 2019

@kayhayen the bdist_wheel part works properly now. However, I ran into something weird when changing up the runCommand method to use subprocess and return stderr,stdout. Refer to the comments above.

Edit: I figured it out, I had to pip uninstall nuitka. The file was importing from the downloaded package source code instead of the source code that I had edited.

@tommyli3318

This comment has been minimized.

Copy link
Member Author

commented Jul 8, 2019

@kayhayen Updated again, made changes to the Virtualenv class to include a new method runCommandWithOutput that uses subprocess to return stdout and stderr.
Now everything works, with outputs saved into strings uncompiled_stdout, uncompiled_stderr, compiled_stdout, and compiled_stderr.
removeDirectory still doesn't work, even after disabling windows defender and adding the range loop to retry. Issuing ls -a, I can see that there is a .git directory that is left over in each package, I think this may be causing permission issues. Do you know of a way around this? I've tried using shutil.rmtree, no luck on that either.

I will get to compareOutput soon.

@tommyli3318

This comment has been minimized.

Copy link
Member Author

commented Jul 8, 2019

Okay, compareOutput now works too.
For the urllib3 package, the output is now

Creating virtualenv for quick test:
--- python (stdout)
+++ nuitka (stdout)

@@ -12,7 +12,7 @@

 test\test_exceptions.py .............                                    [  9%]
 test\test_fields.py ...............                                      [ 11%]
 test\test_filepost.py ...........                                        [ 11%]
-test\test_no_ssl.py ..                                                   [ 12%]
+test\test_no_ssl.py .F                                                   [ 12%]
 test\test_poolmanager.py .........................                       [ 14%]
 test\test_proxymanager.py ...                                            [ 14%]
 test\test_queue_monkeypatch.py .                                         [ 14%]
@@ -47,5 +47,29 @@

 test\with_dummyserver\test_socketlevel.py .............................. [ 98%]
 ......................                                                   [100%]

-=========== 839 passed, 456 skipped, 113 warnings in 49.27 seconds ============
+================================== FAILURES ===================================
+__________________ TestImportWithoutSSL.test_import_urllib3 ___________________

+self = <test.test_no_ssl.TestImportWithoutSSL object at 0xxxxxxxxx>
+
+    def test_import_urllib3(self):
+>       import urllib3  # noqa: F401
+
+test\test_no_ssl.py:86:
+_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
+<frozen importlib._bootstrap>:983: in _find_and_load
+    ???
+<frozen importlib._bootstrap>:967: in _find_and_load_unlocked
+    ???
+<frozen importlib._bootstrap>:668: in _load_unlocked
+    ???
+_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
+
+spec = ModuleSpec(name='urllib3', loader=<class 'nuitka_module_loader'>)
+
+>   ???
+E   KeyError: 'urllib3'
+
+<frozen importlib._bootstrap>:638: KeyError
+====== 1 failed, 838 passed, 456 skipped, 113 warnings in 47.74 seconds =======
+
---------------------------------------------------------------------------------
exit_stdout: 1 exit_stderr: 0
Error, outputs differed.
@tommyli3318

This comment has been minimized.

Copy link
Member Author

commented Jul 8, 2019

Currently, the entire test only uses one virtualenv. Would it be a better idea to use two separate ones for uncompiled and compiled?

@kayhayen kayhayen force-pushed the Nuitka:develop branch from 113e470 to f3367de Jul 8, 2019

@tommyli3318 tommyli3318 force-pushed the tommyli3318:develop-pypi branch from 8628475 to 9b1d6c8 Jul 8, 2019

@tommyli3318 tommyli3318 closed this Jul 8, 2019

@tommyli3318 tommyli3318 force-pushed the tommyli3318:develop-pypi branch from 9b1d6c8 to f3367de Jul 8, 2019

Planning automation moved this from Review in progress to Done Jul 8, 2019

@tommyli3318 tommyli3318 reopened this Jul 8, 2019

Planning automation moved this from Done to In progress Jul 8, 2019

@tommyli3318

This comment has been minimized.

Copy link
Member Author

commented Jul 11, 2019

Added removeTimeDiff function to ignore the time differences of pytest

@tommyli3318 tommyli3318 force-pushed the tommyli3318:develop-pypi branch 2 times, most recently from ec2c65d to 6b720c7 Jul 17, 2019

@tommyli3318

This comment has been minimized.

Copy link
Member Author

commented Jul 18, 2019

I've color-coded the result outputs for each package. Now also prints a colored summary of all tests at the end. Will get to caching soon

@kayhayen

This comment has been minimized.

Copy link
Member

commented Jul 19, 2019

@tommyli3318 The caching refers to git clone mostly, make a test if it exists, then git fetch, git reset --hard origin/branch_name otherwise fresh clone.

@tommyli3318

This comment has been minimized.

Copy link
Member Author

commented Jul 19, 2019

@kayhayen got it, caching is finished :)

@kayhayen
Copy link
Member

left a comment

Good job, sorry for the many comments. :-)

nuitka/tools/testing/Virtualenv.py Outdated Show resolved Hide resolved
nuitka/utils/FileOperations.py Outdated Show resolved Hide resolved
tests/PyPI-pytest/run_all.py Outdated Show resolved Hide resolved
tests/PyPI-pytest/run_all.py Outdated Show resolved Hide resolved
tests/PyPI-pytest/run_all.py Outdated Show resolved Hide resolved
tests/PyPI-pytest/run_all.py Outdated Show resolved Hide resolved
tests/PyPI-pytest/run_all.py Outdated Show resolved Hide resolved
tests/PyPI-pytest/run_all.py Outdated Show resolved Hide resolved
tests/PyPI-pytest/run_all.py Outdated Show resolved Hide resolved

Planning automation moved this from In progress to Review in progress Jul 20, 2019

@tommyli3318 tommyli3318 force-pushed the tommyli3318:develop-pypi branch from d45d32b to 8d95e82 Jul 30, 2019

@kayhayen

This comment has been minimized.

Copy link
Member

commented Aug 1, 2019

@tommyli3318 Can you fix the Travis issues? I am considering the merge or use this, but it should also work with Python2 for that.

tommyli3318 added 5 commits Aug 2, 2019
@tommyli3318

This comment has been minimized.

Copy link
Member Author

commented Aug 3, 2019

@kayhayen Fixed Travis issues :)

nuitka/Tracing.py Outdated Show resolved Hide resolved
nuitka/tools/testing/OutputComparison.py Outdated Show resolved Hide resolved
tests/PyPI-pytest/run_all.py Outdated Show resolved Hide resolved
tommyli3318 added 2 commits Aug 4, 2019

@tommyli3318 tommyli3318 closed this Aug 4, 2019

Planning automation moved this from Review in progress to Done Aug 4, 2019

@tommyli3318 tommyli3318 reopened this Aug 4, 2019

Planning automation moved this from Done to In progress Aug 4, 2019

@tommyli3318

This comment has been minimized.

Copy link
Member Author

commented Aug 4, 2019

Updated with autoformatting @kayhayen

tests/PyPI-pytest/run_all.py Outdated Show resolved Hide resolved
tests/PyPI-pytest/run_all.py Show resolved Hide resolved
nuitka/tools/testing/OutputComparison.py Outdated Show resolved Hide resolved

Planning automation moved this from In progress to Review in progress Aug 4, 2019

@kayhayen

This comment has been minimized.

Copy link
Member

commented Aug 4, 2019

I guess, we can merge this, and let me attempt to run this test. For nightshift CI, one of the concerns would be that it uses the network for git, and as such is vulnerable, but what can we do otherwise, this will have to be that way.

The changes requested are minor, and I will merge after you do them, with more to come. I believe the Jinja2 needs the environment variables hack, which we should focus on next.

@tommyli3318

This comment has been minimized.

Copy link
Member Author

commented Aug 4, 2019

Can you explain the Jinja2 environment variables hack? Why is the ModuleNotFoundError even occurring in the first place?
Package rsa recently switched to Poetry in this commit and removed their setup.py, what should we do in that case?
Package pytz also recently updated their repo, they moved their setup.py into their src folder and I can no longer run setup.py bdist_* due to an AssertionError. Looks like it is their problem. I've submitted an issue to them. I manually suppressed the AssertionError by changing their setup.py and it seems python setup.py bdist_nuitka now successfully builds the compiled wheel so that's good.

tommyli3318 added 2 commits Aug 4, 2019
@tommyli3318

This comment has been minimized.

Copy link
Member Author

commented Aug 5, 2019

@kayhayen
Ok, should be ready for merge now.

@kayhayen
Copy link
Member

left a comment

Good job, now go for the .py_modules stuff if you can, and add more packages nearby.

@kayhayen kayhayen merged commit 8c2438a into Nuitka:develop Aug 8, 2019

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

Planning automation moved this from Review in progress to Done Aug 8, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
2 participants
You can’t perform that action at this time.