Improve TestUnpackArchives to also test the contents of the extracted files

[cjerdonek]

In the process of reviewing PR #5848, I noticed that TestUnpackArchives doesn't actually test if the file contents are extracted correctly when unpacking. So this PR adds that to the existing tests.

[cjerdonek]

Hmm, @pradyunsg, do you know why this PR might be failing on unrelated tests for the following two Windows environments on AppVeyor?

• Environment: PYTHON=C:\Python27, RUN_INTEGRATION_TESTS=True
• Environment: PYTHON=C:\Python36-x64, RUN_INTEGRATION_TESTS=True

I'm a little mystified and thought you might have more experience here.

[pfmoore]

I got that occasionally on the PEP 517 work. It seemed to disappear of its own accord, though, so I never worked out what the problem was. I suspect some sort of "path nested too deeply" or "path too long" issue, maybe? We do have some pretty deep nesting in the tests.

[cjerdonek]

@pfmoore Thanks for the thought. It could certainly be something like that. The odd thing is that I think earlier versions of this PR only added code paths that weren't executed on Windows, so I'm not sure how it could trigger different behavior with respect to paths.

Also, there are lines like this in the logs.

Script result: pip install --user INITools==0.1 --no-binary=:all:
return code: 1
-- stderr: --------------------
Could not install packages due to an EnvironmentError: [Error 3] The system cannot find the path specified: 't:\pytest-of-unknown\pytest-0\popen-gw2\test_install_user_conflict_in_1\workspace\tmp\pip-uninstall-k42xdb\pytest-of-unknown\pytest-0\popen-gw2\test_install_user_conflict_in_1\workspace\venv\user\python27\site-packages\initools-0.3-py2.7.egg-info\dependency_links.txt'

Notice that this substring occurs twice in the path. Maybe that's related to the issue, but I'm not sure what could be causing it:

\\pytest-of-unknown\\pytest-0\\popen-gw2\\test_install_user_conflict_in_1\\workspace\\

[pfmoore]

Yes, that's what I saw with my failures too. As I say, I'm pretty sure when it happened to me I made a change that could have had no impact on that problem, and the issue went away in the next test run anyway.

It looks like something might be misusing one of our test fixtures (that path looks like something generated from the pytest tmpdir fixture) but we have so many layers of patching and customisation going on that I don't even know where to begin debugging it 😢

[benoit-pierre]

Starting to hit that too, might be the new pytest release, looking into it.

[benoit-pierre]

Definitively due to the new pytest release, still not sure why (might be a combination of issues: longer paths, tmpdir fixture now cleaning-up after itself). I suggest pinning pytest to 3.8.2 for now: #5896.

[benoit-pierre]

So this is due to 2 changes with the new pytest release:

• first, the workaround used to avoid paths length limitation on Windows by setting TMP/TEMP to a temporary mapped drive does not work because the new implementation resolve the paths.
• second, even after removing the call to resolve, the change in base temporary directory name (from t:\pytest-0\popen-gw0 to t:\pytest-of-unknown\pytest-0\popen-gw0) is enough to push some tests over the limit.

[cjerdonek]

Thanks for investigating, @benoit-pierre! Is there any discussion on the pytest tracker or release notes explaining why the change was made? Also, for the second issue, why weren't the tests always failing, then? Is there some indeterminism in the path that can affect the path's length?

[benoit-pierre]

It is deterministic. The maximum path length used by a test is determined by its name, using pytest-xdist, and things like using pip uninstall (which will create entries in a temporary directory under the workspace used by the script fixture: for staging possible rollbacks, the reason for what looks like doubled paths).

[cjerdonek]

Oh, I see. I misunderstood. So pytest changed their base directory name in the new release..

[benoit-pierre]

That, and our trick with subst T: ... does not work anymore (with Python 3), so the tests are using C:\Users\appveyor\AppData\Local\Temp\1\pytest-of-unknown\pytest-0\popen-gw1/... instead of t:\pytest-of-unknown\pytest-0\popen-gw1, hence the higher number of failed tests.

[pfmoore]

One minor point, but otherwise this looks good.

[pfmoore]

Maybe explicitly set an encoding here? Or read (and make the expected value) bytes? The expected data is all ASCII, so it shouldn't in theory matter, but explicit is better than implicit and all that...

[cjerdonek]

Thanks, @pfmoore. Good point. Originally I thought about making it bytes, but then I saw the tests were passing anyways, so I didn't think I needed to. :) I'll go ahead and change it because I agree the definiteness is better.

[pradyunsg]

@cjerdonek I've been AWOL for a bit. There's been a lot of stuff happening lately for me.

Thanks for some awesome investigative work @benoit-pierre!

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.