Skip to content

Add fail_on_changes to toxgen #4051

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom
Closed
sentrivana opened this issue Feb 13, 2025 · 0 comments · Fixed by #4072
Closed

Add fail_on_changes to toxgen #4051

sentrivana opened this issue Feb 13, 2025 · 0 comments · Fixed by #4072
Labels
Component: Tests Dealing with tests

Comments

@sentrivana
Copy link
Contributor

sentrivana commented Feb 13, 2025 •
edited
Loading

  • if someone makes changes to tox.ini directly without having them reflected in tox.jinja or the script, CI should fail so that we don't end up having a desynced state
  • regenerate tox.ini and compare hashes, see if there are differences between the committed and the generated tox.ini
    • this has to be a bit smarter and take into account when the script ran last -- it might happen that re-running the script introduces unrelated changes (because e.g. a new version of a framework was released in the meantime) and thus fails the check even if there were no changes made to tox.ini in the PR
    • save the timestamp of when tox.ini was last generated and committed, read this when regenerating for this check, and run the script ignoring any releases that came out after the timestamp
@sentrivana sentrivana added the Component: Tests Dealing with tests label Feb 13, 2025
@sentrivana sentrivana mentioned this issue Feb 18, 2025
Merged
sentrivana added a commit that referenced this issue Feb 19, 2025
@sentrivana
tests: Add fail_on_changes to toxgen (#4072)
67f0491 
Add `fail_on_changes` to toxgen. The idea is that the script will now
have two modes:

- **Normal mode** (when `fail_on_changes` is `False`) that is used to
actually generate the `tox.ini` file. This [will
be](#4050) run in a
cron job in CI and create a PR with the updated test setup.
- The newly added **fail-on-changes mode** (when `fail_on_changes` is
`True`) that is used to detect manual changes to one of the affected
files without updating the rest (e.g. making a manual change to
`tox.ini` without updating the `tox.jinja` template). This will be run
in CI similar to the `fail_on_changes` check of `split-tox-gh-actions`.

The problem with detecting manual changes is that if we just reran the
script on each PR, chances are it would pull in new releases that are
not part of the `tox.ini` on master, making the file look different from
what was committed as if it had unrelated manual changes.

To counteract this, we now store the timestamp when the file was last
generated in `tox.ini`. We use this in fail-on-changes mode to filter
out releases that popped up after the file was last generated. This way,
the package versions should be the same and if there is anything
different in `tox.ini`, it's likely to be the manual changes that we
want to detect.

Closes #4051
# for free to join this conversation on GitHub. Already have an account? # to comment
Assignees
No one assigned
Labels
Component: Tests Dealing with tests
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

tests: Add fail_on_changes to toxgen getsentry/sentry-python
1 participant
@sentrivana