Skip to content
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 the missing sort for lsusb in reboot_check_test.py and fix unused device comparison result (BugFix) #1607

Merged
merged 6 commits into from
Nov 22, 2024

Conversation

tomli380576
Copy link
Contributor

@tomli380576 tomli380576 commented Nov 19, 2024

Description

  • Added a sort step after calling checkbox-support-lsusb
  • Suppress FWTS progress messages to make stderr cleaner and errors more visible
  • Made the comparison output more verbose when a comparison fails
  • Fixed an issue where device comparison result was not updated after calling compare_device_lists

Resolved issues

  1. The original shell script had an explicit checkbox-support-lsusb | sort step that was missing in the python version. Adding this back to preserve the same behavior.
  2. Device comparison result was never updated. This becomes a problem if the only issue on the DUT is device comparison

Documentation

Tests

Unit tests

Sideloaded the fix on 202312-33267 and plugged in an USB stick during reboot to force a device difference. The fix correctly reports that the device config changed and failed the test case

Copy link

codecov bot commented Nov 19, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 48.02%. Comparing base (7b5dd02) to head (edf2947).
Report is 30 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1607      +/-   ##
==========================================
+ Coverage   48.00%   48.02%   +0.02%     
==========================================
  Files         371      371              
  Lines       39833    39849      +16     
  Branches     6730     6732       +2     
==========================================
+ Hits        19121    19138      +17     
  Misses      19994    19994              
+ Partials      718      717       -1     
Flag Coverage Δ
provider-base 24.79% <100.00%> (+0.08%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.


🚨 Try these New Features:

@tomli380576 tomli380576 changed the title Add the missing sort step for lsusb in reboot_check_test.py (BugFix) Add the missing sort step for lsusb in reboot_check_test.py and print detailed diff outputs (BugFix) Nov 19, 2024
@tomli380576 tomli380576 marked this pull request as ready for review November 19, 2024 08:09
@tomli380576 tomli380576 changed the title Add the missing sort step for lsusb in reboot_check_test.py and print detailed diff outputs (BugFix) Add the missing sort for lsusb in reboot_check_test.py and fix unused device comparison result (BugFix) Nov 19, 2024
@tomli380576 tomli380576 requested review from Hook25 and removed request for Hook25 November 19, 2024 08:53
@Hook25 Hook25 self-assigned this Nov 22, 2024
Copy link
Collaborator

@Hook25 Hook25 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1, lgtm

@Hook25 Hook25 merged commit 6205aed into main Nov 22, 2024
49 checks passed
@Hook25 Hook25 deleted the missing-lsusb-sort branch November 22, 2024 16:40
eugene-yujinwu pushed a commit to eugene-yujinwu/checkbox that referenced this pull request Dec 31, 2024
… device comparison result (BugFix) (canonical#1607)

* fix: missing sort in reboot_check_test.py

* fix: detailed diff

* fix: device comp result never used

* test: increase coverage

* feat: use -q to suppress fwts progress messages

* fix: restore stderr
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants