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

[Dr.CI] Handle the closing of disabled test issues #5186

Merged
merged 7 commits into from
May 9, 2024

Conversation

huydhn
Copy link
Contributor

@huydhn huydhn commented May 8, 2024

This extends Dr.CI to handle disabled test issues in several cases:

  1. The closing of disabled test issues whereas a test could start failing on PR after its associated disabled issue is resolved.
  2. A disabled test fails on PR where it should be skipped.

In both cases, the failure will be marked as skipped and the reason is provided.

Testing

pytorch/pytorch#125656

❌ 1 New Failure, 3 Unrelated Failures

As of commit 3a1e14cc5b5d44fa1b04561bccf14d832f70eb1d with merge base b6bcd0917310c657fc60bc7b20415f6948524eb8 (image):

NEW FAILURE - The following job has failed:

FLAKY - The following jobs failed but were likely due to flakiness present on trunk:

pytorch/pytorch#125634

❌ 1 New Failure, 1 Unrelated Failure

As of commit 28cdf5b4af9803379b6206b8ffbdf214949f111b with merge base 22bcfc25ef3043db37e88ad61ba69c85f98571e1 (image):

NEW FAILURE - The following job has failed:

FLAKY - The following job failed but was likely due to flakiness present on trunk:

@huydhn huydhn requested review from clee2000 and a team May 8, 2024 03:59
Copy link

vercel bot commented May 8, 2024

@huydhn is attempting to deploy a commit to the Meta Open Source Team on Vercel.

A member of the Team first needs to authorize it.

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label May 8, 2024
Copy link

vercel bot commented May 8, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
torchci ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 9, 2024 2:15am

@huydhn huydhn marked this pull request as ready for review May 8, 2024 17:47
Copy link
Contributor

@clee2000 clee2000 left a comment

Choose a reason for hiding this comment

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

I think the logic is sound, some minor concerns about token usage, and a lot of minor comments regarding readability of the tests

torchci/lib/jobUtils.ts Show resolved Hide resolved
let prBody = "";
let prShas: { sha: string; title: string }[] = [];
if (octokit) {
const prData = await fetchPR(owner, repo, `${prNumber}`, octokit);
Copy link
Contributor

Choose a reason for hiding this comment

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

should we be worried about rate limits for this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Per our chat, this will incur the cost of 2 GitHub queries per PR

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I guess I will follow up with an alternative to query from Rockset if this turns out to be an issue. I'm not quite sure if the pull_request Rockset table is always up to date. Maybe there is a legit reason we are using GitHub query to get the PR info instead.

torchci/test/drciBot.test.ts Show resolved Hide resolved
torchci/test/jobUtils.test.ts Show resolved Hide resolved
torchci/test/jobUtils.test.ts Outdated Show resolved Hide resolved
torchci/test/jobUtils.test.ts Outdated Show resolved Hide resolved
torchci/lib/jobUtils.ts Show resolved Hide resolved
torchci/pages/api/drci/drci.ts Show resolved Hide resolved
torchci/pages/api/drci/drci.ts Outdated Show resolved Hide resolved
@huydhn
Copy link
Contributor Author

huydhn commented May 9, 2024

lol, I just found a perfect example of a recently close disabled issue in pytorch/pytorch#122074. The issue was closed by pytorch/pytorch#125706. Both PR were landed close together

✅ You can merge normally! (4 Unrelated Failures)

As of commit 14343544cbbe4d553f4d5a2084e3f83ee8028e82 with merge base b6bcd0917310c657fc60bc7b20415f6948524eb8 (image):

FLAKY - The following jobs failed but were likely due to flakiness present on trunk:

BROKEN TRUNK - The following job failed but were present on the merge base:

👉 Rebase onto the `viable/strict` branch to avoid these failures

@huydhn huydhn merged commit f476189 into pytorch:main May 9, 2024
7 checks passed
clee2000 added a commit that referenced this pull request May 9, 2024
After #5186

Link to hud which then redirects to the issue to prevent the PR getting
included in the issue's events like the below

![image](https://github.com/pytorch/test-infra/assets/44682903/89023fe4-f649-42d3-bfd6-be7523055fde)


Ran on one of my PRs, worked fine

<!-- drci-comment-start -->

## 🔗 Helpful Links
### 🧪 See artifacts and rendered test results at
[hud.pytorch.org/pr/125799](https://hud.pytorch.org/pr/125799)
* 📄 Preview [Python docs built from this
PR](https://docs-preview.pytorch.org/pytorch/pytorch/125799/index.html)
* 📄 Preview [C++ docs built from this
PR](https://docs-preview.pytorch.org/pytorch/pytorch/125799/cppdocs/index.html)
* ❓ Need help or want to give feedback on the CI? Visit the
[bot commands
wiki](https://github.com/pytorch/pytorch/wiki/Bot-commands) or our
[office
hours](https://github.com/pytorch/pytorch/wiki/Dev-Infra-Office-Hours)

Note: Links to docs will display an error until the docs builds have
been completed.


## ❌ 1 New Failure, 5 Pending, 3 Unrelated Failures
As of commit 852acd87cd5384ac7c61a93d1e0d894f39a6df13 with merge base
8f27c7f181f05ecac5f1948dc070d7781d3da137 (<sub><sub><img alt="image"
width=70
src="https://img.shields.io/date/1715204973?label=&color=FFFFFF&style=flat-square"></sub></sub>):
<details open><summary><b>NEW FAILURE</b> - The following job has
failed:</summary><p>

* [Lint / lintrunner-noclang /
linux-job](https://hud.pytorch.org/pr/pytorch/pytorch/125799#24795916616)
([gh](https://github.com/pytorch/pytorch/actions/runs/9023455827/job/24795916616))
    `>>> Lint for tools/testing/upload_artifacts.py:`
</p></details>
<details ><summary><b>FLAKY</b> - The following jobs failed but were
likely due to flakiness present on trunk:</summary><p>

* [pull / linux-focal-py3.8-clang10 / test (crossref, 1, 2,
linux.2xlarge)](https://hud.pytorch.org/pr/pytorch/pytorch/125799#24795855797)
([gh](https://github.com/pytorch/pytorch/actions/runs/9023455802/job/24795855797))
(disabled by
[#104012](https://hud.pytorch.org/pytorch/pytorch/issues/104012) but the
issue was closed recently and a rebase is needed to make it pass)
`test_fx.py::TestFXAPIBackwardCompatibility::test_public_api_surface`
* [pull / linux-focal-py3.8-clang10 / test (default, 1, 3,
linux.2xlarge)](https://hud.pytorch.org/pr/pytorch/pytorch/125799#24795855164)
([gh](https://github.com/pytorch/pytorch/actions/runs/9023455802/job/24795855164))
(disabled by
[#104012](https://hud.pytorch.org/pytorch/pytorch/issues/104012) but the
issue was closed recently and a rebase is needed to make it pass)
`test_fx.py::TestFXAPIBackwardCompatibility::test_public_api_surface`
* [pull / linux-focal-py3.8-clang10 / test (dynamo, 1, 3,
linux.2xlarge)](https://hud.pytorch.org/pr/pytorch/pytorch/125799#24795856213)
([gh](https://github.com/pytorch/pytorch/actions/runs/9023455802/job/24795856213))
(disabled by
[#104012](https://hud.pytorch.org/pytorch/pytorch/issues/104012) but the
issue was closed recently and a rebase is needed to make it pass)
`test_fx.py::TestFXAPIBackwardCompatibility::test_public_api_surface`
</p></details>


This comment was automatically generated by Dr. CI and updates every 15
minutes.
<!-- drci-comment-end -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants