search

LukasKalbertodt / libtest-mimic

A small test framework to write your own test harness that looks and behaves like the built-in test harness used by `rustc --test`
https://docs.rs/libtest-mimic
Apache License 2.0
95 stars 26 forks source link

Add Json Format, Intellij Options and make multi-threaded optional #34

Open t-moe opened 6 months ago

t-moe commented 6 months ago

This PR:

Closes #1 Closes #25 Makes #33 obsolete

t-moe commented 6 months ago

@LukasKalbertodt I'm not sure what you meant in https://github.com/LukasKalbertodt/libtest-mimic/pull/33/files#r1235742883 The remaining issues of that PR I've tried to address.

t-moe commented 5 months ago

Hi @LukasKalbertodt

Could you take a moment and review this PR in the coming weeks? This step is crucial for progressing with our other PRs, as this is the only dependency not yet upstreamed to crates.io. Your feedback would be greatly appreciated and will help keep our project moving forward smoothly.

Thank you for your time and assistance!

LukasKalbertodt commented 5 months ago

Hi @t-moe, thanks for the PR! I don't have a lot of time to work on libtest-mimic currently, so my reviews won't be very quick.

I just cherry picked the JSON and no-op args parts of this PR and merged them. They are also already released as 0.7.0! So thank you for that.

What remains in this PR is the "remove send bound" and "add trial lifetime param". Before I ask you to rebase, could you motivate these changes for me? The first one part adds quite a bit of complexity, and the second one even complicates the API. So I'm naturally hesitant to merge that, as I cannot think of a use case for this. So yeah, please explain :)

t-moe commented 5 months ago

Thank you @LukasKalbertodt for taking the time to cherry-pick and release parts of this PR!

What remains in this PR is the "remove send bound" and "add trial lifetime param". Before I ask you to rebase, could you motivate these changes for me? The first one part adds quite a bit of complexity, and the second one even complicates the API. So I'm naturally hesitant to merge that, as I cannot think of a use case for this. So yeah, please explain :)

We use libtest-mimic to enable integration-testing of embedded devices. More specifically, libtest-mimic is used in probe-rs which in turn controls targets running embedded-test. probe-rs first flashes a binary containing all tests onto the target, and then for each test resets the device and then runs the test (communicating via semihosting).

Our primary need was to disable the --test-threads command line option, as concurrent tests aren't feasible on embedded devices.

Because we setup the connection with the Debug Adapter and the target only once, and not before each test, we need to pass some references into the test function. Removing the send bounds and allowing to specify the lifetime, allowed this without hacks on our side, hence this change.

I think from the perspective of a seasoned rust developers these modifications would enhance the library's versatility, making it more adaptable for various use cases. However, I recognize that from a teaching standpoint, the balance between increased flexibility and the resultant complexity might not seem as favorable.

I greatly appreciate your time in reviewing these changes and am more than willing to engage in further discussions or revisions to align this proposal with the library's vision and user base.