-
Notifications
You must be signed in to change notification settings - Fork 27
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
refactor: change logic of threading control in CaseTester for MultiDev (RDT-534) #223
Conversation
b9db548
to
d693f1e
Compare
8955ebd
to
19db0cb
Compare
Hello, I've refactored thread depends code, now it works with generator functions |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! The new approach looks good.
Still got some high-level problem.
The merged junit xml looks like this:
<testcase file="[group dev-0]: components/driver/test_apps/spi/master/main/test_spi_master.c&lt;-------------------&gt; [group dev-1]: components/driver/test_apps/spi/master/main/test_spi_master.c" line="[group dev-0]: 1603&lt;-------------------&gt; [group dev-1]: 1603" time="1.859" name="('esp32c6', 'esp32c6').('defaults', 'defaults').SPI_Master:Test multiple devices [dut-0]">
<system-out>[group dev-0]:
... log ...
&lt;-------------------&gt;
[group dev-1]:
... log ...
</testcase>
Got two places to improve:
[group dev-0]
should be[dut-0]
to keep consistence- The
----------
separator should not be used in attributes, likefile
,line
After this PR got merged, the old The benefits of this approach are:
|
Have went through the code, I love the idea of |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Let's drop the deprecation warning first, usually it's not a good idea to confuse users...
For the optimization of the timeout calculation, you may do it in a separate PR.
472fd39
to
3038131
Compare
p.s. problem was with device reset |
765016a
to
56de649
Compare
oh, somehow I missed it in the review. Glad you found it :) |
56de649
to
89bae77
Compare
89bae77
to
7d0ee35
Compare
1b3c212
to
3529f27
Compare
Thank you @horw LGTM! |
Replaced logic of creating thread
before: new case every time create new thread
now: create thread once, and then thread wait for task
closes #184
closes #162