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

build(test): skip tests on version and changelog changes #1642

Closed
wants to merge 27 commits into from

Conversation

ryannikolaidis
Copy link
Contributor

@ryannikolaidis ryannikolaidis commented Oct 4, 2023

Closes #1549

Currently, when any change is made to an open PR all CI jobs will run. This extends CI time that is unneeded when the change is only a changelog and/or version file update. It also means more time waiting for a job to show as successful / read to add to the merge queue. We don't need to run unit/ingest tests for a changelog update.

Instead we should have a check in the CI pipeline that only runs version checks and changelog checks when the changes are limited to the changelog or version file.

This is deceptively complicated by branch protection (GH really doesn't want you to do this). The issue is that path filtering will result in job skips and branch protection does not look back to previous runs to validate success (on skip).

To solve this PR leverages skip-duplicate-actions. This allows us to skip only when both the file condition (version or changelog file changes) is met as well as validation there is a previous successful run. This seems simple, however it gets more complicated when there are test matrices, because these are not reported as a single job success/failure. The issue is that while skipping a job counts as a success for branch protection, if a job with a matrix strategy skips, the individual per-python versions won't be hit at all (i.e. they won't count as a skip afa branch protection is concerned). Instead we need a new job for each required test matrix job that fails/succeeds depending on the aggregate results of the matrix.

NOTE: we will need to update branch protection to use those _result jobs instead of directly relying on each matrix strategy.
NOTE: skips are counted as "passes" in the cases of a version bump.
NOTE: breaks check-version into a separate job so that we can run always run that, but make the other checks conditional.

Testing

A failure on code changes (change unit test to fail only for v3.8):
https://github.com/Unstructured-IO/unstructured/actions/runs/6450080391

A failure on version change after a failure on code changes (ran version bump after the above step) (note that as expected it doesn't skip despite being a version bump):
https://github.com/Unstructured-IO/unstructured/actions/runs/6450130450

A success on code changes (PR as is):
https://github.com/Unstructured-IO/unstructured/actions/runs/6451418433

A success on version change after success on code changes (ran version bump after the above step) (note that it skips since previous run was successful AND it was a version bump):
https://github.com/Unstructured-IO/unstructured/actions/runs/6451650440

NOTE: I have not exhaustively tested every scenario, so appreciate a second pair of eyes

@ryannikolaidis ryannikolaidis changed the title ci: skip tests build(test): skip tests Oct 7, 2023
@ryannikolaidis ryannikolaidis changed the title build(test): skip tests build(test): skip tests on version and changelog changes Oct 8, 2023
@ryannikolaidis ryannikolaidis marked this pull request as ready for review October 9, 2023 02:39
@ryannikolaidis
Copy link
Contributor Author

closing this for now, need to rethink in terms of maintainability, since this adds quite a bit more complexity to the testing graph

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.

feat/CI: Avoid running unit and ingest test when changes are limited to the changelog file
2 participants