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

Config with c code template acd39fc2 #322

Merged
merged 14 commits into from
Oct 10, 2024

Conversation

dataflake
Copy link
Member

This update introduced a lot of changes so I have structured them into several commits, one per tool that was run over the code, and then some that introduced additional fixed. I hope that will help review it all.

@dataflake dataflake self-assigned this Sep 2, 2024
tox.ini Outdated Show resolved Hide resolved
@dataflake
Copy link
Member Author

Locally all tox test runs fail with tracebacks like this unless I apply the workaround of specifying usedevelop = true in the tox testenv. I do not know what this means.

Traceback (most recent call last):
  File "/Users/jens/src/python/Python-3.8.18/lib/python3.8/unittest/__main__.py", line 18, in <module>
    main(module=None)
  File "/Users/jens/src/python/Python-3.8.18/lib/python3.8/unittest/main.py", line 100, in __init__
    self.parseArgs(argv)
  File "/Users/jens/src/python/Python-3.8.18/lib/python3.8/unittest/main.py", line 124, in parseArgs
    self._do_discovery(argv[2:])
  File "/Users/jens/src/python/Python-3.8.18/lib/python3.8/unittest/main.py", line 244, in _do_discovery
    self.createTests(from_discovery=True, Loader=Loader)
  File "/Users/jens/src/python/Python-3.8.18/lib/python3.8/unittest/main.py", line 154, in createTests
    self.test = loader.discover(self.start, self.pattern, self.top)
  File "/Users/jens/src/python/Python-3.8.18/lib/python3.8/unittest/loader.py", line 349, in discover
    tests = list(self._find_tests(start_dir, pattern))
  File "/Users/jens/src/python/Python-3.8.18/lib/python3.8/unittest/loader.py", line 414, in _find_tests
    yield from self._find_tests(full_path, pattern, namespace)
  File "/Users/jens/src/python/Python-3.8.18/lib/python3.8/unittest/loader.py", line 414, in _find_tests
    yield from self._find_tests(full_path, pattern, namespace)
  File "/Users/jens/src/python/Python-3.8.18/lib/python3.8/unittest/loader.py", line 414, in _find_tests
    yield from self._find_tests(full_path, pattern, namespace)
  [Previous line repeated 1 more time]
  File "/Users/jens/src/python/Python-3.8.18/lib/python3.8/unittest/loader.py", line 405, in _find_tests
    tests, should_recurse = self._find_test_path(
  File "/Users/jens/src/python/Python-3.8.18/lib/python3.8/unittest/loader.py", line 458, in _find_test_path
    raise ImportError(
ImportError: 'test_builtins' module incorrectly imported from '/Users/jens/src/zope/zope.interface/.tox/py38/lib/python3.8/site-packages/zope/interface/common/tests'. Expected '/Users/jens/src/zope/zope.interface/src/zope/interface/common/tests'. Is this module globally installed?
py38: exit 1 (0.56 seconds) /Users/jens/src/zope/zope.interface> coverage run -p -m unittest discover -s src pid=21543
  py38: FAIL code 1 (3.09=setup[2.53]+cmd[0.56] seconds)
  evaluation failed :( (3.11 seconds)

@dataflake
Copy link
Member Author

dataflake commented Sep 15, 2024

Correction: What seems to change is the interface class __name__ value?

I have tried a few variation on the unittest invocation. If I change the current coverage run -p -m unittest discover -s src {posargs} to coverage run -p -m unittest discover -s src/zope/interface {posargs} then all tests run, but many %$#^&% doctests that rely on a specific text representation of the interface class will fail because that changes. Here's an example:

======================================================================
FAIL: test_w_None_no_bases_w_class (tests.test_declarations.Test_implementedByFallback)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/jens/src/zope/zope.interface/src/zope/interface/tests/test_declarations.py", line 698, in test_w_None_no_bases_w_class
    self.assertEqual(spec.__name__,
AssertionError: 'tests.test_declarations.Foo' != 'zope.interface.tests.test_declarations.Foo'
- tests.test_declarations.Foo
+ zope.interface.tests.test_declarations.Foo
? +++++++++++++++

@icemac
Copy link
Member

icemac commented Sep 17, 2024

@dataflake If the only failures are the names we could fix this whith a RENormalizer. Do you think this would be enough or are there more tests to fix which neede individual care (like the one you pasted)?

@dataflake
Copy link
Member Author

If I remember correctly there are 15 failing tests and many of them unit tests that all need to be touched individually. What I am more concerned is what the failure actually means. Is it even important that the string representations look exactly like the current unit tests want?

@dataflake
Copy link
Member Author

The PR #325 demonstrates the test changes as well as the unittest/coverage configuration changes required to make the tests themselves run locally. But as you can see from the GHA test run, it breaks the tests here. And locally the coverage test keeps failing because it thinks there's a coverage of just 32.42%.

This package is a complete one-off because the test configuration must ensure that zope.interface is not imported from another location. That's why it doesn't use zope.testrunner.

I don't know what to do about this other than to punt and just put usedevelop = true back into the tox configuration.

@dataflake
Copy link
Member Author

I have given it another try and compared how the GHA test runners (where the tests were green) work differently from the local tox invocation. The main difference is that they force-install the package with the tests extra just before running the tests. So if I change the tox configuration's [testenv] to drop usedevelop = true and then add pip install -U -e .[test] as a command step before invoking the tests then things work again. But then again this might just do what usedevelop = true does behind the scenes?

The GHA actions reveal another issue, on Python 3.13 the tests will reliably fail with a segmentation fault on all platforms. I am guessing that's a separate issue.

@tseaver what do you think?

@davisagli
Copy link
Member

Note: #324 might help with the segfault. I am waiting for @tseaver to take a look at my latest comment there.

@dataflake dataflake requested a review from tseaver October 4, 2024 09:20
@dataflake
Copy link
Member Author

The tests are all green, please re-review.

@dataflake
Copy link
Member Author

The tests have been green since I merged master into this branch, which contained the fixes from #328 and also the reassurance that some Python 3.13 issues were fixed in Python between rc2 and rc3. I have received very little feedback so far and don't think it helps anyone to wait any longer.

@dataflake dataflake merged commit 02c8c61 into master Oct 10, 2024
53 checks passed
@dataflake dataflake deleted the config-with-c-code-template-acd39fc2 branch October 10, 2024 07:14
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.

4 participants