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

Update to the latest c-code template #73

Merged
merged 3 commits into from Sep 13, 2022
Merged

Update to the latest c-code template #73

merged 3 commits into from Sep 13, 2022

Conversation

mgedmin
Copy link
Member

@mgedmin mgedmin commented Sep 10, 2022

Add a regression test for CFLAGS not having -Ofast, which is known to break things. See zopefoundation/meta#155 for reference.

Add a regression test for CFLAGS not having -Ofast, which is known to
break things.  See zopefoundation/meta#155 for
reference.
@icemac
Copy link
Member

icemac commented Sep 12, 2022

Hm, according to coveralls the new test does not run: https://coveralls.io/builds/52359729/source?filename=src%2Fzodbpickle%2Ftests%2Ftest_compile_flags.py
@mgedmin Do you have an idea why this could happen?

@mgedmin
Copy link
Member Author

mgedmin commented Sep 12, 2022

Hm, according to coveralls the new test does not run.
@mgedmin Do you have an idea why this could happen?

Good catch! That would be because tox.ini does

commands =
    # there's test_pickle_2 for Python 2 and test_pickle_3 for Python 3
    # different test files because metaclass syntax differs on Pythons
    # test_pickle picks and returns the appropriate one
    # without this we'd get an import error (actually syntax error) for one
    # of them
    zope-testrunner --test-path=src -m zodbpickle.tests.test_pickle$ {posargs:-vc}

and, for tox -e coverage,

    coverage run -m zope.testrunner --test-path=src -m zodbpickle.tests.test_pickle$ {posargs:-vc}

(And similar things in buildout.cfg and GitHub Actions, most of them by way of custom test commands in .meta.toml)

I'll see what I can do (probably rename test_pickle_{2,3}.py so their file names don't start with test, after all they're included in the test suite explicitly inside test_pickle.py).

Rename test_pickle_{2,3}.py because these are not supposed to be
collected automatically: they use syntax incompatible with Python 3 and
2 respectively.

Get rid of the -m zodbpickle.tests.test_pickle$ hack that was used to
avoid the aforementioned syntax errors, and was also preventing the new
test_compile_flags.py from being executed.

Get rid of unittest.makeSuite() because I hear it's getting deprecated.
@mgedmin
Copy link
Member Author

mgedmin commented Sep 12, 2022

Wiped some caches:

$ gh api --method DELETE -H "Accept: application/vnd.github+json" /repos/zopefoundation/zodbpickle/actions/caches?key=Linux-pip-3.10
$ gh api --method DELETE -H "Accept: application/vnd.github+json" /repos/zopefoundation/zodbpickle/actions/caches?key=Linux-pip-3.11.0-rc.1

Retried the two failing jobs with debug logging. Found cp310 and cp311 wheels in the Linux-pip-pypy-3.7 cache.

../../../.cache/pip/wheels/26/eb/0c/413ef3f3b51eac5046ef623fab8d71f429297aefda870af4f8/zope.interface-5.4.0-cp311-cp311-linux_x86_64.whl
../../../.cache/pip/wheels/21/a9/8b/0bfc5594d8e109d5b25d6b69e0cff14d09d93e3522dcb16d2b/zope.interface-5.4.0-cp310-cp310-linux_x86_64.whl

Wiped it

$ gh api --method DELETE -H "Accept: application/vnd.github+json" /repos/zopefoundation/zodbpickle/actions/caches?key=Linux-pip-pypy-3.7

Retried again.

@mgedmin
Copy link
Member Author

mgedmin commented Sep 12, 2022

Linux-pip-2.7 cache also has things like

../../../.cache/pip/wheels/26/eb/0c/413ef3f3b51eac5046ef623fab8d71f429297aefda870af4f8/zope.interface-5.4.0-cp311-cp311-linux_x86_64.whl
../../../.cache/pip/wheels/21/a9/8b/0bfc5594d8e109d5b25d6b69e0cff14d09d93e3522dcb16d2b/zope.interface-5.4.0-cp310-cp310-linux_x86_64.whl

$ gh api --method DELETE -H "Accept: application/vnd.github+json" /repos/zopefoundation/zodbpickle/actions/caches?key=Linux-pip-2.7

At this point I think every single cache is poisoned with bad cp310 and cp311 wheels, so

$ gh api --method DELETE -H "Accept: application/vnd.github+json" /repos/zopefoundation/zodbpickle/actions/caches?key=Linux-pip-3.5
$ gh api --method DELETE -H "Accept: application/vnd.github+json" /repos/zopefoundation/zodbpickle/actions/caches?key=Linux-pip-3.6
$ gh api --method DELETE -H "Accept: application/vnd.github+json" /repos/zopefoundation/zodbpickle/actions/caches?key=Linux-pip-3.7
$ gh api --method DELETE -H "Accept: application/vnd.github+json" /repos/zopefoundation/zodbpickle/actions/caches?key=Linux-pip-3.8
$ gh api --method DELETE -H "Accept: application/vnd.github+json" /repos/zopefoundation/zodbpickle/actions/caches?key=Linux-pip-3.9
$ gh api --method DELETE -H "Accept: application/vnd.github+json" /repos/zopefoundation/zodbpickle/actions/caches?key=Linux-pip-pypy-2.7

I would love to know how that sort of thing happens.

@mgedmin
Copy link
Member Author

mgedmin commented Sep 12, 2022

Another cache existed: Linux-pip-3.11.0-beta.3.

$ gh api --method DELETE -H "Accept: application/vnd.github+json" /repos/zopefoundation/zodbpickle/actions/caches?key=Linux-pip-3.11.0-beta.3

Are there more?

$ gh api -H "Accept: application/vnd.github+json" /repos/zopefoundation/zodbpickle/actions/caches|jq -r .actions_caches[].key|sort

None of them start with "Linux-pip-"

@mgedmin
Copy link
Member Author

mgedmin commented Sep 12, 2022

I would love to know how that sort of thing happens.

Theory: when you add a new Python version to the build matrix, there's no Linux-pip-3.10 cache, so it uses a compatible Linux-pip-2.7 or something, builds a bunch of new wheels, and then instead of creating Linux-pip-3.10, updates Linux-pip-2.7.

Nah, can't be right, that would mean the new cache for Linux-pip-3.10 wouldn't ever get created.

Could it be that the cache action both updates the old compatible cache and creates the new one? Would be strange to me...

Copy link
Member

@icemac icemac left a comment

Choose a reason for hiding this comment

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

LGTM.

Thank you for making the test setup way easier!

@mgedmin mgedmin merged commit d45fce6 into master Sep 13, 2022
@mgedmin mgedmin deleted the config-with-c-code branch September 13, 2022 06:48
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.

None yet

2 participants