Consider post-test function

[diondokter]

This looks great!

I like that we have an init function for setting up the device. One thing we missed with defmt-test is being able to toggle a gpio pin. With that pin we could hook up a power measure device like the nrf-ppk to analyze run-time and power usage too.

If there's an (optional) post-test function that gets called immediately after a test, we can actually make this workflow work.

[t-moe]

While a post-test function could be added in theory, it cannot be called if the test panics.

That's why I left it away in the first version.

But I see your point. We plan to measure the power consumption with the same kit later in the project, so I'll have to think about it anyways... ;)

[diondokter]

Ah, panic, that's true!

Well, that could be part of the contract. Like don't rely on this being called for the correctness of your program in case of panic. This would be totally fair IMO.

[t-moe]

Note that the #[init] fn is called before each testcase. I should probably have called it #[before_each] to stay consistent with defmt-test.

As a workaround: Could you implement a drop guard and return that from your init function?

struct PowerGuard;
impl Drop for PowerGuard{
    // set gpio back to low
}

#[init]
fn init() -> PowerGuard
{
    // set gpio to high
}

[diondokter]

Could you implement a drop guard and return that from your init function?

Yes, but then every test would get extra boilerplate. In principle doing stuff so a power analysis works is not part of a test IMO.

And look, I'm not saying this is a must have. But I do think it's a nice to have.

[Yatekii]

This would be totally fair IMO.

Minus one burning tweedegolf office :P

Can we not catch the panic? Is that std only? Will this code all be executed in one go or does the runner flash multiple binaries?

[MabezDev]

Catching a panic is std only, but we could just setup a panic handler to semihost::exit, in this case, the chip would be halted and post-test would never run.

[t-moe]

I don't think we're all talking about the same thing here.

Lets say you have two testcases and an init function. Here is how they are currently run:

// chip is flashed with the binary containing all tests
// chip is run and queried for the available tests (request: semihosting SYS_GET_CMDLINE 0x15, response: semihosting USER1 0x100) 

// chip is reset
// chip is run and instructed to run testcase1 (via semihosting SYS_GET_CMDLINE 0x15)

{
  let state = init();
  let outcome = testcase1(state); // may panic
  embedded_test::export::check_outcome(outcome); // will either run semihosting::process::exit or semihosting::process::abort, and never returns
}

// chip is reset
// chip is run and instructed to run testcase2 (via semihosting SYS_GET_CMDLINE 0x15)

{
  let state = init();
  let outcome = testcase2(); // testcase 2 does not take state
  embedded_test::export::check_outcome(outcome); 
}

The panic-probe crate provides a panic_handler which just invokes semihosting::process::abort().

A potential post-test function would have to be invoked before the call to check_outcome. I did not implement this so far, because the post-test function would not be invoked if the test panics, which can be misleading. (Unless we call the post-test function from the panic_handler, which I think is an ugly hack)

For now you can easily implement a post-test function by implementing a drop guard for the state you return in init. See the updated https://github.com/probe-rs/embedded-test/issues/9#issuecomment-1852244622

[diondokter]

Ah, I did not see that you could return something from init. In that case it's easy to create a drop guard thing like you said!

IMO it would help if it was dropped immediately after the test is run instead of after the check_outcome function. Otherwise that's a good solution.

[t-moe]

IMO it would help if it was dropped immediately after the test is run instead of after the check_outcome function. Otherwise that's a good solution.

Good point. Fixed in https://github.com/probe-rs/embedded-test/commit/a5f4a1ab55f1ce5eec24e206eefddf0b376f7a26

[diondokter]

Cool! Then this usecase is supported well enough. Closing the issue.