Fix skipIfXpu and skipIfHpu disables tests when used on class #151315
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/151315
Note: Links to docs will display an error until the docs builds have been completed. ❗ 1 Active SEVsThere are 1 currently active SEVs. If your PR is affected, please view them below: ✅ You can merge normally! (1 Unrelated Failure)As of commit 4e82023 with merge base 032ef48 ( FLAKY - The following job failed but was likely due to flakiness present on trunk:
This comment was automatically generated by Dr. CI and updates every 15 minutes. |
skipIfXpu and skipIfHPU disables tests when used on class
skipIfXpu and skipIfHPU disables tests when used on classThere was a problem hiding this comment.
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
Comments suppressed due to low confidence (1)
test/distributed/test_functional_api.py:92
- [nitpick] Consider adding a clear skip reason message to the unittest.skipIf decorators for HPU tests to improve readability and consistency with the XPU tests.
@unittest.skipIf(TEST_HPU)
There was a problem hiding this comment.
Change looks reasonable to me(fix invocations though, i.e. failure are real), and please add a test plan. Also, if test class is multi-device, this decorator will skip CPU parts of it as well.
Also, removed "Fixes" as problem is not the actual use of the decorator, but the fact that this behavior is untested
| @@ -90,7 +89,7 @@ def new_subgroups(group_size: int, pg_tag=None): | |||
| return cur_subgroup, subgroups | |||
|
|
|||
|
|
|||
| @skipIfHpu | |||
| @unittest.skipIf(TEST_HPU) | |||
There was a problem hiding this comment.
| @unittest.skipIf(TEST_HPU) | |
| @unittest.skipIf(TEST_HPU, "Unsupported on HPU") |
|
I will submit another PR to refine the |
|
By the way, @malfet , I'm fixing the CI failed test cases. When the CI signal is green, I will request your help in reviewing this PR again. |
|
The merge job was canceled or timed out. This most often happen if two merge requests were issued for the same PR, or if merge job was waiting for more than 6 hours for tests to finish. In later case, please do not hesitate to reissue the merge command |
|
@pytorchbot merge |
Merge startedYour change will be merged once all checks pass (ETA 0-4 Hours). Learn more about merging in the wiki. Questions? Feedback? Please reach out to the PyTorch DevX Team |
|
The merge job was canceled or timed out. This most often happen if two merge requests were issued for the same PR, or if merge job was waiting for more than 6 hours for tests to finish. In later case, please do not hesitate to reissue the merge command |
|
@pytorchbot merge |
Merge startedYour change will be merged once all checks pass (ETA 0-4 Hours). Learn more about merging in the wiki. Questions? Feedback? Please reach out to the PyTorch DevX Team |
|
The merge job was canceled or timed out. This most often happen if two merge requests were issued for the same PR, or if merge job was waiting for more than 6 hours for tests to finish. In later case, please do not hesitate to reissue the merge command |
|
@pytorchbot merge |
Merge startedYour change will be merged once all checks pass (ETA 0-4 Hours). Learn more about merging in the wiki. Questions? Feedback? Please reach out to the PyTorch DevX Team |
|
The merge job was canceled or timed out. This most often happen if two merge requests were issued for the same PR, or if merge job was waiting for more than 6 hours for tests to finish. In later case, please do not hesitate to reissue the merge command |
|
@pytorchbot rebase |
|
@pytorchbot started a rebase job onto refs/rem 9E88 otes/origin/viable/strict. Check the current status here |
|
Successfully rebased |
|
@pytorchbot merge |
Merge startedYour change will be merged once all checks pass (ETA 0-4 Hours). Learn more about merging in the wiki. Questions? Feedback? Please reach out to the PyTorch DevX Team |
|
@pytorchbot revert -m "Seems to have introduced internal regressions, see D74668899. @malfet may you help the author get this PR merged?" -c ghfirst |
|
@pytorchbot successfully started a revert job. Check the current status here. |
…#151315)" This reverts commit ee096b8. Reverted #151315 on behalf of https://github.com/jeanschmidt due to Seems to have introduced internal regressions, see [D74668899](https://www.internalfb.com/diff/D74668899). @malfet may you help the author get this PR merged? ([comment](#151315 (comment)))
|
@EikanWang your PR has been successfully reverted. |
|
May I know the internal failures? So that I can refine this PR. |
Stack from ghstack (oldest at bottom):
cc @H-Huang @awgu @wanchaol @fegin @fduwjj @wz337 @wconstab @d4l3k @voznesenskym @penguinwu @jgong5 @Guobing-Chen @XiaobingSuper @zhuhaozhe @blzheng @wenzhe-nrv @jiayisunx @ipiszy @chenyang78 @kadeng @muchulee8 @amjames @chauhang @aakhundov