Run selftest directly

[radarhere]

Suggestions for python-pillow#9030

• I don't see why .map_metadata_keys(metadata("Pillow")) is needed in Tests/test_pyroma.py
• I've removed references to sys.platform being "android"
• .gitignore already ignores the downloaded zips
• I was able to move the checks back into the Tests directory, by using feat: support multiple commands on iOS pypa/cibuildwheel#2432

[freakboy3742]

Suggestions for python-pillow#9030

• I don't see why .map_metadata_keys(metadata("Pillow")) is needed in Tests/test_pyroma.py

It's needed because on iOS, the tests aren't running in the source folder, so "." isn't a useful reference for reading metadata. You need to go to the package registration.

Interestingly, at some point since I originally wrote this patch, the pyroma test has become disabled on iOS... it's definitely installed (I just confirmed this), but it's not being imported for some reason. I'll need to investigate.

• I've removed references to sys.platform being "android"

👍 That was proactive, as an Android patch should be on it's way shortly.

• .gitignore already ignores the downloaded zips

That looks like a merge conflict - it's been merged in a second time accidentally.

• I was able to move the checks back into the Tests directory, by using feat: support multiple commands on iOS pypa/cibuildwheel#24321

Yes, this is possible; but at the cost of a significantly longer test time. Each test run requires a standalone testbed, which means a complete build, simulator start, and launch. This adds at least 4 minutes (and quite a bit of log traffic) to the test runtime (~8 minutes to ~12 minutes).

[radarhere]

• I was able to move the checks back into the Tests directory, by using feat: support multiple commands on iOS pypa/cibuildwheel#24321

Yes, this is possible; but at the cost of a significantly longer test time. Each test run requires a standalone testbed, which means a complete build, simulator start, and launch. This adds at least 4 minutes (and quite a bit of log traffic) to the test runtime (~8 minutes to ~12 minutes).

I don't think that an extra 4 minutes is a concern - the manylinux2014 and musllinux x86_64 job takes 1 hour and 6 minutes.

[radarhere]

Interestingly, at some point since I originally wrote this patch, the pyroma test has become disabled on iOS... it's definitely installed (I just confirmed this), but it's not being imported for some reason. I'll need to investigate.

If it helps, it looks to me like distutils can't be found, with pyroma tries to import.

[freakboy3742]

I don't think that an extra 4 minutes is a concern - the manylinux2014 and musllinux x86_64 job takes 1 hour and 6 minutes.

Ok... but for the sake of some directory reorganization, local and CI builds are faster, which means less resource contention on CI machines (which, in the case of Apple Silicon, are much more constrained than the Linux machines).

Ultimately, I'm a contributor, so I'll stand by whatever decision the core team makes; but as a contributor, knowing that tests will be 50% slower because of a directory organization choice seems... a odd choice.

Interestingly, at some point since I originally wrote this patch, the pyroma test has become disabled on iOS... it's definitely installed (I just confirmed this), but it's not being imported for some reason. I'll need to investigate.

If it helps, it looks to me like distutils can't be found, with pyroma tries to import.

You've posted that just as I was writing up my discovery of the same.

The good news is that I think I know how to fix this; the bad news is that the fix needs to be at the level of the iOS support package.

In the meantime, you can manually verify that the metadata change is needed by adding __import__('_distutils_hack').add_shim() just before the pyroma import.

I don't know how essential you consider the pyroma test as a merge blocker. The test is currently being skipped, and if the iOS support PR were to be merged, it would start passing as soon as the fix I have in mind lands in a cibuildwheel version.

[radarhere]

I don't know how essential you consider the pyroma test as a merge blocker.

Not essential. See python-pillow#8562

Ok... but for the sake of some directory reorganization, local and CI builds are faster, which means less resource contention on CI machines (which, in the case of Apple Silicon, are much more constrained than the Linux machines).

Ultimately, I'm a contributor, so I'll stand by whatever decision the core team makes; but as a contributor, knowing that tests will be 50% slower because of a directory organization choice seems... a odd choice.

If you're concerned about local builds, then ok.

But something else to consider is that the PR currently runs python -m pytest -vv selftest.py. selftest doesn't function solely as a pytest script - run by itself it prints helpful debugging information about dependency versions and so on.

[freakboy3742]

But something else to consider is that the PR currently runs python -m pytest -vv selftest.py. selftest doesn't function solely as a pytest script - run by itself it prints helpful debugging information about dependency versions and so on.

In which case, a 2 step (which looks like what you've just pushed) may be the best option. Not sure what's going on the build failures on the most recent CI pass, though... if you don't beat me to it, I'll investigate in the morning.

Also - python/cpython#135967 is up, which will fix the pyroma issue (once it propegates down to cibuildwheel).

[radarhere]

The recent failures are because python-pillow#8858 was merged

[freakboy3742]

Ah - a new libavif dependency landed a couple of hours ago.

Testing locally on the old set multibuild dependencies is working for me; so I'll merge this into my own PR and address the libavif issue on the main PR.