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

Automate wake-on-LAN tests (New) #1686

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Conversation

eugene-yujinwu
Copy link
Contributor

Automate wake-on-LAN tests (New)

Description

We would like to automate the wake-on-LAN test instead of running manual tests for it.
Please refer to wake-on-LAN-automatic-tests.md in the commit for more details

Resolved issues

Make the Wake-on-LAN test from manual to automatic.

Documentation

wake-on-LAN-automatic-tests.md

Tests

"[wol_S3_auto] Both success": https://certification.canonical.com/submissions/status/300209

"[wol_S3_auto] Cannot connect to the WOL server" : https://certification.canonical.com/submissions/status/300211

"[wol_S3_auto] One of the WOL disabled" : https://certification.canonical.com/submissions/status/300210

"[wol_auto_test] One of NIC has wake-on-LAN issue" : https://certification.canonical.com/submissions/status/300212

"[wake-on-Lan] test for one NIC": https://certification.canonical.com/hardware/202406-34088/submission/408713/

please refer to wake-on-LAN-automatic-tests.md for more details

Co-authored-by: hanhsuan <[email protected]>
@eugene-yujinwu eugene-yujinwu requested a review from pieqq January 21, 2025 02:52
Copy link

codecov bot commented Jan 21, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 48.99%. Comparing base (b3b62f3) to head (abdf0d4).
Report is 3 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1686   +/-   ##
=======================================
  Coverage   48.99%   48.99%           
=======================================
  Files         372      372           
  Lines       40321    40321           
  Branches     6811     6811           
=======================================
  Hits        19757    19757           
  Misses      19842    19842           
  Partials      722      722           
Flag Coverage Δ
provider-certification-client 57.14% <ø> (ø)

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.

imports: from com.canonical.plainbox import manifest
requires:
manifest.has_ethernet_adapter == 'True'
manifest.has_ethernet_wake_on_lan_support== 'True'
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would suggest we have a manifest about the WoL server has been setup or not.

Copy link
Collaborator

Choose a reason for hiding this comment

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

one more issue is that we uses the RTC to wakeup the system in case the WoL is not working, shall we also add a RTC wakeup manifest as well?

Comment on lines +124 to +125
test_start_time = float(get_timestamp(timestamp_file))
actual_start_time = datetime.datetime.fromtimestamp(test_start_time)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would suggest we move this part (convert the float variable to timestamp) to the get_timestamp function.
as it is the function to read the timestamp from specific file.

Comment on lines +130 to +131
system_back_time = float(get_suspend_boot_time(powertype))
actual_back_time = datetime.datetime.fromtimestamp(system_back_time)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I have some concerns for this part.
first, I would suggest we seperate the get_suspend_boot_time into two functions.
e.g.

    command = ["journalctl", "-b", "0", "--output=short-unix"]
    result = subprocess.check_output(
        command, shell=False, universal_newlines=True
    )
    logs = result.splitlines()
    get_up_timestamp = get_wakeup_timestamp if powertype == "s3" else get_first_boot_timestamp
    actual_back_time = get_up_timestamp(logs)

second, move the part that convert float variable into timestamp into get_wakeup_timestamp and get_first_boot_timestamp.
third, in current implementation, there's a risk that the scripts would crashed (get_suspend_boot_time would return None) when no wakupe message been found in get_suspend_boot_time.

Comment on lines +269 to +272
bring_up_system("rtc", delay * retry * 2)
logging.debug(
"set the rtcwake time: {} seconds ".format(delay * retry * 2)
)
Copy link
Collaborator

Choose a reason for hiding this comment

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

the scripts would raise SystemExit if configure the RTC wake failed, but it's a bit weird to me that we don't test WoL because of RTC wake is not working.

Comment on lines +22 to +27
from wol_server import (
tasker_main,
send_wol_command,
is_pingable,
run_task,
)
Copy link
Contributor

Choose a reason for hiding this comment

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

Since fastapi is imported at the top level inside wol_server, the patch needs to happen before the import. See test_gnome_monitor.py for an example.

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.

3 participants