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 warnings & update some style (dependency injection/type hints). #77989

Merged
merged 6 commits into from
Sep 12, 2024

Conversation

ithinuel
Copy link
Collaborator

@ithinuel ithinuel commented Sep 4, 2024

These change allow ZEPHYR_BASE=$(pwd) PYTHON_PATH=./scripts/tests pytest ./scripts/tests/twister to run locally from zephyr's directory to pass and do so without warning.

Please refer to each commit messages for a short explanation of the related change.

I cannot get ZEPHYR_BASE=$(pwd) PYTHON_PATH=./scripts/tests pytest ./scripts/tests/twister_blackbox to run locally but I lack the time to investigate what am I doing wrong.
But it passes in CI :)

@ithinuel
Copy link
Collaborator Author

@nashif Thank you for the thumb up.
The PR now passes CI. Would you mind reviewing it?
Feel free to ping whomever you think would be a good person to review this too.

Space separated lists are deprecated but this notice is not checked for.
extract_fields_from_arg_list also converts lists back to space-separated
lists causing a warning on get_scenario

Signed-off-by: Wilfried Chauveau <wilfried.chauveau@arm.com>
nashif
nashif previously approved these changes Sep 11, 2024
@@ -36,7 +36,7 @@ def extract_fields_from_arg_list(target_fields: set, arg_list: Union[str, list])
# Move to other_fields
other_fields.append(field)

return extracted_fields, " ".join(other_fields)
return extracted_fields, other_fields
Copy link
Member

Choose a reason for hiding this comment

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

i suspect get_scenario() needs modifications too, to align with the return type change here.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

As far as I can tell, extra_args is expected to be a list (in yaml) so returning it as a list seems ok to me.
I coulding find other places than test_twister that check for the content of this parameter explicitly. I may have missed some relevant location, could you point me to some of them please?

scripts/pylib/twister/twisterlib/twister_main.py Outdated Show resolved Hide resolved
scripts/pylib/twister/twisterlib/handlers.py Outdated Show resolved Hide resolved
scripts/pylib/twister/twisterlib/harness.py Outdated Show resolved Hide resolved
scripts/tests/twister/test_twister.py Outdated Show resolved Hide resolved
teburd
teburd previously approved these changes Sep 11, 2024
CONF_FILE, DTC_OVERLAY_FILE and OVERLAY_CONFIG are deprecated but still
used by the tests causing warnings when running them.

This adds a test_data specific to validate the emission of the warning,
and removes the offending args from the other test_data files.

Signed-off-by: Wilfried Chauveau <wilfried.chauveau@arm.com>
Python 3.12 warns that

> Testing an element's truth value will raise an exception in future
> versions. Use specific 'len(elem)' or 'elem is not None' test instead.
>    if elem_ts := root.find('testsuite'):

Signed-off-by: Wilfried Chauveau <wilfried.chauveau@arm.com>
Adjusting the type hints and code accordingly.

Signed-off-by: Wilfried Chauveau <wilfried.chauveau@arm.com>
This changes how some arguments are set in the `Handler`s.
`options`, `generator_cmd` and `suite_name_check` are now passed as
arguments to the constructor rather than injected from an other module.

Signed-off-by: Wilfried Chauveau <wilfried.chauveau@arm.com>
Twister main expects an instance of TwisterEnv. Let's the linter know
about this.

Signed-off-by: Wilfried Chauveau <wilfried.chauveau@arm.com>
@carlescufi carlescufi merged commit a55bb95 into zephyrproject-rtos:main Sep 12, 2024
29 checks passed
@ithinuel ithinuel deleted the twister-pytests branch September 13, 2024 14:36
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.

7 participants