Ignored Module tests: provide required input tensor by psiddh · Pull Request #19028 · pytorch/executorch

psiddh · 2026-04-21T16:44:49Z

All 4 tests failed because they called forward() with zero arguments on mobilenet_v2 which expects a [1,3,224,224] float input. This was a test bug, not a runtime bug. Add a dummyInput() helper that creates a Tensor.ones with the correct shape, and remove all @ignore annotations.

@ignore

All 4 tests failed because they called forward() with zero arguments on mobilenet_v2 which expects a [1,3,224,224] float input. This was a test bug, not a runtime bug. Add a dummyInput() helper that creates a Tensor.ones with the correct shape, and remove all @ignore annotations. This commit was authored with the help of Claude.

pytorch-bot · 2026-04-21T16:44:57Z

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/19028

📄 Preview Python docs built from this PR

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

❌ 1 New Failure, 4 Unrelated Failures

As of commit 8fe6bec with merge base 66e4656 ():

NEW FAILURE - The following job has failed:

pull / test-qnn-passes-linux / linux-job (gh)
backends/qualcomm/tests/test_passes.py::TestPasses::test_mha_to_sha

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

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

pull / unittest / macos / macos-job (gh) (trunk failure)
export/tests/test_target_recipes.py::TestTargetRecipes::test_mv2_model
pull / unittest / windows / windows-job (gh) (trunk failure)
##[error]The operation was canceled.
pull / unittest-editable / macos / macos-job (gh) (trunk failure)
export/tests/test_target_recipes.py::TestTargetRecipes::test_mv2_model
pull / unittest-editable / windows / windows-job (gh) (trunk failure)
##[error]The operation was canceled.

This comment was automatically generated by Dr. CI and updates every 15 minutes.

github-actions · 2026-04-21T16:45:40Z

This PR needs a `release notes:` label

If your change should be included in the release notes (i.e. would users of this library care about this change?), please use a label starting with release notes:. This helps us keep track and include your important work in the next release notes.

To add a label, you can comment to pytorchbot, for example
@pytorchbot label "release notes: none"

For more information, see
https://github.com/pytorch/pytorch/wiki/PyTorch-AutoLabel-Bot#why-categorize-for-release-notes-and-how-does-it-work.

Copilot

Pull request overview

Fixes previously ignored Android instrumentation tests for Module by providing the required MobilenetV2 input tensor so forward()/execute() are invoked with valid arguments.

Changes:

Remove @Ignore annotations from four instrumentation tests.
Add a dummyInput() helper that constructs a [1, 3, 224, 224] float tensor and pass it into forward() / execute().
Update the multi-threaded forward test to use the dummy input.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-21T16:48:49Z

+        val results = module.forward(EValue.from(dummyInput()))
        Assert.assertTrue(results[0].isTensor)


In the multi-thread test each worker allocates a new 1x3x224x224 float tensor via dummyInput(). With 100 threads this can create significant short-lived allocations (~60MB+), which can make the instrumentation test flaky on memory-constrained devices. Consider pre-creating a single Tensor (or even EValue) and reusing it across threads/tests to avoid repeated allocations.

JacobSzwejbka · 2026-04-21T19:13:41Z

@claude review this PR

claude · 2026-04-21T19:14:24Z

Claude finished @JacobSzwejbka's task in 52s —— View job

Review of PR #19028

Read the changed file and diff
Analyze the changes
Post review feedback

The fix is correct. The 4 tests were @Ignored because forward() was called with no arguments on mobilenet_v2, which requires a [1, 3, 224, 224] float input tensor. Adding the dummyInput() helper and passing it to each call is the right approach.

Minor items:

Naming convention (ModuleInstrumentationTest.kt:166): INPUT_SHAPE uses SCREAMING_CASE, which is conventional for const val in Kotlin. However, longArrayOf(...) cannot be const val (arrays aren't compile-time constants), so it's a private val. The sibling file ModuleE2ETest.kt:74 uses inline longArrayOf(1, 3, 224, 224) rather than a named constant, so there's no local precedent either way. This is a very minor style nit — not worth blocking on.
Multi-threaded allocations (ModuleInstrumentationTest.kt:141): Each of the 100 threads creates a new 1×3×224×224 float tensor (~600KB each, ~60MB total). This is fine for a test — Tensor.ones is cheap, the tensors are short-lived, and the test is verifying thread-safety of Module.forward(), not allocation performance. Pre-sharing a tensor across threads would actually weaken the test by introducing a shared mutable input. No change needed.
testForwardOnDestroyedModule (ModuleInstrumentationTest.kt:125): This test still calls module.forward() with no arguments — but that's intentional, since the test asserts IllegalStateException from calling forward on a destroyed module. The exception is thrown before argument validation, so the zero-arg call is fine here.

Verdict: This is a clean, minimal fix. LGTM.

JacobSzwejbka · 2026-04-21T20:55:39Z

@claude explain the ci failures on this PR

claude · 2026-04-21T20:56:26Z

Claude finished @JacobSzwejbka's task in 1m 47s —— View job

Get CI status overview
Investigate the new failure (test_mha_to_sha)
Check if failures are related to PR changes
Summarize findings

CI Failure Analysis

This PR only modifies a single file (ModuleInstrumentationTest.kt). None of the CI failures are caused by this PR's changes.

1. New Failure: `test-qnn-passes-linux` / `test_mha_to_sha`

Not related to this PR. The test fails with:

ModuleNotFoundError: No module named 'executorch.examples.qualcomm'

at backends/qualcomm/tests/test_passes.py:152, where it tries to import executorch.examples.qualcomm.oss_scripts.llama.masking_utils. This is a CI environment/packaging issue — the examples/qualcomm directory isn't being installed as a Python package in the test Docker container. This PR doesn't touch any Qualcomm backend code.

The Dr. CI bot flags this as a "NEW FAILURE" (not present on the merge base), which likely means this test was recently added or its dependencies changed in a recent trunk commit. It's unrelated to the Android test fix in this PR.

2. Cancelled Job: `unittest-editable / windows / windows-job`

This was simply cancelled — not a failure. CI jobs get cancelled for various infrastructure reasons (timeouts, resource contention, etc.). Retrying should resolve it.

3. Broken Trunk (3 failures)

These all existed on the merge base commit (66e4656) before this PR:

test_mv2_model (macOS + macOS-editable): export/tests/test_target_recipes.py::TestTargetRecipes::test_mv2_model — pre-existing failure on trunk
Windows unittest: cancelled with The operation was canceled — infrastructure issue on trunk

Recommendation: Rebasing onto viable/strict (as Dr. CI suggests) would clear the broken trunk failures. The test_mha_to_sha failure needs to be investigated separately by the Qualcomm backend team — it's a missing module issue in the CI test environment.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 4 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-21T21:19:13Z

    val module = Module.load(getTestFilePath(TEST_FILE_NAME))

-    val results = module.forward()
+    val results = module.forward(EValue.from(dummyInput()))
    Assert.assertTrue(results[0].isTensor)


This test now runs forward() successfully, but the Module is never destroyed. Since Module owns native resources and GC timing is non-deterministic, this can cause memory retention across instrumentation tests (especially now that these tests are no longer @ignore). Consider wrapping the test body in try/finally and calling module.destroy() in the finally block.

Copilot · 2026-04-21T21:19:13Z

    val module = Module.load(getTestFilePath(TEST_FILE_NAME))

    module.loadMethod(FORWARD_METHOD)

-    val results = module.forward()
+    val results = module.forward(EValue.from(dummyInput()))
    Assert.assertTrue(results[0].isTensor)


The Module instance created in this test is not destroyed. Because Module wraps native state and tests are now enabled, relying on GC can retain native memory longer than intended and introduce flakes. Consider calling module.destroy() (ideally via try/finally) after the assertion.

Copilot · 2026-04-21T21:19:13Z

    val module = Module.load(getTestFilePath(TEST_FILE_NAME))

-    val results = module.execute(FORWARD_METHOD)
+    val results = module.execute(FORWARD_METHOD, EValue.from(dummyInput()))
    Assert.assertTrue(results[0].isTensor)


The Module created here is never destroyed. Since Module owns native resources, consider cleaning it up with module.destroy() (e.g., in a finally block) to avoid retaining native memory across the instrumentation test run.

Copilot · 2026-04-21T21:19:14Z

        latch.await(5000, TimeUnit.MILLISECONDS)
-        val results = module.forward()
+        val results = module.forward(EValue.from(dummyInput()))
        Assert.assertTrue(results[0].isTensor)


This test loads a Module and runs it across many threads but never calls module.destroy() after the threads finish. Given the native resources involved (and now that the test is enabled), add a destroy in teardown/try-finally after joins to reduce memory retention and potential test flakiness.

psiddh requested a review from kirklandsign as a code owner April 21, 2026 16:44

Copilot AI review requested due to automatic review settings April 21, 2026 16:44

meta-cla 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 Apr 21, 2026

psiddh changed the title ~~Fix 4 @Ignored Module tests: provide required input tensor~~ Ignored Module tests: provide required input tensor Apr 21, 2026

Copilot started reviewing on behalf of psiddh April 21, 2026 16:45 View session

psiddh mentioned this pull request Apr 21, 2026

Android ET APIs : Path to Stable (From Experimental) #18950

Open

Copilot AI reviewed Apr 21, 2026

View reviewed changes

Apply suggestions from code review

8fe6bec

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot AI review requested due to automatic review settings April 21, 2026 21:15

Copilot started reviewing on behalf of psiddh April 21, 2026 21:15 View session

Copilot AI reviewed Apr 21, 2026

View reviewed changes

		val results = module.forward(EValue.from(dummyInput()))
		Assert.assertTrue(results[0].isTensor)

Conversation

psiddh commented Apr 21, 2026

Uh oh!

pytorch-bot Bot commented Apr 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/19028

❌ 1 New Failure, 4 Unrelated Failures

Uh oh!

github-actions Bot commented Apr 21, 2026

This PR needs a release notes: label

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI Apr 21, 2026

Choose a reason for hiding this comment

Uh oh!

JacobSzwejbka commented Apr 21, 2026

Uh oh!

claude Bot commented Apr 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review of PR #19028

Uh oh!

JacobSzwejbka commented Apr 21, 2026

Uh oh!

claude Bot commented Apr 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

CI Failure Analysis

1. New Failure: test-qnn-passes-linux / test_mha_to_sha

2. Cancelled Job: unittest-editable / windows / windows-job

3. Broken Trunk (3 failures)

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Apr 21, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 21, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 21, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 21, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

pytorch-bot Bot commented Apr 21, 2026 •

edited

Loading

This PR needs a `release notes:` label

claude Bot commented Apr 21, 2026 •

edited

Loading

claude Bot commented Apr 21, 2026 •

edited

Loading

1. New Failure: `test-qnn-passes-linux` / `test_mha_to_sha`

2. Cancelled Job: `unittest-editable / windows / windows-job`