Fix: Add test case for setting the default settings and check the get… #247
Conversation
sandhya-c
changed the title
|
build seems to be failing. I guess new macros has to be used instead of old ones. Recently updates have happened One such example : LZT_TEST_F has to be used instead of TEST_F |
|
For commit msg, Fix tag may not be appropriate, since this is a new test. test: Add test for validating ECC default state |
There was a problem hiding this comment.
Pull Request Overview
This PR adds a test case to verify ECC (Error Correcting Code) state management functionality, specifically testing the ability to set default ECC settings and retrieve the ECC state correctly. The test ensures that when ECC settings are configured with default values, the API correctly returns the expected state information.
Key changes:
- Added comprehensive test case for ECC default state handling
- Implemented validation of ECC state transitions and pending actions
- Added proper structure initialization and API call verification
Comments suppressed due to low confidence (1)
conformance_tests/sysman/test_sysman_ecc/src/test_sysman_ecc.cpp:190
- The test name contains a typo: 'GIven' should be 'Given'. Test names should be clear and properly spelled for maintainability.
GIvenValidDeviceHandleAndECCAvailableWhenEccGetDefaultStateIsQueriedAndSetThenDefaultValuesAreReturnedFromGetEccAPI) {
| configurable == static_cast<ze_bool_t>(true)) { | ||
| // Get ECC state with defaults | ||
| EXPECT_EQ(ZE_RESULT_SUCCESS, zesDeviceGetEccState(device, &getProps)); | ||
| zes_device_ecc_state_t defaultState = getProps.currentState; |
There was a problem hiding this comment.
The variable 'defaultState' is assigned but never used in the test. Consider removing this unused variable or using it for validation if that was the intended purpose.
Copilot uses AI. Check for mistakes.
sandhya-c
force-pushed
the
level_zero_tests_ecc
branch
3 times, most recently
from
d728058 to
c4cbfc2
Compare
|
|
||
| LZT_TEST_F( | ||
| ECC_TEST, | ||
| GivenValidDeviceHandleAndECCAvailableWhenEccGetDefaultStateIsQueriedAndSetThenDefaultValuesAreReturnedFromGetEccAPI) { |
There was a problem hiding this comment.
The test name is dubious as it mentions querying default state, but in the test we are just using/reading the current ecc value.
| for (const auto &device : devices) { | ||
| // Check if ECC is available and configurable | ||
| zes_device_ecc_properties_t getProps = {}; | ||
| zes_device_ecc_default_properties_ext_t extProps = {}; |
There was a problem hiding this comment.
How are we using this? The default state would come as part of this ext structure but we dont seem to be using it,
| // Set ECC state with default settings | ||
| zes_device_ecc_desc_t newState = {ZES_STRUCTURE_TYPE_DEVICE_ECC_DESC, | ||
| nullptr}; | ||
| newState.state = getProps.currentState; |
There was a problem hiding this comment.
What are we trying to do here? Why are we setting the current state again?
sandhya-c
force-pushed
the
level_zero_tests_ecc
branch
4 times, most recently
from
fc3c827 to
8f872c4
Compare
…t ECC state Related-To: VLCLJ-2434 Signed-off-by: kalyan alle <[email protected]>
sandhya-c
force-pushed
the
level_zero_tests_ecc
branch
from
8f872c4 to
427689d
Compare
… ECC state
Related-To: VLCLJ-2434