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

Make tests pass on conda builds #151

Merged
merged 6 commits into from
Apr 23, 2023

Conversation

AlexWaygood
Copy link
Member

The test suite doesn't currently pass on conda builds of Python. This is somewhat annoying for me personally, as I generally use conda when I want to use an older version of Python, and testing on older versions of Python is pretty important for typing_extensions :)

The main reason why the tests fail on conda is that conda debundles the test package from the standard library. This PR just vendors test.ann_module, test.ann_module2 and test.ann_module3 from the CPython main branch (and renames them as _ann_module, _ann_module2, and _ann_module3). This is probably a good thing to do anyway; the Lib/test directory is sort of an implementation detail of CPython's internal test suite.

There was also an Any test that was failing on Python <3.11 due to the module of a nested class being __main__ instead of typing_extensions. I'm not sure why this is different on conda, but it doesn't seem that big a deal to just change the test here.

Copy link
Collaborator

@hauntsaninja hauntsaninja left a comment

Choose a reason for hiding this comment

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

Hmm, will these get distributed to users? Maybe it would be cleaner to inline into test_typing_extensions and exec?

@AlexWaygood
Copy link
Member Author

AlexWaygood commented Apr 22, 2023

Maybe it would be cleaner to inline into test_typing_extensions and exec?

Could do! We already have src/_typed_dict_test_helper.py on the main branch, though I guess that also isn't ideal now that you mention it.

What about creating an src/test_data directory for putting these files in? That would have the (small, but nice) advantage that we'd still get syntax highlighting on GitHub/IDEs.

@JelleZijlstra
Copy link
Member

We definitely should make it so these files don't get put into people's site-packages on installation. I think I prefer inlining the files into test_typing_extensions so we don't have to deal with the complexity of distributing the additional files.

@AlexWaygood AlexWaygood marked this pull request as draft April 23, 2023 03:13
@AlexWaygood AlexWaygood marked this pull request as ready for review April 23, 2023 06:11
Comment on lines +1059 to +1068
@classmethod
def setUpClass(cls):
with tempfile.TemporaryDirectory() as tempdir:
sys.path.append(tempdir)
Path(tempdir, "ann_module.py").write_text(ANN_MODULE_SOURCE)
Path(tempdir, "ann_module2.py").write_text(ANN_MODULE_2_SOURCE)
Path(tempdir, "ann_module3.py").write_text(ANN_MODULE_3_SOURCE)
cls.ann_module = importlib.import_module("ann_module")
cls.ann_module2 = importlib.import_module("ann_module2")
cls.ann_module3 = importlib.import_module("ann_module3")
Copy link
Member Author

Choose a reason for hiding this comment

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

There may be a simpler way of doing this, but I couldn't immediately figure it out

@JelleZijlstra JelleZijlstra merged commit 41a8288 into python:main Apr 23, 2023
@AlexWaygood AlexWaygood deleted the vendor-annmodules branch April 23, 2023 21:59
ShaneMurphy2 pushed a commit to ShaneMurphy2/typing_extensions that referenced this pull request May 17, 2023
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.

3 participants