Skip signals from older runs of the same workflows #129291
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/129291
Note: Links to docs will display an error until the docs builds have been completed. ✅ No FailuresAs of commit e4a8bab with merge base 1c75ddf ( This comment was automatically generated by Dr. CI and updates every 15 minutes. |
| @@ -397,6 +397,7 @@ def test_gql_complexity(self, *args: Any) -> None: | |||
| # self.assertGreater(len(pr.get_checkrun_conclusions()), 3) | |||
| self.assertGreater(pr.get_commit_count(), 60) | |||
|
|
|||
| @skip("GitHub doesn't keep this data anymore") | |||
There was a problem hiding this comment.
why not delete the test if we don't expect it to start working again later?
There was a problem hiding this comment.
It might be useful to know this in case there is a more recent sample, but it's very unlikely I guess, so cleaning it up then
|
|
||
| if ( | ||
| workflow_id not in workflows | ||
| or workflows[workflow_id].run_id < workflow_run_id |
There was a problem hiding this comment.
nit: consider adding a comment explaining that this check is to help ignore reruns
|
@pytorchbot merge -f 'CI only fix, no need to run trunk jobs' |
Merge startedYour change will be merged immediately since you used the force (-f) flag, bypassing any CI checks (ETA: 1-5 minutes). Please use Learn more about merging in the wiki. Questions? Feedback? Please reach out to the PyTorch DevX Team |
I discovered this bug in trymerge when debugging #129013 in which Dr.CI reported no relevant failures while mergebot complained about some unrelated ROCm failures #129013 (comment).
It turns out that mergebot took into account stale signals from older runs of the same workflow here. For example,
Notice that both runs came from the same push to commit be69191 with ciflow/rocm/129013. So, we just need to check the signals from the newer run.
Note that Dr.CI handles this part correctly using the logic in https://github.com/pytorch/test-infra/blob/main/torchci/pages/api/drci/drci.ts#L1079-L1088. So, the fix in this PR is to bring the same logic to trymerge.
Testing
pytest -v test_trymerge.py