-
Notifications
You must be signed in to change notification settings - Fork 66
Fix: Handling GTEST_FAIL logic for multi-device scenario for power, performance and scheduler modules #273
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.
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
Fix: Handling GTEST_FAIL logic for multi-device scenario for power, performance and scheduler modules #273
Conversation
Sub-tasks: VLCLJ-2570, VLCLJ-2568, VLCLJ-2577 Fix: Handling GTEST_FAIL logic for multi-device scenario for power, performance and scheduler modules. Signed-off-by: viki435 <[email protected]>
build seems to be failing |
Sub-tasks: VLCLJ-2570, VLCLJ-2568, VLCLJ-2577 Fix: Handling GTEST_FAIL logic for multi-device scenario for power, performance and scheduler modules. Signed-off-by: viki435 <[email protected]>
Sub-tasks: VLCLJ-2570, VLCLJ-2568, VLCLJ-2577 Fix: Handling GTEST_FAIL logic for multi-device scenario for power, performance and scheduler modules. Signed-off-by: viki435 <[email protected]>
Sub-tasks: VLCLJ-2570, VLCLJ-2568, VLCLJ-2577 Fix: Handling GTEST_FAIL logic for multi-device scenario for power, performance and scheduler modules. Signed-off-by: viki435 <[email protected]>
Conflicts have to be resolved in this PR! |
Sub-tasks: VLCLJ-2570, VLCLJ-2568, VLCLJ-2577 Fix: Handling GTEST_FAIL logic for multi-device scenario for power, performance and scheduler modules. Signed-off-by: viki435 <[email protected]>
Sub-tasks: VLCLJ-2570, VLCLJ-2568, VLCLJ-2577 Fix: Handling GTEST_FAIL logic for multi-device scenario for power, performance and scheduler modules. Signed-off-by: viki435 <[email protected]>
Sub-tasks: VLCLJ-2570, VLCLJ-2568, VLCLJ-2577 Fix: Handling GTEST_FAIL logic for multi-device scenario for power, performance and scheduler modules. Signed-off-by: viki435 <[email protected]>
Sub-tasks: VLCLJ-2570, VLCLJ-2568, VLCLJ-2577 Fix: Handling GTEST_FAIL logic for multi-device scenario for power, performance and scheduler modules. Signed-off-by: viki435 <[email protected]>
Sub-tasks: VLCLJ-2570, VLCLJ-2568, VLCLJ-2577 Fix: Handling GTEST_FAIL logic for multi-device scenario for power, performance and scheduler modules. Signed-off-by: viki435 <[email protected]>
Sub-tasks: VLCLJ-2570, VLCLJ-2568, VLCLJ-2577 Fix: Handling GTEST_FAIL logic for multi-device scenario for power, performance and scheduler modules. Signed-off-by: viki435 <[email protected]>
Sub-tasks: VLCLJ-2570, VLCLJ-2568, VLCLJ-2577 Fix: Handling GTEST_FAIL logic for multi-device scenario for power, performance and scheduler modules. Signed-off-by: viki435 <[email protected]>
Sub-tasks: VLCLJ-2570, VLCLJ-2568, VLCLJ-2577 Fix: Handling GTEST_FAIL logic for multi-device scenario for power, performance and scheduler modules. Signed-off-by: viki435 <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR fixes GTEST_FAIL logic for multi-device scenarios across power, performance, and scheduler modules. The key changes ensure that tests fail only when all devices lack support, rather than failing immediately when a single device doesn't support the feature.
Key changes:
- Added class-level boolean flags to track if any device supports the feature
- Replaced immediate failure on unsupported features with logging and continued iteration
- Added final check to fail only if no devices support the feature across all devices
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
File | Description |
---|---|
test_sysman_scheduler.cpp | Added is_scheduler_supported flag and refactored failure logic to check all devices before failing |
test_sysman_power.cpp | Added is_power_supported flag and updated failure handling for multi-device power testing |
test_sysman_performance.cpp | Added is_performance_supported flag and modified failure logic for performance module testing |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
conformance_tests/sysman/test_sysman_power/src/test_sysman_power.cpp
Outdated
Show resolved
Hide resolved
@@ -266,27 +316,34 @@ class PerformanceModuleParamComputePerformanceFactorTest | |||
LZT_TEST_P( | |||
PERFORMANCE_COMPUTE_TEST, | |||
GivenValidPerformanceHandleWhenSettingMultiplePerformanceFactorForComputeThenValidPerformanceFactorIsReturned) { | |||
bool is_perf_supported = false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why is it defined inside the test?
it should be added in class PerformanceModuleParamComputePerformanceFactorZesTest & PerformanceModuleParamComputePerformanceFactorTest
and can be renamed to is_performance_supported to maintain consistency in this entire file
power_sustained_set.limitValueLocked); | ||
EXPECT_EQ(power_sustained_get.interval, power_sustained_set.interval); | ||
EXPECT_EQ(power_sustained_get.limit, power_sustained_set.limit); | ||
EXPECT_EQ(power_peak_get.limitValueLocked, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why power_sustained is changed to power_peak ?
@@ -607,298 +764,229 @@ LZT_TEST_F( | |||
} | |||
EXPECT_ZE_RESULT_SUCCESS(status); | |||
} else { | |||
LOG_INFO << "Set limit not supported due to sustained " | |||
LOG_INFO << "Set limit not supported due to peak " |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
similarly here. why sustained is changed to peak ?
@@ -607,298 +764,229 @@ LZT_TEST_F( | |||
} | |||
EXPECT_ZE_RESULT_SUCCESS(status); | |||
} else { | |||
LOG_INFO << "Set limit not supported due to sustained " | |||
LOG_INFO << "Set limit not supported due to peak " |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you make sure that in this file only gtest_fail logic is changed.
other functional test logic should not be changed
…erformance and scheduler modules. Primary JIRA: VLCLJ-2513 Sub-tasks: VLCLJ-2570, VLCLJ-2568, VLCLJ-2577 Signed-off-by: viki435 <[email protected]>
…erformance and scheduler modules. Primary JIRA: VLCLJ-2513 Sub-tasks: VLCLJ-2570, VLCLJ-2568, VLCLJ-2577 Signed-off-by: viki435 <[email protected]>
Signed-off-by: viki435 <[email protected]>
f749ec5
to
58d06c2
Compare
This PR can be closed since a new one is raised #284 |
Primary JIRA: VLCLJ-2513
Sub-tasks: VLCLJ-2570, VLCLJ-2568, VLCLJ-2577
Fix: Handling GTEST_FAIL logic for multi-device scenario for power, performance and scheduler modules.