Skip to content

Conversation

@stanley31huang
Copy link
Collaborator

@stanley31huang stanley31huang commented Jul 17, 2025

Description

revised watchdog_config_test.py, to split system-config part into one check_timeout and check_service functions.
add watchdog_test.py for following purpose

  • watchdog detection test, detect the watchdog devices from /sys/class/watchdog path, insteads of read the watchdog devices from udev resource. It will probe watchdog kernel module while watchdog is not enabled by default
  • watchdog reset test, trigger kernel panic and expect the system recover by watchdog. It will probe watchdog kernel module is not enabled by default.

This PR has a dependency with PR 2021
#2021

Resolved issues

Documentation

Tests

watchdog is enabled by default and timeout is set

https://certification.canonical.com/hardware/202507-37000/submission/442180/

watchdog is enabled by default, but timeout is not set

https://certification.canonical.com/hardware/202503-36377/submission/442178/

watchdog is not enable by default, and timeout is not set

https://certification.canonical.com/hardware/202503-36377/submission/442181/

watchdog test on system running Ubuntu 18.04

https://certification.canonical.com/hardware/201712-26012/submission/442108/

@stanley31huang stanley31huang force-pushed the revise_watchdog_test_scripts branch from d994471 to ea4e6c7 Compare July 17, 2025 03:44
@codecov
Copy link

codecov bot commented Jul 17, 2025

Codecov Report

❌ Patch coverage is 79.09270% with 106 lines in your changes missing coverage. Please review.
✅ Project coverage is 69.20%. Comparing base (7b6dd5b) to head (67053b2).
⚠️ Report is 42 commits behind head on main.

Files with missing lines Patch % Lines
checkbox-ng/checkbox_ng/launcher/controller.py 65.17% 38 Missing and 1 partial ⚠️
...ckbox-ng/plainbox/impl/session/remote_assistant.py 74.73% 20 Missing and 4 partials ⚠️
checkbox-ng/checkbox_ng/launcher/subcommands.py 60.00% 13 Missing and 3 partials ⚠️
...box-support/checkbox_support/dbus/gnome_monitor.py 87.96% 10 Missing and 3 partials ⚠️
checkbox-ng/checkbox_ng/launcher/config.py 86.56% 8 Missing and 1 partial ⚠️
providers/resource/bin/dmi_resource.py 70.00% 3 Missing ⚠️
checkbox-ng/plainbox/impl/session/assistant.py 92.00% 2 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##             main    #2022       +/-   ##
===========================================
+ Coverage   51.07%   69.20%   +18.12%     
===========================================
  Files         386      216      -170     
  Lines       41561    23377    -18184     
  Branches     7719     4683     -3036     
===========================================
- Hits        21226    16177     -5049     
+ Misses      19574     6559    -13015     
+ Partials      761      641      -120     
Flag Coverage Δ
checkbox-ng 70.91% <74.00%> (+0.21%) ⬆️
checkbox-support 65.06% <91.15%> (+0.40%) ⬆️
contrib-provider-ce-oem ?
provider-base ?
provider-certification-client 57.14% <ø> (ø)
provider-certification-server 57.14% <ø> (ø)
provider-dss ?
provider-genio 96.90% <ø> (ø)
provider-gpgpu 93.06% <ø> (ø)
provider-iiotg 100.00% <ø> (ø)
provider-resource 39.38% <70.00%> (+1.18%) ⬆️
provider-sru 97.97% <ø> (ø)
release-tools ?

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.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@pieqq pieqq self-assigned this Jul 17, 2025
@stanley31huang stanley31huang force-pushed the revise_watchdog_test_scripts branch 2 times, most recently from c4c2f84 to 7368793 Compare July 18, 2025 03:10
Copy link
Collaborator

@pieqq pieqq left a comment

Choose a reason for hiding this comment

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

It looks like the code provided here has been (at least partially) generated with AI. It's not a problem per se, but AI made some strange decisions. For example:

  • writing very detailed docstrings that don't provide any useful information (for example the watchdog_argparse function)
  • being overzealous with argparse's add_argument function without clear rationale
  • overusing the shlex.split function even when it's not necessary
  • the tests are full of comments that are not really useful (they just describe something that's very obvious on the following line)

Could you do a first pass review to check this kind of things, and get back to us after?

Also, I noticed the test submissions don't include the case where the watchdog service is already started. Could you add it? (you might have to re-run the test for the other cases anyway if you do some extra cleanup with the current codebase).

@stanley31huang stanley31huang marked this pull request as draft July 21, 2025 05:02
@stanley31huang stanley31huang force-pushed the revise_watchdog_test_scripts branch from 7368793 to ebde282 Compare July 21, 2025 05:49
revise watchdog_config_test.py
add watchdog_test.py
@stanley31huang stanley31huang force-pushed the revise_watchdog_test_scripts branch from ebde282 to d715d63 Compare July 21, 2025 08:01
ignore pep8 E501 for the usage
@stanley31huang stanley31huang force-pushed the revise_watchdog_test_scripts branch from 64f6c12 to ebed817 Compare July 25, 2025 04:20
@stanley31huang stanley31huang force-pushed the revise_watchdog_test_scripts branch 2 times, most recently from 54e9de3 to 888c969 Compare August 6, 2025 01:48
add watchdog config handler
@stanley31huang stanley31huang force-pushed the revise_watchdog_test_scripts branch from 888c969 to 8906065 Compare August 6, 2025 01:53
@stanley31huang stanley31huang marked this pull request as ready for review August 6, 2025 02:13
@stanley31huang stanley31huang requested a review from pieqq August 6, 2025 02:13
Copy link
Collaborator

@pieqq pieqq left a comment

Choose a reason for hiding this comment

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

I left a bunch of comments and suggestions below.

One thing that I found confusing is that some of the functions you define are type-hinted, while others are not (and when they are type-hinted, the type provided is not always correct, which triggered errors in my text editor!).

start_time = Path(log_dir).joinpath(WATCHDOG_LOG_FILE).read_text()
system_uptime = float(Path("/proc/uptime").read_text().split(" ")[0])
time_diff = time.time() - system_uptime - float(start_time)
if time_diff > 300:
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not a fan of hardcoded values like this, especially to guess if it was caused by user interaction. Slower systems may trigger this even though everything went fine.

My take would be to introduce a variable that can be set when running the script (this can be done using an environment variable); this way, on slow systems, this can be adjusted.

Comment on lines +545 to +550
mock_ins = Mock()
mock_ins.method = "detect"
mock_ins.module = "module"
mock_ins.identity = "identity"
mock_argparse.return_value = mock_ins

Copy link
Collaborator

Choose a reason for hiding this comment

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

What does mock_ins stand for?

stanley31huang and others added 7 commits August 11, 2025 09:24
…gFix) (#2023)

Stage libasound2t64 instead of the virtual package libasound2
* Fix missing docstring

* Fix pep8 linelen in docstring
gntzio and others added 26 commits August 14, 2025 09:05
…BugFix) (#1996)

* Fix crash in ManifestBrowser when only hidden manifests are present.

* The manifest screen is not shown even in interactive mode if all the manifests are hidde.

* Comply the code with Python 3.5.

* Adjust tests by putting them in a proper test classes + comments. Reduce code duplication.

* Rename ManifestBrowser.get_flattened_values, apply the same fix for _save_manifest in RemoteController, make tests more readable.
Add new chassis type for graphic, monitor and power management test
cases
Use python3-evdev to generate random input events (mouse movements, keyboard typing) in order to detect potential issues during screen rotation testing.

Fix OEMQA-5102
Fix lp:2045249
… tests (New) (#1534)

WWAN tests can now be executed twice. This update is developed following the discovery of a failure during enablement where the WWAN modem would only work once after a fresh boot.
revise the description of regulator manifest

revise the description of regulator manifest
…VSCode + gitignore venv folder (Infra)

* Add vscode env for better dev experience working with checkbox using VSCode.

* Prevent PlainboxInitTests.test_get_origin_venv from failing under VSCode. VSCode installed as snap has already SNAP_NAME env variable defined which fails the test since plainbox.get_origin() respects SNAP_NAME more than VIRTUAL_ENV.

* Add readme for VS Code dev env + apply code review changes.
Remove check_iwlwifi_microcode_crash from SRU test plan

As discussed with HWE/Kernel team, the check_iwlwifi_microcode_crash
checks for error messages in the kernel log, but this does not
necesarily mean there is a problem with the WiFi functionality, as the
driver can fix some of these problems on the go.

This job is still interesting to have during enablement phase.

For SRU, testing the actual functionality (via the wireless connection
jobs) is enough.

Checkbox records the journal log as part of its submission, so if
wireless functionality fails, it's always possible to review if this is
due to microcode error from the journal.

Fix RTW-543
…t (BugFix) (#1677)

Use glmark2, glmark2-wayland and their es2 variants to determine whether the DUT is using hardware rendering instead of unity_support_test. It checks the output of glmark2 for a GL_RENDERER string and uses the same logic as unity_support_test to see if it's produced by a software renderer.
…tional (Bugfix) (#2049)

* Remove pcm/default.conf from ALSA_CONFIG_PATH

* Change the permission of executables to 775

* Revert "Change the permission of executables to 775"

This reverts commit c7dc1ce.

* Running alsa tests as regular user

* Add drain for alsa_test.cpp to force flushing PCM buffer to play sound
Update FWTS to V25.07.00 for series18~24 snaps

This new version contains a fix that might help with the problem
described in #1628
Align the usb storage test cases in docking test plan
* SRIOV tests and unit tests

sriov checkbox files

* Updates made to sriov.py and unit tests to increase the coverage

* Updated for black format c heck

* Updates made to address the PR comments

* Updated unit tests
Moved the checkbox jobs to networking

* Moved job file to networking

* Fixed black formatting

* Fix black formatting

* Fixed additional fomatting issues

* Removed this file as it was mistakely placed here

* Fixed black formatting again!

* Updated for PR changes

* removed try except block

* Fix flake8 issue
Remove unnecessary else statment

* Black formatting issue

* flake8 issue

* flake 8 issue

* Remove try except block from check_interface_vendor

* Updates to unit tests

* black formating fix

* Removed line for python 3.5

* Changes made to comply with Python 3.5

* pythong 3.5 issue

* python 3.5 issue

* Removed sys.exit from main

* Adding a cleanup sriov script to remove virtual adapters after
a test is run

* Fix black and flake8 formating

* Fix black formatting

* - Add has_sriov manifest entry to enable SR-IOV testing
- Only SR-IOV enabled interfaces will run the functional tests
- Add info/sriov-check_{interface} template test that check
  if SR-IOV is enabled for an interface
- Update SR-IOV LXD and LXD_VM tests to depend on
  interface-specific SR-IOV capability check
increased camera quality threshold
* refactor docking test

* Improving the test steps description

* modify the description of _purpose and _verification
…gfix) (#2069)

* - Add an option to use a value other than 10% for  --oom-avoid-bytes
  in memory stress-ng
- Add the check to set --oom-avoid-bytes 5% for RAM over 255GB

* fixed black formatting and flake8 issue
Remove pulseaudio-utils from Noble, since pipewire is the default sound server for Noble
…2054)

* Migrate docs starter-pack to extension version 1.2.0

The starter-pack has been upgraded to the extension-based version:
more tooling supports are provided and most styling configurations are
centralized in the canonical-sphinx extension.
This upgrade uses the extension v5.1.0, and the starter-pack
1.2.0
https://github.com/canonical/sphinx-docs-starter-pack/releases/tag/1.2.0

* Update Makefile to the latest version

* update conf.py to latest and clean up requirements

* update spelling wordlist and doc fix

* add release version to title

* black formatting

* update doc check workflow to use new tooling

* build readthedocs only upon docs changes

* remove unused docs scripts: metrics and update
* revised LED tests

extend LED test case to suppor multi color LED

* revised the led_sysfs_resource.py

revised the led_sysfs_resource.py

* update test job and test plan

update test job and plan
* Add $XDG_RUNTIME_DIR for alsa tests

* Inherit envvars more aggressively

The point of inheriting them from only DISPLAY having processes was to
target user processes. This prioritizes them (as we do know the
username) but still proceeds till we run out of options. Additionally
this also calculates XDG_RUNTIME_DIR and DBUS_SESSION_BUS_ADDRESS always
if it wasn't found anywhere to (try) to make the job start anyway with a
  sane-ish value

* Python 3.5 :(

* Also test the bailout mechanism

---------

Co-authored-by: liaou3 <[email protected]>
* Rename bootstrap_todo_list to start_bootstrap and use the apis in silent
bootstrap

Minor: better docstring

* Create bootstrapping API to avoid using flags directly

* Rename resume_session to prepare_resume_session

resume session doesn't really resume the session,
more work is needed to get the session fully resumed

* Also propagate the bootstrap api change to remote_assistant

* Fix unit tests

* Test new bootstrapping property

* Backward compatible type hints using comments

Python3.5 (sigh) doesn't implement pep526

* Bump REMOTE_API_VERSION

* Fix typos in the docstrings

* Fix outdated todo_list getter

* Remote assistant refactoring (new) (#2057)

* Full refactoring backported to remote_assistant.py

* Backport controller refactoring

Forgotten removed api

* Fix unit tests by renaming apis and changing whats_up response

* Refactored allowed_when and _state -> state

Decorator was moved to top level of the module, state now has a
non-private property (that logs transitions) and was changed accordingly

Minor fix: use .state and not ._state in decorator

* Renamed resume_session to prepare_resume_session

Minor: forgotten 2 state comparison to str instead of new enum

* Missing bootstrap_and_continue

* Remove outdated resume machanism

* Check that all states have a connection_strategy

* Support optional parameters of interaction

* Include test plan in session metadata

* Move functions, rename functions that continue with _continue

* Default value of resume_by_id should be empty dict

* Propagate prepare_resume_session

* Also update name in the test

* Don't finalize the session until resume is decided

This removes session finalization in all branches. It was added because
the start_new_sesison function starts a new session, so it fails on the
second iteration. The correct solution to this was not to spam finalize
everywhere but to split that function up into start and select_test_plan

(thanks me, I know you did this to me)

* Port renames and new _continue process to tests

* Also fix local changes to subcommands

* Test test plan selection function

Minor: use new continue session function

* Disable fail-fast

* Update checkbox-ng/checkbox_ng/launcher/controller.py

Co-authored-by: Fernando Bravo <[email protected]>

---------

Co-authored-by: Fernando Bravo <[email protected]>

---------

Co-authored-by: Fernando Bravo <[email protected]>
* Add sane_product key

* Move all hardcoded consts to new sane_product

* Test sane_product translator

* Also test the unknown
fixed issues related to watchdog_config_test.py
@stanley31huang stanley31huang marked this pull request as draft August 14, 2025 01:30
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.