-
Notifications
You must be signed in to change notification settings - Fork 52
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
Add LED manifest entries for interactive LED checks (BugFix) #1385
Conversation
Instead of asking the tester to manually skip tests when a device does not have a given LED (for instance no Microphone Mute LED), this commit introduces manifest entries for all the related LED jobs. This way, the tester defines what LEDs the device has before running the tests, and tests that are not required due to a lack of specific LED are automatically skipped.
The generic "has_led_indicator" manifest entry, which originally came from the CE-OEM provider for IoT testing, is renamed as "has_led_gpio_sysfs" to better reflect the need for IoT projects. It is grouped with other LED-related manifest entries by using the same prompt.
led/wlan, led/wlan-disabled and led/bluetooth are all testing a subset of what led/wireless is testing. This commit - removes these jobs - replaces their calls with led/wireless in related test plans
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1385 +/- ##
==========================================
+ Coverage 45.12% 45.14% +0.02%
==========================================
Files 366 367 +1
Lines 39058 39079 +21
Branches 6607 6611 +4
==========================================
+ Hits 17626 17644 +18
- Misses 20758 20760 +2
- Partials 674 675 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚨 Try these New Features:
|
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.
This modification makes sense to IoT platforms. Thanks!
@GabrielChenCC Do you have any platform with separate suspend LED ? |
@hanhsuan For ThinkPad prototypes, they all have a separate suspend LED; for ThinkCentre and ThinkStation prototypes, they do not have suspend LED. |
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.
+1 on the new manifest entries, I have a doubt about the job removal, Are we sure that the wireless is a superset of removed bt+wifi tests? The comment seems to hint to the opposite actually.
@hanhsuan @GabrielChenCC the |
@pieqq I think so, for the led/suspend, every ThinkPad has a suspend LED. It is indeed a test item. If we don't add a test case to verify it, it is very likely to be missed during testing. This gap should be remedied. So I guess we should add another manifest for the suspend led and add the test to a test plan. |
This submission shows |
Based on feedback from QA team, add a manifest for the suspend LED and use it in the led/suspend job.
@hanhsuan @GabrielChenCC I've made the modification discussed in d5bbe5f. I also found that the Please review the change :) |
LGTM |
Look good to me. Thanks. |
Description
In order to prevent tester from having to manually skip tests that check LEDs that might not be present on a device, this PR introduces manifest entries for all the LED checks that are currently used, so that the tester can select what LED indicators the DUT has:
Before this PR, there was only one generic
has_led_indicator
that originally came from the CE-OEM provider to test some IoT-specific LEDs. This manifest entry is renamed to a more specifichas_led_gpio_sysfs
and grouped with the other LED manifest entries using the same prompt.Please check individual commit messages for more information.
Resolved issues
https://warthogs.atlassian.net/browse/CHECKBOX-1322
Documentation
Tests
Running a Desktop Cert test plan and selecting the LED tests, the new manifest entries are displayed and can be tailored to match the LEDs available in the DUT.