Skip to content

Commit b34be83

Browse files
authored
Enable program_test in CI and fix flaky LoadFromMutableSegment test (#16709)
Enable program_test target in CMakeLists.txt which was previously disabled (TODO T191569140). Fix the LoadFromMutableSegment test which was fragile due to checking for a hardcoded byte value (232) that varies across platforms and PyTorch versions. The test now: 1. Loads the full 36-byte weight tensor instead of a single byte 2. Verifies data consistency by comparing two loads with memcmp 3. Checks that loaded data is non-trivial (contains non-zero bytes) 4. Validates source immutability by mutating local buffer and reloading to verify original data is preserved This maintains the test's purpose of validating mutable segment loading while being robust across different environments.
1 parent e53613f commit b34be83

2 files changed

Lines changed: 53 additions & 23 deletions

File tree

runtime/executor/test/CMakeLists.txt

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -133,12 +133,17 @@ set_property(TEST method_test PROPERTY ENVIRONMENT ${test_env})
133133
# TODO(T191569140): Enable this test. et_cxx_test(method_meta_test SOURCES
134134
# method_meta_test.cpp EXTRA_LIBS extension_data_loader)
135135

136-
# TODO(T191569140): Enable this test. et_cxx_test( program_test SOURCES
137-
# program_test.cpp EXTRA_LIBS extension_data_loader )
136+
et_cxx_test(
137+
program_test SOURCES program_test.cpp EXTRA_LIBS extension_data_loader
138+
program_schema
139+
)
140+
add_dependencies(program_test generated_pte_files)
141+
set_property(TEST program_test PROPERTY ENVIRONMENT ${test_env})
138142

139-
# target_include_directories( program_test PRIVATE
140-
# "${CMAKE_INSTALL_PREFIX}/schema/include"
141-
# "${EXECUTORCH_ROOT}/third-party/flatbuffers/include" )
143+
target_include_directories(
144+
program_test PRIVATE "${CMAKE_INSTALL_PREFIX}/schema/include"
145+
"${EXECUTORCH_ROOT}/third-party/flatbuffers/include"
146+
)
142147

143148
et_cxx_test(
144149
kernel_resolution_test SOURCES kernel_resolution_test.cpp EXTRA_LIBS

runtime/executor/test/program_test.cpp

Lines changed: 43 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -187,6 +187,14 @@ TEST_F(ProgramTest, BadMagicFailsToLoad) {
187187
}
188188
}
189189

190+
// These tests require ET_ENABLE_PROGRAM_VERIFICATION to be enabled.
191+
// In Release builds, verification is disabled by default to save binary size.
192+
#ifndef ET_ENABLE_PROGRAM_VERIFICATION
193+
#define ET_ENABLE_PROGRAM_VERIFICATION 1
194+
#endif
195+
196+
#if ET_ENABLE_PROGRAM_VERIFICATION
197+
190198
TEST_F(ProgramTest, VerificationCatchesTruncation) {
191199
// Get the program data.
192200
size_t full_data_len = add_loader_->size().get();
@@ -234,6 +242,8 @@ TEST_F(ProgramTest, VerificationCatchesCorruption) {
234242
ASSERT_EQ(program.error(), Error::InvalidProgram);
235243
}
236244

245+
#endif // ET_ENABLE_PROGRAM_VERIFICATION
246+
237247
TEST_F(ProgramTest, UnalignedProgramDataFails) {
238248
// Make a local copy of the data, on an odd alignment.
239249
size_t data_len = add_loader_->size().get();
@@ -508,32 +518,47 @@ TEST_F(ProgramTest, LoadFromMutableSegment) {
508518
Result<Program> program = Program::load(&training_loader.get());
509519
ASSERT_EQ(program.error(), Error::Ok);
510520

511-
// dummy buffers to load into
512-
uint8_t buffer[1] = {0};
513-
uint8_t buffer2[1] = {0};
521+
// The linear layer weight tensor is 3x3 floats = 36 bytes.
522+
// Load the full weight data to verify the loading mechanism works correctly.
523+
constexpr size_t kWeightSize = 36;
524+
uint8_t buffer[kWeightSize] = {0};
525+
uint8_t buffer2[kWeightSize] = {0};
514526

515-
// Load some mutable segment data
527+
// Load the mutable segment data (linear.weight at offset_index 1)
516528
Error err = ProgramTestFriend::load_mutable_subsegment_into(
517-
&program.get(), 0, 1, 1, buffer);
529+
&program.get(), 0, 1, kWeightSize, buffer);
518530
EXPECT_EQ(err, Error::Ok);
519531

520-
// Check that the data loaded correctly, and then mutate it
521-
EXPECT_EQ(buffer[0], 232); // 232 comes from inspecting the file itself. The
522-
// file is seeded so this value should be stable.
523-
buffer[0] = 0;
532+
// Load the same data into a second buffer
533+
err = ProgramTestFriend::load_mutable_subsegment_into(
534+
&program.get(), 0, 1, kWeightSize, buffer2);
535+
EXPECT_EQ(err, Error::Ok);
536+
537+
// Verify both loads return identical data
538+
EXPECT_EQ(std::memcmp(buffer, buffer2, kWeightSize), 0)
539+
<< "Loading the same segment twice should return identical data";
540+
541+
// Verify that the loaded data is non-trivial (not all zeros).
542+
bool has_nonzero = false;
543+
for (size_t i = 0; i < kWeightSize; ++i) {
544+
if (buffer[i] != 0) {
545+
has_nonzero = true;
546+
break;
547+
}
548+
}
549+
EXPECT_TRUE(has_nonzero)
550+
<< "Loaded mutable segment data should not be all zeros";
524551

525-
// Load the same mutable segment data from file into a different buffer.
552+
// Mutate the local buffer and reload to verify source immutability
553+
std::memset(buffer, 0xFF, kWeightSize);
526554
err = ProgramTestFriend::load_mutable_subsegment_into(
527-
&program.get(),
528-
0, // mutable_data_segments_index
529-
1, // offset_index
530-
1, // size
531-
buffer2);
555+
&program.get(), 0, 1, kWeightSize, buffer);
532556
EXPECT_EQ(err, Error::Ok);
533557

534-
// Check that new data loaded from the file does not reflect the change to
535-
// buffer.
536-
EXPECT_EQ(buffer2[0], 232);
558+
// Verify the reloaded data matches the original (local mutation didn't
559+
// affect the file source)
560+
EXPECT_EQ(std::memcmp(buffer, buffer2, kWeightSize), 0)
561+
<< "Reloading after local mutation should return original data";
537562

538563
const executorch_flatbuffer::Program* flatbuffer_program =
539564
ProgramTestFriend::GetInternalProgram(&program.get());

0 commit comments

Comments
 (0)