Migrate broken tests to ET_SKIP_IF macro (#19659)#19659
Merged
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/19659
Note: Links to docs will display an error until the docs builds have been completed. ❗ 2 Active SEVsThere are 2 currently active SEVs. If your PR is affected, please view them below: ⏳ No Failures, 1 PendingAs of commit e7e74d5 with merge base f3387d0 ( This comment was automatically generated by Dr. CI and updates every 15 minutes. |
meta-cla
Bot
added
the
CLA Signed
Contributor
|
@metascroy has exported this pull request. If you are a Meta employee, you can view the originating Diff in D105645070. |
This PR needs a
|
metascroy
added a commit
to metascroy/executorch
that referenced
this pull request
May 19, 2026
Summary: Pull Request resolved: pytorch#19659 Follow-up to D105634345 -- migrate the remaining `if (cond) { GTEST_SKIP() << reason; }` sites in `executorch/kernels/test/op_*_test.cpp` (and `UnaryUfuncRealHBBF16ToFloatHBF16Test.cpp`) to the new `ET_SKIP_IF(cond, reason)` macro introduced in that diff. # Why In fbcode, TestX flags any test that consistently calls `GTEST_SKIP()` at runtime as "broken" / `ConsistentlySkipping`. The `SupportedFeatures::get()->X` guards are statically known per kernel variant -- they aren't infra failures, the test just doesn't apply -- but they still trip the broken-test signal and file oncall tasks. `ET_SKIP_IF` keeps the OSS `GTEST_SKIP` behavior and rewrites the fbcode build to an early `return;` so the test reports PASS. See D105634345, T208053850, https://fb.workplace.com/groups/testinfra.discuss/permalink/2044665472719153/. # What is in this diff Mechanical migration of ~219 sites across 84 test files. The transform: ``` if (cond) { GTEST_SKIP() << "reason"; } ``` becomes: ``` ET_SKIP_IF(cond, "reason"); ``` Each migrated file now `#include`s `executorch/kernels/test/supported_features_skip.h` (where the macro lives). xplat mirrors are updated alongside their fbcode counterparts. A handful of edge cases needed manual handling: * `op_log_softmax_test.cpp::TestForwardWithLargeNumbersOptimized` -- the `GTEST_SKIP` site has a sibling `expect_failure();` inside the `if`. Kept the `if` block and used `ET_SKIP_IF(true, ...)` after the failure check so the early-return semantics still match the original skip. * `op_scalar_tensor_test.cpp::GENERATE_TEST` -- `GTEST_SKIP` lived inside a `#define` macro with `\` line continuations; rewritten to a multi-line `ET_SKIP_IF` with line continuations. * `op_split_copy_test.cpp::DtypeMismatchDies` -- unconditional `GTEST_SKIP()` (no `if`); converted to `ET_SKIP_IF(true, ...)` so the test returns PASS in fbcode and continues to SKIP in OSS. OSS behavior is unchanged in every case: the macro expands to the prior `if (cond) GTEST_SKIP() << reason;` form. Differential Revision: D105645070
metascroy
force-pushed
the
export-D105645070
branch
from
293c212 to
19e45e2
Compare
meta-codesync
Bot
changed the title
metascroy
added a commit
to metascroy/executorch
that referenced
this pull request
May 19, 2026
Summary: Pull Request resolved: pytorch#19659 Follow-up to D105634345 -- migrate the remaining `if (cond) { GTEST_SKIP() << reason; }` sites in `executorch/kernels/test/op_*_test.cpp` (and `UnaryUfuncRealHBBF16ToFloatHBF16Test.cpp`) to the new `ET_SKIP_IF(cond, reason)` macro introduced in that diff. # Why In fbcode, TestX flags any test that consistently calls `GTEST_SKIP()` at runtime as "broken" / `ConsistentlySkipping`. The `SupportedFeatures::get()->X` guards are statically known per kernel variant -- they aren't infra failures, the test just doesn't apply -- but they still trip the broken-test signal and file oncall tasks. `ET_SKIP_IF` keeps the OSS `GTEST_SKIP` behavior and rewrites the fbcode build to an early `return;` so the test reports PASS. See D105634345, T208053850, https://fb.workplace.com/groups/testinfra.discuss/permalink/2044665472719153/. # What is in this diff Mechanical migration of ~219 sites across 84 test files. The transform: ``` if (cond) { GTEST_SKIP() << "reason"; } ``` becomes: ``` ET_SKIP_IF(cond, "reason"); ``` Each migrated file now `#include`s `executorch/kernels/test/supported_features_skip.h` (where the macro lives). xplat mirrors are updated alongside their fbcode counterparts. A handful of edge cases needed manual handling: * `op_log_softmax_test.cpp::TestForwardWithLargeNumbersOptimized` -- the `GTEST_SKIP` site has a sibling `expect_failure();` inside the `if`. Kept the `if` block and used `ET_SKIP_IF(true, ...)` after the failure check so the early-return semantics still match the original skip. * `op_scalar_tensor_test.cpp::GENERATE_TEST` -- `GTEST_SKIP` lived inside a `#define` macro with `\` line continuations; rewritten to a multi-line `ET_SKIP_IF` with line continuations. * `op_split_copy_test.cpp::DtypeMismatchDies` -- unconditional `GTEST_SKIP()` (no `if`); converted to `ET_SKIP_IF(true, ...)` so the test returns PASS in fbcode and continues to SKIP in OSS. OSS behavior is unchanged in every case: the macro expands to the prior `if (cond) GTEST_SKIP() << reason;` form. Differential Revision: D105645070
metascroy
force-pushed
the
export-D105645070
branch
from
19e45e2 to
e5d1c33
Compare
metascroy
added a commit
to metascroy/executorch
that referenced
this pull request
May 19, 2026
Summary: Pull Request resolved: pytorch#19659 Follow-up to D105634345 -- migrate the remaining `if (cond) { GTEST_SKIP() << reason; }` sites in `executorch/kernels/test/op_*_test.cpp` (and `UnaryUfuncRealHBBF16ToFloatHBF16Test.cpp`) to the new `ET_SKIP_IF(cond, reason)` macro introduced in that diff. # Why In fbcode, TestX flags any test that consistently calls `GTEST_SKIP()` at runtime as "broken" / `ConsistentlySkipping`. The `SupportedFeatures::get()->X` guards are statically known per kernel variant -- they aren't infra failures, the test just doesn't apply -- but they still trip the broken-test signal and file oncall tasks. `ET_SKIP_IF` keeps the OSS `GTEST_SKIP` behavior and rewrites the fbcode build to an early `return;` so the test reports PASS. See D105634345, T208053850, https://fb.workplace.com/groups/testinfra.discuss/permalink/2044665472719153/. # What is in this diff Mechanical migration of ~219 sites across 84 test files. The transform: ``` if (cond) { GTEST_SKIP() << "reason"; } ``` becomes: ``` ET_SKIP_IF(cond, "reason"); ``` Each migrated file now `#include`s `executorch/kernels/test/supported_features_skip.h` (where the macro lives). xplat mirrors are updated alongside their fbcode counterparts. A handful of edge cases needed manual handling: * `op_log_softmax_test.cpp::TestForwardWithLargeNumbersOptimized` -- the `GTEST_SKIP` site has a sibling `expect_failure();` inside the `if`. Kept the `if` block and used `ET_SKIP_IF(true, ...)` after the failure check so the early-return semantics still match the original skip. * `op_scalar_tensor_test.cpp::GENERATE_TEST` -- `GTEST_SKIP` lived inside a `#define` macro with `\` line continuations; rewritten to a multi-line `ET_SKIP_IF` with line continuations. * `op_split_copy_test.cpp::DtypeMismatchDies` -- unconditional `GTEST_SKIP()` (no `if`); converted to `ET_SKIP_IF(true, ...)` so the test returns PASS in fbcode and continues to SKIP in OSS. OSS behavior is unchanged in every case: the macro expands to the prior `if (cond) GTEST_SKIP() << reason;` form. Differential Revision: D105645070
metascroy
force-pushed
the
export-D105645070
branch
from
e5d1c33 to
4c90d7b
Compare
metascroy
added a commit
to metascroy/executorch
that referenced
this pull request
May 19, 2026
Summary: Pull Request resolved: pytorch#19659 Follow-up to D105634345 -- migrate the remaining `if (cond) { GTEST_SKIP() << reason; }` sites in `executorch/kernels/test/op_*_test.cpp` (and `UnaryUfuncRealHBBF16ToFloatHBF16Test.cpp`) to the new `ET_SKIP_IF(cond, reason)` macro introduced in that diff. # Why In fbcode, TestX flags any test that consistently calls `GTEST_SKIP()` at runtime as "broken" / `ConsistentlySkipping`. The `SupportedFeatures::get()->X` guards are statically known per kernel variant -- they aren't infra failures, the test just doesn't apply -- but they still trip the broken-test signal and file oncall tasks. `ET_SKIP_IF` keeps the OSS `GTEST_SKIP` behavior and rewrites the fbcode build to an early `return;` so the test reports PASS. See D105634345, T208053850, https://fb.workplace.com/groups/testinfra.discuss/permalink/2044665472719153/. # What is in this diff Mechanical migration of ~219 sites across 84 test files. The transform: ``` if (cond) { GTEST_SKIP() << "reason"; } ``` becomes: ``` ET_SKIP_IF(cond, "reason"); ``` Each migrated file now `#include`s `executorch/kernels/test/supported_features_skip.h` (where the macro lives). xplat mirrors are updated alongside their fbcode counterparts. A handful of edge cases needed manual handling: * `op_log_softmax_test.cpp::TestForwardWithLargeNumbersOptimized` -- the `GTEST_SKIP` site has a sibling `expect_failure();` inside the `if`. Kept the `if` block and used `ET_SKIP_IF(true, ...)` after the failure check so the early-return semantics still match the original skip. * `op_scalar_tensor_test.cpp::GENERATE_TEST` -- `GTEST_SKIP` lived inside a `#define` macro with `\` line continuations; rewritten to a multi-line `ET_SKIP_IF` with line continuations. * `op_split_copy_test.cpp::DtypeMismatchDies` -- unconditional `GTEST_SKIP()` (no `if`); converted to `ET_SKIP_IF(true, ...)` so the test returns PASS in fbcode and continues to SKIP in OSS. OSS behavior is unchanged in every case: the macro expands to the prior `if (cond) GTEST_SKIP() << reason;` form. Differential Revision: D105645070
metascroy
force-pushed
the
export-D105645070
branch
from
4c90d7b to
4581519
Compare
Summary: Pull Request resolved: pytorch#19659 Follow-up to D105634345 -- migrate the remaining `if (cond) { GTEST_SKIP() << reason; }` sites in `executorch/kernels/test/op_*_test.cpp` (and `UnaryUfuncRealHBBF16ToFloatHBF16Test.cpp`) to the new `ET_SKIP_IF(cond, reason)` macro introduced in that diff. # Why In fbcode, TestX flags any test that consistently calls `GTEST_SKIP()` at runtime as "broken" / `ConsistentlySkipping`. The `SupportedFeatures::get()->X` guards are statically known per kernel variant -- they aren't infra failures, the test just doesn't apply -- but they still trip the broken-test signal and file oncall tasks. `ET_SKIP_IF` keeps the OSS `GTEST_SKIP` behavior and rewrites the fbcode build to an early `return;` so the test reports PASS. See D105634345, T208053850, https://fb.workplace.com/groups/testinfra.discuss/permalink/2044665472719153/. # What is in this diff Mechanical migration of ~219 sites across 84 test files. The transform: ``` if (cond) { GTEST_SKIP() << "reason"; } ``` becomes: ``` ET_SKIP_IF(cond, "reason"); ``` Each migrated file now `#include`s `executorch/kernels/test/supported_features_skip.h` (where the macro lives). xplat mirrors are updated alongside their fbcode counterparts. A handful of edge cases needed manual handling: * `op_log_softmax_test.cpp::TestForwardWithLargeNumbersOptimized` -- the `GTEST_SKIP` site has a sibling `expect_failure();` inside the `if`. Kept the `if` block and used `ET_SKIP_IF(true, ...)` after the failure check so the early-return semantics still match the original skip. * `op_scalar_tensor_test.cpp::GENERATE_TEST` -- `GTEST_SKIP` lived inside a `#define` macro with `\` line continuations; rewritten to a multi-line `ET_SKIP_IF` with line continuations. * `op_split_copy_test.cpp::DtypeMismatchDies` -- unconditional `GTEST_SKIP()` (no `if`); converted to `ET_SKIP_IF(true, ...)` so the test returns PASS in fbcode and continues to SKIP in OSS. OSS behavior is unchanged in every case: the macro expands to the prior `if (cond) GTEST_SKIP() << reason;` form. Differential Revision: D105645070
metascroy
force-pushed
the
export-D105645070
branch
from
4581519 to
e7e74d5
Compare
psiddh
approved these changes
May 19, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary:
Follow-up to D105634345 -- migrate the remaining
if (cond) { GTEST_SKIP() << reason; }sites inexecutorch/kernels/test/op_*_test.cpp(andUnaryUfuncRealHBBF16ToFloatHBF16Test.cpp) to the newET_SKIP_IF(cond, reason)macro introduced in that diff.Why
In fbcode, TestX flags any test that consistently calls
GTEST_SKIP()at runtime as "broken" /ConsistentlySkipping. TheSupportedFeatures::get()->Xguards are statically known per kernel variant -- they aren't infra failures, the test just doesn't apply -- but they still trip the broken-test signal and file oncall tasks.ET_SKIP_IFkeeps the OSSGTEST_SKIPbehavior and rewrites the fbcode build to an earlyreturn;so the test reports PASS.See D105634345, T208053850, https://fb.workplace.com/groups/testinfra.discuss/permalink/2044665472719153/.
What is in this diff
Mechanical migration of ~219 sites across 84 test files. The transform:
becomes:
Each migrated file now
#includesexecutorch/kernels/test/supported_features_skip.h(where the macro lives). xplat mirrors are updated alongside their fbcode counterparts.A handful of edge cases needed manual handling:
op_log_softmax_test.cpp::TestForwardWithLargeNumbersOptimized-- theGTEST_SKIPsite has a siblingexpect_failure();inside theif. Kept theifblock and usedET_SKIP_IF(true, ...)after the failure check so the early-return semantics still match the original skip.op_scalar_tensor_test.cpp::GENERATE_TEST--GTEST_SKIPlived inside a#definemacro with\line continuations; rewritten to a multi-lineET_SKIP_IFwith line continuations.op_split_copy_test.cpp::DtypeMismatchDies-- unconditionalGTEST_SKIP()(noif); converted toET_SKIP_IF(true, ...)so the test returns PASS in fbcode and continues to SKIP in OSS.OSS behavior is unchanged in every case: the macro expands to the prior
if (cond) GTEST_SKIP() << reason;form.Differential Revision: D105645070