Merge ~alanec/plainbox-provider-checkbox:cpufreq_test-excp-fix into plainbox-provider-checkbox:master

Proposed by Adrian Lane
Status: Merged
Approved by: Adrian Lane
Approved revision: 56579f1e583dd09d4cd8da9c6b8ec5e13dc61aa8
Merged at revision: 8fdcabedc6fd3ca073b385d80f8085ed3cc6c849
Proposed branch: ~alanec/plainbox-provider-checkbox:cpufreq_test-excp-fix
Merge into: plainbox-provider-checkbox:master
Diff against target: 54 lines (+9/-5)
1 file modified
bin/cpufreq_test.py (+9/-5)
Reviewer Review Type Date Requested Status
Jeff Lane  Approve
Jonathan Cave (community) Abstain
Review via email: mp+395815@code.launchpad.net

Description of the change

Minor fixup of exception handling class in bin/cpufreq_test.py. This fix will allow this class to now behave as fully intended, exiting cleanly without traceback if cpufreq scaling is non-supported.
Additionally warnings (logging.warning) are now visible in all modes, included quiet, as quiet is the default operating mode in Checkbox.

Sample output to demo if scaling via cpufreq is unsupported:

z3r0@maas ~ ❯❯❯ python3 test.py
## Warning: scaling via CpuFreq non-supported ##
z3r0@maas ~ ❯❯❯ python3 test.py -q
## Warning: scaling via CpuFreq non-supported ##
z3r0@maas ~ ❯❯❯ python3 test.py -d
## Warning: scaling via CpuFreq non-supported ##
z3r0@maas ~ ❯❯❯ python3 test.py -r
## Warning: scaling via CpuFreq non-supported ##

To post a comment you must log in.
Revision history for this message
Adrian Lane (alanec) wrote :

Above demo performed on a VM.

Revision history for this message
Jonathan Cave (jocave) wrote :

I would personally prefer the test to be skipped if scaling is not supported rather than the test run and result in a pass.

review: Abstain
Revision history for this message
Jeff Lane  (bladernr) wrote :

Lets file a new bug to address Jonathan's concern. I don't disagree, but that would be out of scope for fixing this particular bug.

SO, Approve for this with the following conditions:

1: File a new bug to add a new resource job to create something like cpufreq.supported
2: Add an arg to the cpufreq script so that we can simply check that it is supported and if so, set cpufreq.supported to True.
3: modify the test job in P-P-C to require that resource == True.

review: Approve
Revision history for this message
Jonathan Cave (jocave) wrote :

Happy with that course of action, thanks

Revision history for this message
Adrian Lane (alanec) wrote :

This sounds good to me as well. I'll start working on adding this functionality next week.
Thanks-

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/bin/cpufreq_test.py b/bin/cpufreq_test.py
2index 81ee276..ec3cf6c 100755
3--- a/bin/cpufreq_test.py
4+++ b/bin/cpufreq_test.py
5@@ -40,9 +40,11 @@ class CpuFreqTestError(Exception):
6 """Exception handling."""
7 def __init__(self, message):
8 super().__init__()
9+ # warn and exit if cpufreq scaling non-supported
10 if 'scaling_driver' in message:
11- logging.error(
12- '%s\n## Fatal: scaling via cpufeq unsupported ##')
13+ logging.warning(
14+ '## Warning: scaling via CpuFreq non-supported ##')
15+ sys.exit()
16 # exempt systems unable to change intel_pstate driver mode
17 elif 'intel_pstate/status' in message:
18 pass
19@@ -675,6 +677,8 @@ class CpuFreqCoreTest(CpuFreqTest):
20 for idx, freq in enumerate(self.scaling_freqs):
21 # re-init some attributes after 1st pass
22 if idx:
23+ # time buffer ensure all prior freq intervals processed
24+ time.sleep(1)
25 # reset freq list
26 self.__observed_freqs = []
27 # reset signal.signal() event loop bit
28@@ -685,7 +689,7 @@ class CpuFreqCoreTest(CpuFreqTest):
29 load_observe_map(freq)
30
31
32-def parse_args_logging():
33+def parse_arg_logging():
34 """ Ingest arguments and init logging."""
35 def init_logging(_user_arg):
36 """ Pass user arg and configure logging module."""
37@@ -732,7 +736,7 @@ def parse_args_logging():
38 '-q', '-Q', '--quiet',
39 dest='log_level',
40 action='store_const',
41- # repurpose built-in logging level
42+ # allow visible warnings in quiet mode
43 const=logging.WARNING,
44 help='suppress output')
45 parser_mutex_grp.add_argument(
46@@ -747,7 +751,7 @@ def parse_args_logging():
47
48 def main():
49 # configure and start logging
50- user_arg = parse_args_logging()
51+ user_arg = parse_arg_logging()
52 # instantiate CpuFreqTest as cpu_freq_test
53 cpu_freq_test = CpuFreqTest()
54 # provide access to reset() method

Subscribers

People subscribed via source and target branches