trunner: Implementing pytest support by lukkrusz · Pull Request #445 · phoenix-rtos/phoenix-rtos-tests

lukkrusz · 2026-01-19T15:22:33Z

PyTest support implementation for our existing testing framework.

Description

The implementation includes

pytest harness
pytest plugins:
- plugin for IO Stream control
- plugin bridging PyTest with our TestRunner (report)
sample pytest tests with conftest.py
TestType Enum for simpler and more controllable input validation

Motivation and Context

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)
Chore (refactoring, style fixes, git/CI config, submodule management, no code logic changes)

How Has This Been Tested?

Already covered by automatic testing.
New test added: (add PR link here).
Tested by hand on: (list targets here).

Checklist:

My change requires a change to the documentation.
I have updated the documentation accordingly.
I have added tests to cover my changes.
All new and existing linter checks and tests passed.
My changes generate no new compilation warnings for any of the targets.

Special treatment

This PR needs additional PRs to work (#449).
I will merge this PR by myself when appropriate.

Previous conversation: #443

gemini-code-assist · 2026-01-19T15:22:55Z

Summary of Changes

Hello @lukkrusz, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request significantly extends the testing capabilities of the trunner framework by integrating Pytest. It provides a dedicated Pytest harness that allows users to define and execute tests using the popular Pytest framework, with results seamlessly reported back to the TestRunner. The changes include new plugins to manage Pytest's output and map its test outcomes to the framework's subtest structure, alongside a new TestType enum for improved test configuration and validation.

Highlights

Pytest Integration: Introduces full support for Pytest within the existing testing framework, enabling the use of Pytest for test execution.
Custom Pytest Plugins: Implements two key Pytest plugins: one for controlling and capturing I/O stream output, and another for bridging Pytest's detailed test reports with the framework's TestRunner subresult system.
TestType Enum: Adds a new TestType Enum to trunner.types.py for clearer and more robust input validation and configuration of different test types (e.g., HARNESS, PYTEST, UNITY).
Sample Pytest Tests: Includes example Pytest tests and a conftest.py file to demonstrate how to write and configure Pytest tests within the framework.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	`/gemini review`	Performs a code review for the current pull request in its current state.
Pull Request Summary	`/gemini summary`	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	`/gemini help`	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

gemini-code-assist

Code Review

This pull request introduces pytest support, which is a great addition to the testing framework. The implementation is solid, with good use of pytest plugins to bridge with the existing TestRunner and to control output. The new TestType enum improves the configuration parsing by making it more robust and readable.

I've left a few comments with suggestions for improvement:

In the sample test file, there's a fragile test that depends on execution order. I've suggested how to make it more robust.
The error message for unknown test types in the configuration could be more descriptive.
Most importantly, for failed pytest tests, the failure details were not being captured. I've suggested a change to include these details in the test report, which will be very helpful for debugging.
There is some code duplication in the sample conftest.py file which could be refactored.

sample/pytest/test-pytest.py

trunner/harness/pytest.py

sample/pytest/conftest.py

trunner/config.py

github-actions · 2026-01-19T15:38:11Z

Unit Test Results

9 748 tests +223 9 136 ✅ +203 53m 29s ⏱️ +48s
602 suites + 19 612 💤 + 20
1 files ± 0 0 ❌ ± 0

Results for commit 3159a4b. ± Comparison against base commit 4b75909.

♻️ This comment has been updated with latest results.

mateusz-bloch

Only quick look.

I’m not sure what the expected behaviour were, but wouldn’t it be better to parse specific failures per test case? For example if there were hundreds of test cases, this could be more readable, and we would still have the full output available via -S. @damianloew

mateusz-bloch · 2026-01-19T15:36:30Z

trunner/harness/pytest.py

+        result.status = Status.FAIL
+    elif all(sub.status == Status.SKIP for sub in result.subresults) and result.subresults:
+        result.status = Status.SKIP
+    elif not result.subresults and exit_code != 0:


Hmmm, there is a case where a test may return a non-zero exit code and we would still treat it as OK. For example, when a cleanup fixture fails, pytest usually reports an ERROR. From what I can see, this is not being parsed either.

Example:

While I'm on it, in a case when the teardown fails, do we want to treat the entire case as FAIL or just the ultimate test result?

One could argue that the test case passed but it was a post-test operation that failed.
As is now, in case of such a failure, the full pytest trace is being released like on the provided screenshot

The decision has been made to treat setup/teardown ERROR as the entire test-case FAIL.

trunner/harness/pytest.py

mateusz-bloch · 2026-01-19T15:50:01Z

trunner/harness/pytest.py

+        *options,
+        "-v",
+        "-s",
+        "--tb=no",


Do we need to disable the traceback?

The traceback suppression can shorten the logs, thus improving the readability when we hit many errors during a test run but I could change it to default and support a kwarg to set it in test yamls. What do you think, @damianloew ?

I agree that in case of failure we should provide the traceback output next to the result of the particular test case. In the current approach there can be situation where the error message is not sufficient - when I simply removed dut fixture there is no information about that:

Referring to @mateusz-bloch comment about parsing specific failures per test case - I agree on that. We should provide the solution where we have only the message about the failure, not the whole pytest output when we have fail. Now the output looks very similar to this with streaming option when we have at least one fail.

Our python tests in phoenix-rtos-tests repo are outdated a bit, but after quickly rewriting one test case to the new approach with adding subresults the behavior in case of failure would be like that:

We want to have the similar behavior in pytest tests - traceback/longer error message printed next to the test case result (without the whole streaming output when this option is not set). When we have streaming option active I don't see any contraindications to not show the traceback 2 times - in streaming output and then next to the results.

Sorry about misleading screenshot - of course the overall result should be FAIL.

damianloew

First suggestions, just to order the changes first

trunner/harness/pytest.py

nalajcie

Pointing out some issues in the current code. Please note that the commit list is too verbose, the final history should not include fixes to your own code (added within the same PR) - I would expect 2-3 commits in the final history.

Adding separate commits after review might be beneficial, for tracking review progress, but they should be squashed sooner than later.

trunner/harness/pytest.py

nalajcie · 2026-01-22T11:51:45Z

trunner/harness/pytest.py

+    def get_logs(self) -> str:
+        """Get the full, unsuppressed pytest log
+        """
+        return self.buffer.getvalue()


does it work if we're streaming? (we're attaching the buffer only if _suppress == true

It's indeed a dead functionality.

We are removing the log retention since the important to us bits are being parsed to trunner anyway.
I will also make use of trunner logging for this purpose and modify the pytest output to fit our needs.

ok, the test stdout will still be caught by DUT wrapping? (I'm wondering what will finally be available in JUnit XML output)

I'm not sure if I follow. JUnit XML will contain testsuites, testcases, fail/skip messages followed by system-out traceback (in case of failure).

Each testcase result is being handled before the terminal output, internally.
The expected terminal output, as established with @damianloew, would be a mix of output from DUT and test stdout, it would look like:
/sample/test/pytest.py::test_send_hello...
[DUT] Hello
/sample/test/pytest.py::test_send_hello PASS

trunner/harness/pytest.py

nalajcie · 2026-01-22T14:25:58Z

trunner/config.py

        self.test.harness = PyHarness(self.ctx.target.dut, self.ctx, unity_harness, self.test.kwargs)

+    def _parse_pytest(self):
+        self.test.shell = None


why overwriting shell? If the shell must not be set, then the parser should probably fail when the key is present?

can't we have shell for pytest-based tests?

We established that for now we keep the assumption that all pytest tests won't use the shell. The current pytest tests that we want to introduce into the trunner do not interact with the system shell, and the primary goal now is to run them in the unified environment.

If we planned to write tests that interact with our system using pytest, we would extend this support. Tt would probably be a nice idea, but we want to carry out this process gradually.

About failing - now test.shell is always being set - depending on config in yaml ShellOptions with additional arguments or not is being set (always we want to parse prompt). I think that we can keep this approach and just treat pytest tests separately. That's why we cannot raise fail in such case.

However, @lukkrusz we should leave a comment noting that it's temporary - ultimately I'd require to set shell: False or sth similar in test.yaml for tests, which don't interact with a system shell.

When you do some design decision (like "interacting with shell via pytest is not possible") you need to write them down (at least in the code) - otherwise it looks like a bug / hack / unfinished implementation.
I feel that this is done only to make running tests on host-generic-pc easier, having shell on other targets won't complicate running pytest-based tests at all, so probably this assumption can be removed in the future (I might be wrong, as I've only read the code).

I would appreciate adding at least one real test which actually interacts with the DUT in CI, as I'm not sure what's the intended use case.

I see that @lukkrusz left the following comment elsewhere:

# Specific case where we do not need the shell command # TODO: In the future, we could support shell communication

However, we can also clarify that in the commit description and maybe express it in a better way.

The main reason of this support now is providing the possibility to write or run the existent functional tests using pytest in our internal project, where we don't interact with a system console - to have the ability to run them even if we don't have a port with this console available (release fw).

You've quoted the Host test runner implementation, so I consider it a limitation of this target, not an overall requirement. Overwriting self.test.shell = None while parsing a config (generic code) without any comment is a bad code pattern.

So we want to accept the scenario where we don't have access to the system console - empty DUT like on host target, but also the situation where dut device is available and we can parse some additional information there.

We will try to show it somehow in a test.

We discussed a new approach and I hope that setting shell to None and changing Host target won't be needed then. Thanks for the suggestions!

trunner/target/host.py

sample/pytest/test.yaml

sample/pytest/conftest.py

trunner/types.py

sample/pytest/main.c

damianloew

Suggestions for the code before the recent force push. I haven't seen much unnecessary code, which is good - changes are quite clear for now

trunner/types.py

trunner/harness/pytest.py

damianloew · 2026-01-26T12:38:41Z

trunner/harness/pytest.py

+
+def pytest_harness(dut: Dut, ctx: TestContext, result: TestResult, **kwargs) -> TestResult:
+    test_path = ctx.project_path / kwargs.get("path")
+    options = kwargs.get("options", "").split()


It seems that we won't want to change pytest options per test (all of the command line options are rather for the whole pytest campaign configuration), so probably we can remove that.

After some thinking, we can bring it back - it's helpful when we want to add a custom argument for a test - for example to skip initialization phase, which is not always necessary.

trunner/harness/pytest.py

damianloew · 2026-01-27T12:16:19Z

trunner/harness/pytest.py

+
+    try:
+        exit_code = pytest.main(cmd_args, plugins=[bridge_plugin, log_plugin])
+    except Exception as e:


Let's handle various exception types separately - when you provided dut to be used in pytest tests - you have to be ready for failures typical for pexpect, so similarly to pyharness.

UnicodeDecodeError - should be handled

AssertionError - seems to be fine (printing traceback from pytest)

EOF, or TIMEOUT from pexpect - I am not sure how it behaves now, to be verified

HarnessError - Probably won't be raised from pytest, so we could skip that

trunner/harness/pytest.py

sample/pytest/main.c

damianloew

Looking only at tests for now

damianloew · 2026-02-06T14:04:45Z

sample/pytest/conftest.py

+
+@pytest.fixture(scope="session")
+def pexpect_bin(dut):
+    pexpect = dut.pexpect_proc


pexpect var name may be problematic, especially if there was import pexpect, let's change it a bit.

damianloew · 2026-02-06T14:23:41Z

sample/pytest/conftest.py

+HARDCODED_VERB = 2
+
+
+def _print(text: str):


We can remove this debug print - python prints are not a valid use case in our tests, so I don't see a value of leaving it. You already have shown session scope utility even without conftest_print, you can also use global_session_resource simply without additional logs.

damianloew · 2026-02-06T14:26:08Z

sample/pytest/conftest.py

+import pytest
+import trunner
+
+HARDCODED_VERB = 2


To be removed according to the comment below.

damianloew · 2026-02-06T14:33:59Z

sample/pytest/test-pytest.py

+    assert True
+
+
+@pytest.mark.skip(reason="CI CHECK")


Let's keep our format - for every skip we want to leave the issue number. As it's a demo we can do it this way:

Suggested change

@pytest.mark.skip(reason="CI CHECK")

@pytest.mark.skip(reason="#x issue")

damianloew · 2026-02-06T14:34:52Z

sample/pytest/test-pytest.py

+    assert False, "Always Fail"
+
+
+@pytest.mark.skip(reason="Should not fail because it's skipped")


I don't see the reason to leave this case - as it's almost the same as the previous one.

sample/pytest/test-pytest.py

sample/pytest/test.py

damianloew · 2026-02-06T16:24:12Z

sample/pytest/test-psh.py

+    psh.assert_cmd(
+        pexpect_psh,
+        f"echo {msg}",
+        expected=f"{resp}\r+\n",


Suggested change

expected=f"{resp}\r+\n",

expected=fr"{resp}\r+\n",

damianloew · 2026-02-06T16:55:38Z

sample/pytest/test-psh.py

+        pexpect_psh,
+        f"echo {msg}",
+        expected=f"{resp}\r+\n",
+        result="dont-check",


This argument is intended to check the return code, instead of 0 or 1 in echo_resp_code_list you can pass "success"/"fail" and pass it here. Would be simpler than marking that you don't want to check return code and then checking it on your own.

sample/pytest/test-psh.py

Includes conftest.py, yaml config, binary and Makefile JIRA: CI-614

JIRA: CI-614

lukkrusz requested review from damianloew and mateusz-bloch January 19, 2026 15:22

gemini-code-assist bot reviewed Jan 19, 2026

View reviewed changes

sample/pytest/test-pytest.py Outdated Show resolved Hide resolved

trunner/harness/pytest.py Outdated Show resolved Hide resolved

sample/pytest/conftest.py Outdated Show resolved Hide resolved

trunner/config.py Outdated Show resolved Hide resolved

mateusz-bloch reviewed Jan 19, 2026

View reviewed changes

damianloew reviewed Jan 19, 2026

View reviewed changes

trunner/harness/pytest.py Show resolved Hide resolved

trunner/harness/pytest.py Outdated Show resolved Hide resolved

lukkrusz force-pushed the lukkrusz/pytest_support_add branch 3 times, most recently from 2ead454 to 75b6ffa Compare January 22, 2026 11:39

nalajcie requested changes Jan 22, 2026

View reviewed changes

lukkrusz marked this pull request as draft January 23, 2026 15:47

github-actions bot reviewed Jan 27, 2026

View reviewed changes

damianloew requested changes Jan 27, 2026

View reviewed changes

github-actions bot reviewed Jan 29, 2026

View reviewed changes

sample/pytest/main.c Outdated Show resolved Hide resolved

sample/pytest/main.c Outdated Show resolved Hide resolved

lukkrusz force-pushed the lukkrusz/pytest_support_add branch 4 times, most recently from a09a72d to 988c5d7 Compare February 3, 2026 12:16

lukkrusz marked this pull request as ready for review February 4, 2026 09:46

lukkrusz force-pushed the lukkrusz/pytest_support_add branch from 988c5d7 to 5848076 Compare February 6, 2026 12:38

damianloew requested changes Feb 6, 2026

View reviewed changes

lukkrusz force-pushed the lukkrusz/pytest_support_add branch 8 times, most recently from aab8f8f to fc8dab1 Compare February 11, 2026 10:09

lukkrusz marked this pull request as draft February 11, 2026 14:47

lukkrusz force-pushed the lukkrusz/pytest_support_add branch 2 times, most recently from 6e6faba to ee690c8 Compare February 12, 2026 12:27

lukkrusz marked this pull request as ready for review February 12, 2026 12:42

lukkrusz requested a review from damianloew February 12, 2026 12:54

lukkrusz marked this pull request as draft February 24, 2026 14:55

lukkrusz force-pushed the lukkrusz/pytest_support_add branch 4 times, most recently from b1fb0fd to 77c859e Compare March 3, 2026 13:15

lukkrusz force-pushed the lukkrusz/pytest_support_add branch 3 times, most recently from 6cc0d0a to 8023e6d Compare March 5, 2026 14:14

lukkrusz added 3 commits March 5, 2026 15:32

sample/pytest: add sample pytest cases

8efa601

Includes conftest.py, yaml config, binary and Makefile JIRA: CI-614

trunner: add pytest harness and pytest plugins

ce0a2df

JIRA: CI-614

trunner: fix typo

3159a4b

JIRA: CI-614

lukkrusz force-pushed the lukkrusz/pytest_support_add branch from 8023e6d to 3159a4b Compare March 5, 2026 14:33

lukkrusz marked this pull request as ready for review March 5, 2026 14:51

	@pytest.mark.skip(reason="CI CHECK")
	@pytest.mark.skip(reason="#x issue")

		assert False, "Always Fail"


		@pytest.mark.skip(reason="Should not fail because it's skipped")

Conversation

lukkrusz commented Jan 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Motivation and Context

Types of changes

How Has This Been Tested?

Checklist:

Special treatment

Previous conversation: #443

Uh oh!

gemini-code-assist bot commented Jan 19, 2026

Summary of Changes

Highlights

Footnotes

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Jan 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Unit Test Results

Uh oh!

mateusz-bloch left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

damianloew left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

nalajcie left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lukkrusz commented Jan 19, 2026 •

edited

Loading

github-actions bot commented Jan 19, 2026 •

edited

Loading