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.

Sign up for GitHub

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

Jump to bottom

Fix Tox to use Poetry hermetically #531

Merged
merged 5 commits into from
Oct 17, 2023
Merged

Fix Tox to use Poetry hermetically #531

merged 5 commits into from
Oct 17, 2023

Conversation

JoshuaLeivers
Copy link
Contributor

With the existing setup, Tox could sometimes end up using the Poetry virtualenv for all test runs, rather than isolated environments. This meant that only the developer's Python version would be used, defeating the point of the Tox setup. Using tox-poetry-installer, the Tox runs can install the Poetry dependencies hermetically.

@JoshuaLeivers
Copy link
Contributor Author

It looks like the Python version doesn't need to change as I mentioned in the issue, as long as tox-poetry-installer is listed as a dependency in tox.ini specifically, due to how Tox deals with setting up environments. As long as the version you are using to operate Poetry/Tox itself when running it isn't <=3.7, it should work fine.

@JoshuaLeivers
Copy link
Contributor Author

This change fixes #526 . Would it be possible to get this reviewed?

@JoshuaLeivers JoshuaLeivers changed the title Fix Tox to use Poetry hermetically (#526) Fix Tox to use Poetry hermetically Sep 27, 2023
@JoshuaLeivers JoshuaLeivers mentioned this pull request Oct 12, 2023
Closed
@cetanu
Copy link
Collaborator

cetanu commented Oct 16, 2023

Looking now, thanks btw

cetanu
cetanu previously approved these changes Oct 16, 2023
View reviewed changes
@cetanu
Copy link
Collaborator

cetanu commented Oct 16, 2023

Looks like there are some conflicts and I'm not able to sort them out

@JoshuaLeivers
Copy link
Contributor Author

Sorry about the force-pushes - I've just rebased this onto master and fixed the conflicts.

@JoshuaLeivers
Copy link
Contributor Author

Did you want me to rebase again, or did you want to do it once any PRs have been merged that were planned? I've set it to allow edits by maintainers, but I'm not sure if that allows you to do that or not.

@Gobot1234
Copy link
Collaborator

Your PR should be the last thing to touch the deps for a while so please could you rebase again.

@JoshuaLeivers
Copy link
Contributor Author

Rebased.

@JoshuaLeivers
Copy link
Contributor Author

Ah, will quickly have to re-sign the commits again

@JoshuaLeivers
Fix Tox configuration to use Poetry hermetically
527048d 
With the existing setup, Tox could sometimes end up using the
Poetry virtualenv for all test runs.
@JoshuaLeivers
Split dev dependencies into test and dev
e6a304b 
This split should mean that Tox only installs the dependencies that
are actually necessary for testing, rather than all of the `dev`
dependencies.
@JoshuaLeivers
Move tox-poetry-installer dependency into tox.ini
92f4538 
This should mean that the project can continue to support Python
3.7, due to how Tox handles its own direct dependencies.
@JoshuaLeivers
Change Tox environments to be 3.7, 3.8, 3.11
4fbe196 
This covers the earliest supported version, the earliest non-EOL
supported version, and the latest supported version.
Gobot1234
Gobot1234 previously approved these changes Oct 16, 2023
View reviewed changes
@Gobot1234
Copy link
Collaborator

I'm assuming it's not possible to use the old tox_ini_legacy key still right?

@JoshuaLeivers
Copy link
Contributor Author

JoshuaLeivers commented Oct 16, 2023 •
edited
Loading

I believe can do that if you want, though having a separate tox.ini means you can use the pre-commit hook to format it and it's a bit easier to deal with in text editors/IDEs. I haven't tested if the legacy key works with tox-poetry-installer though, so I'm not 100% sure if it does work or not, and couldn't find any mention online of the combination.

@JoshuaLeivers
Copy link
Contributor Author

Did you want me to try and change the config back to being within legacy_tox_ini, or am I good to leave it as-is?

@Gobot1234
Copy link
Collaborator

If you wouldn't mind giving it a go, that'd be nice. I'd like to keep the number of config files down

@JoshuaLeivers
Copy link
Contributor Author

It seems to work fine (though isn't supported by the tox-ini-fmt pre-commit hook, so I'll have to remove that). Should I keep the default testing environments as they are, or would it be worth possibly replacing Python 3.11 with Python 3.12? For that matter, is 3.12 planned to be enabled in the CI pipeline?

@JoshuaLeivers
Move Tox config back into pyproject.toml
3e9026f 
The formatting `pre-commit` hook doesn't support this, so it has
also been removed.
@JoshuaLeivers
Copy link
Contributor Author

JoshuaLeivers commented Oct 17, 2023 •
edited
Loading

Linking an open issue about the pre-commit hook's support for this: tox-dev/tox-ini-fmt#128

@Gobot1234 Gobot1234 merged commit 24db532 into danielgtaylor:master Oct 17, 2023
17 checks passed
@Gobot1234
Copy link
Collaborator

Thanks

@JoshuaLeivers JoshuaLeivers deleted the tox-fix branch October 17, 2023 16:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Gobot1234 Gobot1234 Gobot1234 approved these changes

@cetanu cetanu cetanu left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@JoshuaLeivers @cetanu @Gobot1234