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.

Sign up for GitHub

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

Jump to bottom

scripts: twister: Fix NOTRUN in test_only #80221

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
+26 −14
Open

scripts: twister: Fix NOTRUN in test_only #80221

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions scripts/pylib/twister/twisterlib/reports.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -354,6 +354,12 @@ def json_report(self, filename, version="NA", platform=None, filters=None):
elif instance.status == TwisterStatus.SKIP:
suite["status"] = TwisterStatus.SKIP
suite["reason"] = instance.reason
elif instance.status == TwisterStatus.NOTRUN:
suite["status"] = TwisterStatus.NOTRUN
suite["reason"] = instance.reason
Copy link
Collaborator

Choose a reason for hiding this comment

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

this line need to removed to pass the test_report.py blackbox testing, as it expect to be no reason when NOTRUN

Copy link
Collaborator Author

@LukaszMrugala LukaszMrugala Oct 23, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

In my opinion, the errors were caused by an assumption in the test_footprint.py. Proper usage of the NOTRUN status has broken that assumption, so I've fixed the test.

else:
suite["status"] = TwisterStatus.NONE
suite["reason"] = 'Unknown Instance status.'
Copy link
Collaborator

Choose a reason for hiding this comment

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

same as above


if instance.status != TwisterStatus.NONE:
suite["execution_time"] = f"{float(handler_time):.2f}"
Expand Down
9 changes: 6 additions & 3 deletions scripts/pylib/twister/twisterlib/testplan.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -652,6 +652,9 @@ def load_from_file(self, file, filter_platform=[]):
self.hwm
)

if self.options.test_only and not instance.run:
Copy link
Member

Choose a reason for hiding this comment

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

hmm, what is the reason for this ?
it seems that now for test_only the resulting report will have these test suites with empty testcases[] ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

seems ok, this is my test result.

{
    "environment":{
        "os":"posix",
        "zephyr_version":"v3.7.0-4864-g94df9b68416c",
        "toolchain":"zephyr",
        "commit_date":"2024-10-23T09:00:05+00:00",
        "run_date":"2024-10-23T22:46:13+00:00",
        "options":{
            "device_testing":true,
            "hardware_map":"/home/ubuntu/nxp/frdm_k64f/map.yaml",
            "test_only":true,
            "platform":[
                "frdm_k64f"
            ],
            "west_flash":[]
        }
    },
    "testsuites":[
        {
            "name":"samples/hello_world/sample.basic.helloworld",
            "arch":"arm",
            "platform":"frdm_k64f",
            "path":"samples/hello_world",
            "run_id":"81b66401fdba63c34f93d328a90dbc85",
            "runnable":true,
            "retries":0,
            "dut":"0240000047784e4500229003d917001be561000097969900",
            "status":"passed",
            "execution_time":"4.85",
            "build_time":"0.00",
            "testcases":[
                {
                    "identifier":"sample.basic.helloworld",
                    "execution_time":"4.85",
                    "status":"passed"
                }
            ]
        }
    ]
}

Copy link
Member

Choose a reason for hiding this comment

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

I think this PR needs rebase to include recent changes done #77250 - looks like there is a regression introduced there for --test-only
I've filed #80332 for that.

Copy link
Collaborator

@hakehuang hakehuang Oct 24, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

according to the fix logic it is platform naming issue, which is not related to test status.
and there is no issue for this pr

ubuntu@ubuntu-OptiPlex-7050:/home/shared/disk/zephyr_project/zephyr_test/zephyr$ ./scripts/twister -p qemu_x86 -T tests/benchmarks/latency_measure -b
ZEPHYR_BASE unset, using "/home/shared/disk/zephyr_project/zephyr_test/zephyr"
Renaming output directory to /home/shared/disk/zephyr_project/zephyr_test/zephyr/twister-out.4
INFO    - Using Ninja..
INFO    - Zephyr version: v3.7.0-4864-g94df9b68416c
INFO    - Using 'zephyr' toolchain.
INFO    - Building initial testsuite list...
INFO    - Writing JSON report /home/shared/disk/zephyr_project/zephyr_test/zephyr/twister-out/testplan.json
INFO    - JOBS: 16
INFO    - Adding tasks to the queue...
INFO    - Added initial list of jobs to queue
INFO    - Total complete:    3/   3  100%  built (not run):    2, skipped:    1, failed:    0, error:    0
INFO    - 3 test scenarios (3 test instances) selected, 1 configurations skipped (1 by static filter, 0 at runtime).
INFO    - 0 of 3 test configurations passed (0.00%), 2 built (not run), 0 failed, 0 errored, 1 skipped with 0 warnings in 102.50 seconds
INFO    - 0 test configurations executed on platforms, 2 test configurations were only built.
INFO    - Saving reports...
INFO    - Writing JSON report /home/shared/disk/zephyr_project/zephyr_test/zephyr/twister-out/twister.json
INFO    - Writing xunit report /home/shared/disk/zephyr_project/zephyr_test/zephyr/twister-out/twister.xml...
INFO    - Writing xunit report /home/shared/disk/zephyr_project/zephyr_test/zephyr/twister-out/twister_report.xml...
INFO    - Run completed
ubuntu@ubuntu-OptiPlex-7050:/home/shared/disk/zephyr_project/zephyr_test/zephyr$  ./scripts/twister -p qemu_x86 -T tests/benchmarks/latency_measure --test-only
ZEPHYR_BASE unset, using "/home/shared/disk/zephyr_project/zephyr_test/zephyr"
Keeping artifacts untouched
INFO    - Using Ninja..
INFO    - Zephyr version: v3.7.0-4864-g94df9b68416c
INFO    - Using 'zephyr' toolchain.
INFO    - JOBS: 8
INFO    - Adding tasks to the queue...
INFO    - Added initial list of jobs to queue
INFO    - Total complete:    2/   2  100%  built (not run):    0, skipped:    0, failed:    0, error:    0
INFO    - 3 test scenarios (2 test instances) selected, 0 configurations skipped (0 by static filter, 0 at runtime).
INFO    - 2 of 2 test configurations passed (100.00%), 0 built (not run), 0 failed, 0 errored, 0 skipped with 0 warnings in 24.23 seconds
INFO    - In total 2 test cases were executed, 0 skipped on 0 out of total 10 platforms (0.00%)
INFO    - 2 test configurations executed on platforms, 0 test configurations were only built.
INFO    - Saving reports...
INFO    - Writing JSON report /home/shared/disk/zephyr_project/zephyr_test/zephyr/twister-out/twister.json
INFO    - Writing xunit report /home/shared/disk/zephyr_project/zephyr_test/zephyr/twister-out/twister.xml...
INFO    - Writing xunit report /home/shared/disk/zephyr_project/zephyr_test/zephyr/twister-out/twister_report.xml...
INFO    - Run completed

and rebase to latest it fails.
and if I run with full name it is OK in this PR with rebase to latest code, so those are two issues, and can be treated separately. @golowanow @LukaszMrugala

ubuntu@ubuntu-OptiPlex-7050:/home/shared/disk/zephyr_project/zephyr_test/zephyr$ ./scripts/twister -p qemu_x86/atom -T tests/benchmarks/latency_measure -b
ZEPHYR_BASE unset, using "/home/shared/disk/zephyr_project/zephyr_test/zephyr"
Renaming output directory to /home/shared/disk/zephyr_project/zephyr_test/zephyr/twister-out.7
INFO    - Using Ninja..
INFO    - Zephyr version: v3.7.0-5112-g55ea6a8fe290
INFO    - Using 'zephyr' toolchain.
INFO    - Building initial testsuite list...
INFO    - Writing JSON report /home/shared/disk/zephyr_project/zephyr_test/zephyr/twister-out/testplan.json
INFO    - JOBS: 16
INFO    - Adding tasks to the queue...
INFO    - Added initial list of jobs to queue
INFO    - Total complete:    3/   3  100%  built (not run):    2, skipped:    1, failed:    0, error:    0
INFO    - 3 test scenarios (3 test instances) selected, 1 configurations skipped (1 by static filter, 0 at runtime).
INFO    - 0 of 3 test configurations passed (0.00%), 2 built (not run), 0 failed, 0 errored, 1 skipped with 0 warnings in 18.83 seconds
INFO    - 0 test configurations executed on platforms, 2 test configurations were only built.
INFO    - Saving reports...
INFO    - Writing JSON report /home/shared/disk/zephyr_project/zephyr_test/zephyr/twister-out/twister.json
INFO    - Writing xunit report /home/shared/disk/zephyr_project/zephyr_test/zephyr/twister-out/twister.xml...
INFO    - Writing xunit report /home/shared/disk/zephyr_project/zephyr_test/zephyr/twister-out/twister_report.xml...
INFO    - Run completed
ubuntu@ubuntu-OptiPlex-7050:/home/shared/disk/zephyr_project/zephyr_test/zephyr$ ./scripts/twister -p qemu_x86/atom -T tests/benchmarks/latency_measure --test-only
ZEPHYR_BASE unset, using "/home/shared/disk/zephyr_project/zephyr_test/zephyr"
Keeping artifacts untouched
INFO    - Using Ninja..
INFO    - Zephyr version: v3.7.0-5112-g55ea6a8fe290
INFO    - Using 'zephyr' toolchain.
INFO    - JOBS: 8
INFO    - Adding tasks to the queue...
INFO    - Added initial list of jobs to queue
INFO    - Total complete:    2/   2  100%  built (not run):    0, skipped:    0, failed:    0, error:    0
INFO    - 3 test scenarios (2 test instances) selected, 0 configurations skipped (0 by static filter, 0 at runtime).
INFO    - 2 of 2 test configurations passed (100.00%), 0 built (not run), 0 failed, 0 errored, 0 skipped with 0 warnings in 16.56 seconds
INFO    - In total 2 test cases were executed, 0 skipped on 0 out of total 850 platforms (0.00%)
INFO    - 2 test configurations executed on platforms, 0 test configurations were only built.
INFO    - Saving reports...
INFO    - Writing JSON report /home/shared/disk/zephyr_project/zephyr_test/zephyr/twister-out/twister.json
INFO    - Writing xunit report /home/shared/disk/zephyr_project/zephyr_test/zephyr/twister-out/twister.xml...
INFO    - Writing xunit report /home/shared/disk/zephyr_project/zephyr_test/zephyr/twister-out/twister_report.xml...
INFO    - Run completed

and apply #80346 also works for short name, but there is CI issue need fix

Copy link
Collaborator Author

@LukaszMrugala LukaszMrugala Oct 24, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

hmm, what is the reason for this ? it seems that now for test_only the resulting report will have these test suites with empty testcases[] ?

This will remove unrunnable test instances from the --test-only run. Previously, we've had irrelevant suites reported in the ending summary, which is incorrect.

To illustrate: Imagine that a --build-only run contains two test instances. Both are built and achieve NOTRUN status. Then, we use Twister in --test-only mode. One testcase is runnable and gets a PASS. Second one is still not runnable.

Previously, the second instance would be read and included in the report as a NOTRUN. This is inconsistent with the --test-only's behaviour when it comes to SKIP suites - those are discarded at the start and not included. Now, the unrunnable NOTRUN suite will be discarded as well. This is good, because the suite will not be processed either way, so including it in the report is erroneous.

I'm currently checking rebase locally.

Copy link
Member

Choose a reason for hiding this comment

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

Previously, the second instance would be read and included in the report as a NOTRUN. This is inconsistent with the --test-only's behaviour when it comes to SKIP suites - those are discarded at the start and not included.
Now, the unrunnable NOTRUN suite will be discarded as well. This is good, because the suite will not be processed either way, so including it in the report is erroneous.

I'm not sure it is good. In build-only -> test-only mode the twister.json is overwritten, so skipped non-runnable will disappear with their built status. Also, with --detailed-skipped-report option the 'SKIP'-ed should be included, whereas NOTRUN-ed now omitted.

continue

instance.metrics['handler_time'] = ts.get('execution_time', 0)
instance.metrics['used_ram'] = ts.get("used_ram", 0)
instance.metrics['used_rom'] = ts.get("used_rom",0)
Expand All @@ -669,9 +672,9 @@ def load_from_file(self, file, filter_platform=[]):
instance.status = TwisterStatus.NONE
instance.reason = None
instance.retries += 1
# test marked as passed (built only) but can run when
# --test-only is used. Reset status to capture new results.
elif status == TwisterStatus.PASS and instance.run and self.options.test_only:
# test marked as built only can run when --test-only is used.
# Reset status to capture new results.
elif status == TwisterStatus.NOTRUN and instance.run and self.options.test_only:
instance.status = TwisterStatus.NONE
instance.reason = None
else:
Expand Down
3 changes: 2 additions & 1 deletion scripts/tests/twister/test_testplan.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -766,6 +766,7 @@ def test_testplan_load(
testplan.apply_filters = mock.Mock()

with mock.patch('twisterlib.testinstance.TestInstance.create_overlay', mock.Mock()), \
mock.patch('twisterlib.testinstance.TestInstance.check_runnable', return_value=True), \
pytest.raises(exception) if exception else nullcontext():
testplan.load()

Expand Down Expand Up @@ -1600,7 +1601,7 @@ def get_platform(name):
'testcases': {
'TS1.tc1': {
'status': TwisterStatus.PASS,
'reason': None,
'reason': 'passed',
'duration': 60.0,
'output': ''
}
Expand Down
16 changes: 9 additions & 7 deletions scripts/tests/twister_blackbox/test_footprint.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

# pylint: disable=no-name-in-module
from conftest import ZEPHYR_BASE, TEST_DATA, testsuite_filename_mock, clear_log_in_test
from twisterlib.statuses import TwisterStatus
from twisterlib.testplan import TestPlan


Expand Down Expand Up @@ -76,9 +77,10 @@ def test_compare_report(self, caplog, out_path, old_ram_multiplier, expect_delta
with open(os.path.join(out_path, 'twister.json')) as f:
j = json.load(f)
for ts in j['testsuites']:
if 'reason' not in ts:
if TwisterStatus(ts.get('status')) == TwisterStatus.NOTRUN:
# We assume positive RAM usage.
ts[self.RAM_KEY] *= old_ram_multiplier

with open(os.path.join(out_path, 'twister.json'), 'w') as f:
f.write(json.dumps(j, indent=4))

Expand Down Expand Up @@ -137,7 +139,7 @@ def test_footprint_from_buildlog(self, out_path):
with open(os.path.join(out_path, 'twister.json')) as f:
j = json.load(f)
for ts in j['testsuites']:
if 'reason' not in ts:
if TwisterStatus(ts.get('status')) == TwisterStatus.NOTRUN:
assert self.RAM_KEY in ts
old_values += [ts[self.RAM_KEY]]

Expand All @@ -162,7 +164,7 @@ def test_footprint_from_buildlog(self, out_path):
with open(os.path.join(out_path, 'twister.json')) as f:
j = json.load(f)
for ts in j['testsuites']:
if 'reason' not in ts:
if TwisterStatus(ts.get('status')) == TwisterStatus.NOTRUN:
assert self.RAM_KEY in ts
new_values += [ts[self.RAM_KEY]]

Expand Down Expand Up @@ -202,7 +204,7 @@ def test_footprint_threshold(self, caplog, out_path, old_ram_multiplier,
with open(os.path.join(out_path, 'twister.json')) as f:
j = json.load(f)
for ts in j['testsuites']:
if 'reason' not in ts:
if TwisterStatus(ts.get('status')) == TwisterStatus.NOTRUN:
# We assume positive RAM usage.
ts[self.RAM_KEY] *= old_ram_multiplier
with open(os.path.join(out_path, 'twister.json'), 'w') as f:
Expand Down Expand Up @@ -271,7 +273,7 @@ def test_show_footprint(self, caplog, out_path, flags, old_ram_multiplier, expec
with open(os.path.join(out_path, 'twister.json')) as f:
j = json.load(f)
for ts in j['testsuites']:
if 'reason' not in ts:
if TwisterStatus(ts.get('status')) == TwisterStatus.NOTRUN:
# We assume positive RAM usage.
ts[self.RAM_KEY] *= old_ram_multiplier
with open(os.path.join(out_path, 'twister.json'), 'w') as f:
Expand Down Expand Up @@ -344,7 +346,7 @@ def test_last_metrics(self, caplog, out_path, old_ram_multiplier, expect_delta_l
with open(os.path.join(out_path, 'twister.json')) as f:
j = json.load(f)
for ts in j['testsuites']:
if 'reason' not in ts:
if TwisterStatus(ts.get('status')) == TwisterStatus.NOTRUN:
# We assume positive RAM usage.
ts[self.RAM_KEY] *= old_ram_multiplier
with open(os.path.join(out_path, 'twister.json'), 'w') as f:
Expand Down Expand Up @@ -441,7 +443,7 @@ def test_all_deltas(self, caplog, out_path, old_ram_multiplier, expect_delta_log
with open(os.path.join(out_path, 'twister.json')) as f:
j = json.load(f)
for ts in j['testsuites']:
if 'reason' not in ts:
if TwisterStatus(ts.get('status')) == TwisterStatus.NOTRUN:
# We assume positive RAM usage.
ts[self.RAM_KEY] *= old_ram_multiplier
with open(os.path.join(out_path, 'twister.json'), 'w') as f:
Expand Down
6 changes: 3 additions & 3 deletions scripts/tests/twister_blackbox/test_runner.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,19 +46,19 @@ class TestRunner:
['qemu_x86/atom', 'qemu_x86_64/atom', 'intel_adl_crb/alder_lake'],
{
'selected_test_scenarios': 3,
'selected_test_instances': 6,
'selected_test_instances': 4,
'skipped_configurations': 0,
'skipped_by_static_filter': 0,
'skipped_at_runtime': 0,
'passed_configurations': 4,
'built_configurations': 2,
'built_configurations': 0,
'failed_configurations': 0,
'errored_configurations': 0,
'executed_test_cases': 8,
'skipped_test_cases': 0,
'platform_count': 0,
'executed_on_platform': 4,
'only_built': 2
'only_built': 0
}
)
]
Expand Down
Loading