Refactor: Split firmware flash test in multiple tests for different fw types #283

kalyanalle · 2025-08-21T17:34:57Z

Related-To: VLCLJ-2532/VLCLJ-2533

Operational Constraint/Known Behavior:

The OptionROM code and OptionROM data firmware flashing tests both use the same firmware name (OptionROM.bin) as retrieved from get_firmware_properties.
It is the user's responsibility to ensure that the correct OptionROM.bin file is placed in the directory specified by the ZE_LZT_FIRMWARE_DIRECTORY environment variable.
The test does not distinguish between code and data; it uses the firmware handle's name as provided by the API.
If different images are required for code and data, the user must manage the file content accordingly.

vishnu-khanth · 2025-08-25T05:28:27Z

New tests have to follow similar changes done firmware tests in commit
7027d05#diff-2ce519d2aec202571b08fdd2425714438d86a0cac9bcfce7f82a6ff70a97d6e5

Better to rebase to latest master and apply the changes

kalyanalle · 2025-08-25T08:18:21Z

New tests have to follow similar changes done firmware tests in commit
7027d05#diff-2ce519d2aec202571b08fdd2425714438d86a0cac9bcfce7f82a6ff70a97d6e5

Better to rebase to latest master and apply the changes

done! , taken care.

conformance_tests/sysman/test_sysman_firmware/src/test_sysman_firmware.cpp

…w types OptionROM firmwares Related-To: VLCLJ-2532 Removing GivenValidFirmwareHandleWhenFlashingOptionRomThenExpectFirmwareFlashingSuccess and creating single parameterized test with two different params GFX and OptionROM. Signed-off-by: Alle, Kalyan <[email protected]>

vishnu-khanth · 2025-09-08T07:49:21Z

conformance_tests/sysman/test_sysman_firmware/src/test_sysman_firmware.cpp

+    uint32_t count = 0;
+    count = lzt::get_firmware_handle_count(device);
+    if (count > 0) {
+      is_firmware_supported = true;


After this LOG_INFO << "Firmware handles are available on this device! "; could be added

vishnu-khanth · 2025-09-08T07:54:12Z

PR title could also be updated to

refactor: Split firmware flash test in multiple tests for different fw types

Copilot

Pull Request Overview

This PR refactors the firmware flashing tests to support parameterized testing for specific firmware types (GFX and OptionROM). The changes replace a generic firmware flashing test with a more targeted approach that can test different firmware types individually.

Key changes:

Removed the generic firmware flashing test that attempted to flash all available firmware
Added parameterized test infrastructure to test specific firmware types (GFX and OptionROM)
Enhanced logging to provide better visibility into firmware name and path during testing

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

Copilot · 2025-09-08T08:29:01Z

conformance_tests/sysman/test_sysman_firmware/src/test_sysman_firmware.cpp

+          std::vector<char> testFwImage(
+              static_cast<size_t>(inFileStream.tellg()));
+          inFileStream.seekg(0, inFileStream.beg);
+          inFileStream.read(testFwImage.data(), testFwImage.size());
+          lzt::flash_firmware(firmware_handle,
+                              static_cast<void *>(testFwImage.data()),
+                              testFwImage.size());


The variable name testFwImage uses inconsistent naming compared to the existing flash_firmware function which uses test_fw_image (snake_case). Consider using test_fw_image for consistency.

Suggested change

std::vector<char> testFwImage(

static_cast<size_t>(inFileStream.tellg()));

inFileStream.seekg(0, inFileStream.beg);

inFileStream.read(testFwImage.data(), testFwImage.size());

lzt::flash_firmware(firmware_handle,

static_cast<void *>(testFwImage.data()),

testFwImage.size());

std::vector<char> test_fw_image(

static_cast<size_t>(inFileStream.tellg()));

inFileStream.seekg(0, inFileStream.beg);

inFileStream.read(test_fw_image.data(), test_fw_image.size());

lzt::flash_firmware(firmware_handle,

static_cast<void *>(test_fw_image.data()),

test_fw_image.size());

Copilot · 2025-09-08T08:29:01Z

conformance_tests/sysman/test_sysman_firmware/src/test_sysman_firmware.cpp

+          std::string fwName(reinterpret_cast<char *>(propFw.name));
+          std::string fwToLoad = fwDir + "/" + fwName + ".bin";
+          LOG_INFO << " Firmware Name: " << fwName;
+          LOG_INFO << " Firmware Path: " << fwToLoad;
+          std::ifstream inFileStream(fwToLoad,


The variable names fwName and fwToLoad use inconsistent camelCase naming compared to the existing flash_firmware function which uses fw_name and fw_to_load (snake_case). Consider using snake_case for consistency with the existing codebase.

Suggested change

std::string fwName(reinterpret_cast<char *>(propFw.name));

std::string fwToLoad = fwDir + "/" + fwName + ".bin";

LOG_INFO << " Firmware Name: " << fwName;

LOG_INFO << " Firmware Path: " << fwToLoad;

std::ifstream inFileStream(fwToLoad,

std::string fw_name(reinterpret_cast<char *>(propFw.name));

std::string fw_to_load = fwDir + "/" + fw_name + ".bin";

LOG_INFO << " Firmware Name: " << fw_name;

LOG_INFO << " Firmware Path: " << fw_to_load;

std::ifstream inFileStream(fw_to_load,

Copilot · 2025-09-08T08:29:02Z

conformance_tests/sysman/test_sysman_firmware/src/test_sysman_firmware.cpp

+          std::ifstream inFileStream(fwToLoad,
+                                     std::ios::binary | std::ios::ate);
+          if (!inFileStream.is_open()) {
+            LOG_INFO << "Skipping test as firmware image not found";
+            GTEST_SKIP();
+          }
+          std::vector<char> testFwImage(
+              static_cast<size_t>(inFileStream.tellg()));
+          inFileStream.seekg(0, inFileStream.beg);
+          inFileStream.read(testFwImage.data(), testFwImage.size());


The variable name inFileStream uses inconsistent camelCase naming compared to the existing flash_firmware function which uses in_filestream (snake_case). Consider using in_filestream for consistency.

Suggested change

std::ifstream inFileStream(fwToLoad,

std::ios::binary | std::ios::ate);

if (!inFileStream.is_open()) {

LOG_INFO << "Skipping test as firmware image not found";

GTEST_SKIP();

}

std::vector<char> testFwImage(

static_cast<size_t>(inFileStream.tellg()));

inFileStream.seekg(0, inFileStream.beg);

inFileStream.read(testFwImage.data(), testFwImage.size());

std::ifstream in_filestream(fwToLoad,

std::ios::binary | std::ios::ate);

if (!in_filestream.is_open()) {

LOG_INFO << "Skipping test as firmware image not found";

GTEST_SKIP();

}

std::vector<char> testFwImage(

static_cast<size_t>(in_filestream.tellg()));

in_filestream.seekg(0, in_filestream.beg);

in_filestream.read(testFwImage.data(), testFwImage.size());

vishnu-khanth · 2025-09-08T08:33:52Z

@kalyanalle can you share how the test names look like when the test is run ?
Changes looks good to me

vishnu-khanth · 2025-09-08T08:35:26Z

And I see that commit msg is updated, this PR title also can be updated

vishnu-khanth

LGTM!

kalyanalle requested review from vishnu-khanth and AshwinKumarKulkarni August 21, 2025 17:35

vishnu-khanth requested a review from Copilot August 25, 2025 05:25

This comment was marked as outdated.

Sign in to view

kalyanalle force-pushed the gfx_oprom_firmware_flash branch from fab4416 to 90a3b13 Compare August 25, 2025 08:17

vishnu-khanth reviewed Sep 2, 2025

View reviewed changes

conformance_tests/sysman/test_sysman_firmware/src/test_sysman_firmware.cpp Outdated Show resolved Hide resolved

vishnu-khanth reviewed Sep 2, 2025

View reviewed changes

conformance_tests/sysman/test_sysman_firmware/src/test_sysman_firmware.cpp Outdated Show resolved Hide resolved

kalyanalle force-pushed the gfx_oprom_firmware_flash branch from 90a3b13 to 428f851 Compare September 2, 2025 15:23

kalyanalle requested review from vishnu-khanth and Copilot September 2, 2025 15:24

This comment was marked as outdated.

Sign in to view

kalyanalle force-pushed the gfx_oprom_firmware_flash branch from 428f851 to 4c1357b Compare September 8, 2025 06:40

kalyanalle requested a review from Copilot September 8, 2025 06:41

This comment was marked as outdated.

Sign in to view

vishnu-khanth reviewed Sep 8, 2025

View reviewed changes

kalyanalle force-pushed the gfx_oprom_firmware_flash branch from 4c1357b to a7e60a4 Compare September 8, 2025 08:27

kalyanalle requested review from Copilot and vishnu-khanth September 8, 2025 08:28

Copilot AI reviewed Sep 8, 2025

View reviewed changes

kalyanalle changed the title ~~Update: Adding tests for flashing GFX, OptionROM firmwares~~ Refactor: Split firmware flash test in multiple tests for different fw types Sep 8, 2025

vishnu-khanth approved these changes Sep 8, 2025

View reviewed changes

Refactor: Split firmware flash test in multiple tests for different fw types #283

Are you sure you want to change the base?

Refactor: Split firmware flash test in multiple tests for different fw types #283

Uh oh!

Conversation

kalyanalle commented Aug 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vishnu-khanth commented Aug 25, 2025

Uh oh!

This comment was marked as outdated.

Uh oh!

kalyanalle commented Aug 25, 2025

Uh oh!

Uh oh!

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

vishnu-khanth Sep 8, 2025

Choose a reason for hiding this comment

Uh oh!

kalyanalle Sep 8, 2025

Choose a reason for hiding this comment

Uh oh!

vishnu-khanth commented Sep 8, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Copilot AI Sep 8, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 8, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 8, 2025

Choose a reason for hiding this comment

Uh oh!

vishnu-khanth commented Sep 8, 2025

Uh oh!

vishnu-khanth commented Sep 8, 2025

Uh oh!

vishnu-khanth left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

kalyanalle commented Aug 21, 2025 •

edited

Loading