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

scripts: twister: Fix NOTRUN in test_only #80221

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

LukaszMrugala
Copy link
Collaborator

When using the --build-only into --test-only Twister setup, NOTRUN statuses were not properly rerun.

Now they are properly run again if runnable.

Fixes incorrect test statistics in the --test-only summary.
Adds status and reason fields for all testsuites of the JSON report. (This seems to break footprint tests. WIP.)

@LukaszMrugala LukaszMrugala added the In progress For PRs: is work in progress and should not be merged yet. For issues: Is being worked on label Oct 22, 2024
@LukaszMrugala LukaszMrugala force-pushed the lmrugalx_notrun_buildonly_testonly branch from e2123de to b6d9d6a Compare October 22, 2024 14:01
@LukaszMrugala
Copy link
Collaborator Author

The issue was raised in #80120

@hakehuang
Copy link
Collaborator

The issue was raised in #80120
@LukaszMrugala
I revert the changes that introduce isues, and revert to my original version which does not introduce new problem and resolves #80119, so do you plan to provide a full solution? I can close my PR if you can catch up the freeze window.

@@ -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

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.

suite["reason"] = instance.reason
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

@LukaszMrugala LukaszMrugala force-pushed the lmrugalx_notrun_buildonly_testonly branch 5 times, most recently from cf044af to 94df9b6 Compare October 23, 2024 09:00
@LukaszMrugala LukaszMrugala added area: Twister Twister and removed In progress For PRs: is work in progress and should not be merged yet. For issues: Is being worked on labels Oct 23, 2024
@LukaszMrugala LukaszMrugala marked this pull request as ready for review October 23, 2024 10:00
@@ -618,6 +618,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

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

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.

When using the --build-only into --test-only
Twister setup, NOTRUN statuses were not properly rerun.

Now they are properly run again if runnable.

Signed-off-by: Lukasz Mrugala <[email protected]>
@LukaszMrugala LukaszMrugala force-pushed the lmrugalx_notrun_buildonly_testonly branch from 94df9b6 to d989d18 Compare October 24, 2024 14:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants