Skip to content

fix: let the unity_tester fixture accept emulator duts (CII-249) - #430

Merged
hfudev merged 2 commits into
espressif:mainfrom
Harshal5:fix/unity_tester_emulator_duts
Aug 28, 2026
Merged

fix: let the unity_tester fixture accept emulator duts (CII-249)#430
hfudev merged 2 commits into
espressif:mainfrom
Harshal5:fix/unity_tester_emulator_duts

Conversation

@Harshal5

Copy link
Copy Markdown
Contributor

Description

Every test using the unity_tester fixture errors out under --embedded-services idf,espemu or --embedded-services idf,qemu:

E   Failed: fixture function has more than one 'yield':

except ImportError:
    yield None
        yield None                                                                                 
    yield CaseTester(to_list(dut))

Two separate bugs in the same fixture:

  1. Double yield. yield None sits inside the for loop, so it does not end the generator, execution continues to yield CaseTester() and pytest rejects the fixture.
  2. The gate is incorrect. isinstance(_dut, IdfDut) can never hold for an emulator dut: IdfDut(IdfUnityDutMixin, SerialDut) is serial based, so dut_factory.py picks EspEmuDut / QemuDut instead. But the factory deliberately mixes IdfUnityDutMixin into those duts when the idf service is used (dut_factory.py:393), and that mixin is all CaseTester needs, it drives dut.test_menu, not anything serial specific.

Because ESP-IDF's root conftest.py wraps this fixture:

@pytest.fixture
def case_tester(unity_tester: CaseTester) -> CaseTester:
     return unity_tester

so every ESP-IDF test taking case_tester is unrunnable on either emulator.

Fix

Yield exactly once per path, and accept any dut that carries the mixing CaseTester relies on:

duts = to_list(dut)
if all(isinstance(_dut, IdfUnityDutMixin) for _dut in duts):
    yield CaseTester(duts)
else:                                                                                                      
    yield None
  • IdfDut still satisfies the check, since it inherits IdfUnityDutMixin, thus, no change for serial runs.
  • Applies equally to the qemu service; nothing qemu specific is needed.

Checklist

Before submitting a Pull Request, please ensure the following:

  • 🚨 This PR does not introduce breaking changes.
  • All CI checks (GH Actions) pass.
  • Documentation is updated as needed.
  • Tests are updated or added as necessary.
  • Code is well-commented, especially in complex areas.
  • Git history is clean — commits are squashed to the minimum necessary.

The fixture yielded twice when a dut was not an `IdfDut`, so every case using
it errored under `--embedded-services idf,espemu` or `idf,qemu`. Accept any
dut carrying `IdfUnityDutMixin`, which is what `CaseTester` actually needs.
@Harshal5

Copy link
Copy Markdown
Contributor Author

@hfudev Could you PTAL?

@github-actions github-actions Bot changed the title fix: let the unity_tester fixture accept emulator duts fix: let the unity_tester fixture accept emulator duts (CII-249) Aug 28, 2026
@github-actions

github-actions Bot commented Aug 28, 2026

Copy link
Copy Markdown
Title Coverage Tests Skipped Failures Errors Time
Qemu Coverage 7 0 💤 0 ❌ 0 🔥 25.299s ⏱️
3.14 X64 Coverage 122 19 💤 0 ❌ 0 🔥 15m 29s ⏱️
Espemu Coverage 3 0 💤 0 ❌ 0 🔥 3.485s ⏱️
3.10 ARM64 Coverage 122 21 💤 0 ❌ 0 🔥 13m 12s ⏱️

@hfudev

hfudev commented Aug 28, 2026

Copy link
Copy Markdown
Member

Hi @Harshal5 the fix in general makes sense. but now this test case is failing constantly (after i retried once)

ERROR test_expect_unity_test_ouput.py::test_expect_unity_test_output_basic - RuntimeError: Failed to process thread exception

could you help check the error?

test_expect_from_timeout left a daemon thread writing for 7.5s while the test
returned after 4s, so it wrote into a torn-down queue and raised BrokenPipeError
after the session. pytest reported that against whichever test ran next.
@Harshal5

Copy link
Copy Markdown
Contributor Author

Hi @Harshal5 the fix in general makes sense. but now this test case is failing constantly (after i retried once)

ERROR test_expect_unity_test_ouput.py::test_expect_unity_test_output_basic - RuntimeError: Failed to process thread exception

could you help check the error?

Thanks for looking. I dug into that error using an AI agent and I don't think it comes from this change.

Issue

The reported message is the tail of a chained exception; the cause in the job log is:

ERROR at setup of test_expect_unity_test_output_basic
thread = <Thread(Thread-1 (write_bytes), stopped daemon ...)>
>   hook(args)
E   KeyError: 'tracemalloc'
/usr/local/lib/python3.10/threading.py:1326: KeyError

Thread-1 (write_bytes) belongs to the previous test in that file, test_expect_from_timeout: it starts a daemon thread that writes 5 times at 1.5s intervals (7.5s), while the test itself returns after dut.expect(pexpect.TIMEOUT, timeout=4).
The thread outlives teardown, writes into a queue that is gone, and its excepthook raises KeyError: 'tracemalloc' on Python 3.10. pytest then reports the queued thread exception during the setup of the next test, which is why it lands on test_expect_unity_test_output_basic, before any fixture body runs.

Why CI sees it and a plain local run doesn't

Without coverage the run finishes before the pipe is gone. With --cov (what CI always uses) the session is slow enough that the thread's next write hits a closed pipe: I reproduced the warning locally on 3.14 just by adding --cov.

Fix

I've pushed a commit that stops the writer thread with the test that starts it (a threading.Event plus a join in finally). With --cov enabled locally the thread exception is gone and the file behaves as before otherwise. That should also remove a latent flake for every PR, since any test that happened to run after this one could inherit the error.

@hfudev
hfudev merged commit 97d83c5 into espressif:main Aug 28, 2026
7 checks passed
@hfudev

hfudev commented Aug 28, 2026

Copy link
Copy Markdown
Member

thank you! will release next Monday

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants