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

Actually run the tests in python 3.12 and 3.13 and remove snapshottest dependency #1572

Merged
merged 5 commits into from
Aug 8, 2024

Conversation

PoByBolek
Copy link
Contributor

@PoByBolek PoByBolek commented Jul 16, 2024

I noticed that 614449e (PR #1550) introduced an invalid tox environment: py12 instead of py312. This mistake was later repeated in 6834385 (PR #1561): py13 instead of py313.

On my machine, tox list outputs:

default environments:
py38       -> [no description]
py39       -> [no description]
py310      -> [no description]
py311      -> [no description]
py312      -> [no description]
py313      -> [no description]
mypy       -> [no description]
pre-commit -> [no description]

additional environments:
py12       -> [no description]
py13       -> [no description]

And tox run doesn't run any tests in the py312 and py313 environments. This is also true in the Github workflows.

For example, in this "successful" run in the py312 environment from 2024-07-01 tox only runs pip install .[test] and nothing else.
grafik

This looks very different from this run in the py311 environment, which also runs pytest:
grafik


This PR actually runs the tests in the py312 and py313 and drops the snapshottest dependency (which was only used in the example tests) so that the tests also work in Python 3.12 and above. This is necessary because snapshottest (which appears dead) is using Python's imp module, which has been deprected since 3.4 and was removed in 3.12.

@PoByBolek
Copy link
Contributor Author

From what I can tell there are at least two open pull requests in snapshottest that could fix our test failures in py312 and py313:

so that the tests pass in 3.12 and 3.13 again
because the snapshottest package doesn't work on Python 3.12 and above
@PoByBolek PoByBolek changed the title Actually run the tests in python 3.12 and 3.13 Actually run the tests in python 3.12 and 3.13 and remove snapshottest dependency Jul 16, 2024
@PoByBolek
Copy link
Contributor Author

I also dropped the snapshottest dependency and simply moved the expectactions from the snap_*.py files to the corresponding test cases in the example tests. So the tests should now work across all environments again 😄

@dulmandakh
Copy link
Contributor

I did notice that tests on Python 3.12 are not working as expected, but hadn't chance to dig into it. Thank you 🙏

I agree that Graphene needs to drop snapshottest as it's failing to work on Python 3.12 and above. I'll try to take time and review your changes.

Copy link
Contributor

@dulmandakh dulmandakh left a comment

Choose a reason for hiding this comment

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

LGTM

@PoByBolek
Copy link
Contributor Author

@erikwrede ping?

erikwrede
erikwrede previously approved these changes Aug 7, 2024
Copy link
Member

@erikwrede erikwrede left a comment

Choose a reason for hiding this comment

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

Sorry for the delay, currently really busy in my day-to-day.

Thanks for picking this up and pinging me!

@erikwrede
Copy link
Member

Seems like we have some test failures in 3.13. Will investigate later.

@PoByBolek
Copy link
Contributor Author

looks like the json module raises a new error message for trailing commas since python/cpython@cfa25fe (Python 3.13.0a3)

@erikwrede erikwrede merged commit 48678af into graphql-python:master Aug 8, 2024
7 checks passed
@erikwrede
Copy link
Member

Thank you!

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