Skip to content

Ignored Module tests: provide required input tensor #19028

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.

Sign up for GitHub

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

Open
psiddh wants to merge 2 commits into
base: main
Choose a base branch
from
+7 −17
Open

Ignored Module tests: provide required input tensor #19028

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
698acec
Fix 4 @Ignored Module tests: provide required input tensor
psiddh Apr 21, 2026
8fe6bec
Apply suggestions from code review
psiddh Apr 21, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 7 additions & 17 deletions ...ecutorch_android/src/androidTest/java/org/pytorch/executorch/ModuleInstrumentationTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ import java.util.concurrent.atomic.AtomicInteger
import org.apache.commons.io.FileUtils
import org.junit.Assert
import org.junit.Before
import org.junit.Ignore
import org.junit.Test
import org.junit.runner.RunWith
import org.pytorch.executorch.TestFileUtils.getTestFilePath
Expand All @@ -40,15 +39,12 @@ class ModuleInstrumentationTest {
inputStream.close()
}

@Ignore(
"The forward has failure that needs to be fixed before enabling this test: [Executorch Error 0x12] Invalid argument: Execution failed for method: forward "
)
@Test
@Throws(IOException::class, URISyntaxException::class)
fun testModuleLoadAndForward() {
val module = Module.load(getTestFilePath(TEST_FILE_NAME))

val results = module.forward()
val results = module.forward(EValue.from(dummyInput()))
Assert.assertTrue(results[0].isTensor)
Comment on lines 45 to 48
Copy link

Copilot AI Apr 21, 2026

Choose a reason for hiding this comment

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

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 uses AI. Check for mistakes.
}

Expand All @@ -58,29 +54,23 @@ class ModuleInstrumentationTest {
val module = Module.load(getTestFilePath(TEST_FILE_NAME))
}

@Ignore(
"The forward has failure that needs to be fixed before enabling this test: [Executorch Error 0x12] Invalid argument: Execution failed for method: forward "
)
@Test
@Throws(IOException::class)
fun testModuleLoadMethodAndForward() {
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)
Comment on lines 60 to 65
Copy link

Copilot AI Apr 21, 2026

Choose a reason for hiding this comment

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

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 uses AI. Check for mistakes.
}

@Ignore(
"The forward has failure that needs to be fixed before enabling this test: [Executorch Error 0x12] Invalid argument: Execution failed for method: forward "
)
@Test
@Throws(IOException::class)
fun testModuleLoadForwardExplicit() {
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)
Comment on lines 71 to 74
Copy link

Copilot AI Apr 21, 2026

Choose a reason for hiding this comment

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

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 uses AI. Check for mistakes.
}

Expand Down Expand Up @@ -135,9 +125,6 @@ class ModuleInstrumentationTest {
Assert.assertThrows(IllegalStateException::class.java) { module.forward() }
}

@Ignore(
"The forward has failure that needs to be fixed before enabling this test: [Executorch Error 0x12] Invalid argument: Execution failed for method: forward "
)
@Test
@Throws(InterruptedException::class, IOException::class)
fun testForwardFromMultipleThreads() {
Expand All @@ -151,7 +138,7 @@ class ModuleInstrumentationTest {
try {
latch.countDown()
latch.await(5000, TimeUnit.MILLISECONDS)
val results = module.forward()
val results = module.forward(EValue.from(dummyInput()))
Assert.assertTrue(results[0].isTensor)
Comment on lines +141 to 142
Copy link

Copilot AI Apr 21, 2026

Choose a reason for hiding this comment

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

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.

Copilot uses AI. Check for mistakes.
Comment on lines 140 to 142
Copy link

Copilot AI Apr 21, 2026

Choose a reason for hiding this comment

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

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.

Copilot uses AI. Check for mistakes.
completed.incrementAndGet()
} catch (_: InterruptedException) {}
Expand All @@ -176,5 +163,8 @@ class ModuleInstrumentationTest {
private const val NON_PTE_FILE_NAME = "/test.txt"
private const val FORWARD_METHOD = "forward"
private const val NONE_METHOD = "none"
private val inputShape = longArrayOf(1, 3, 224, 224)

private fun dummyInput(): Tensor = Tensor.ones(inputShape, DType.FLOAT)
}
}
Loading