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

Evaluate canary-run condition in run-tests SelectiveChecks #45387

Merged
merged 2 commits into from
Jan 4, 2025

Conversation

gopidesupavan
Copy link
Member

@gopidesupavan gopidesupavan commented Jan 3, 2025

This will help cases like when run-tests becomes false if it is canary-run .
Some times tests are getting skipped in canary run.

We have condition in jobs to trigger for run-tests, even thought this works but often some times run-tests becomes false when the last merge commit main has only some UI changes. then this causing skipping of canary run tests jobs. see below it has skipped most of the jobs. because the selective check here https://github.com/apache/airflow/actions/runs/12602875121/job/35126775759#step:8:217 triggered only the ui file changes

https://github.com/apache/airflow/actions/runs/12602875121/job/35126775759
https://github.com/apache/airflow/actions/runs/12602875121/job/35126775759#step:8:310


^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named {pr_number}.significant.rst or {issue_number}.significant.rst, in newsfragments.

.github/workflows/ci.yml Outdated Show resolved Hide resolved
@potiuk
Copy link
Member

potiuk commented Jan 4, 2025

I think it would be better to reevaluate 'run-tests' condition in this case in selective_checks and make sure it's never false in canary runs. This would be more natural, actually testable with unit tests and we would avoid the strange ternary

@gopidesupavan
Copy link
Member Author

I think it would be better to reevaluate 'run-tests' condition in this case in selective_checks and make sure it's never false in canary runs. This would be more natural, actually testable with unit tests and we would avoid the strange ternary

Yeah make sense , i think i got it where its failing check, let me get updated changes..

@gopidesupavan gopidesupavan changed the title Evaluate canary-run condition in workflow Evaluate canary-run condition in run-tests SelectiveChecks Jan 4, 2025
Copy link
Member

@potiuk potiuk left a comment

Choose a reason for hiding this comment

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

Yep. Precisely :)

@potiuk potiuk merged commit c37d5cf into apache:main Jan 4, 2025
86 checks passed
@gopidesupavan gopidesupavan deleted the fix-selective-checks branch January 4, 2025 15:16
LefterisXefteris pushed a commit to LefterisXefteris/airflow that referenced this pull request Jan 5, 2025
)

* evaluate canary run condition in workflow

* Update run tests check to verify canary run
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants