Test harness

[str4d]

Closes #53.

[str4d]

Currently it is not possible to provide arguments to external FAPs. I've opened flipperdevices/flipperzero-firmware#2505 upstream requesting this. In the meantime, this just means that test filtering won't work.

[str4d]

I haven't yet figured out if any of the Flipper Zero SDKs let a running external FAP learn where it was loaded from. Without that knowledge, it's gonna be tricky to reliably redirect stdout to a file that both won't collide with an existing file on the Flipper Zero, and that this tool can find.

[str4d]

I ended up just using a common fixed path of /ext/flipperzero-rs-stdout, on the assumption that external FAPs cannot run concurrently, so only one app at a time will ever be running, and as long as we immediately read the file and then remove it we're probably fine. This will do until the Flipper Zero SDK supports RPC usage.

[JarvisCraft]

You may try addressing the issues in Flipperzero Telegram group, i.e. Development (SW&HW) channel.

[str4d]

flipperdevices/flipperzero-firmware#2505 (comment) indicated that an improved host-device communication protocol is planned for external FAPs, so that is likely what will be used instead when available.

[str4d]

Okay, I have this working on Linux and macOS! Current output:

$ cd crates/
$ cargo test
   Compiling proc-macro2 v1.0.52
   Compiling unicode-ident v1.0.8
   Compiling quote v1.0.26
   Compiling syn v1.0.109
   Compiling ufmt-write v0.1.0
   Compiling flipperzero-rt v0.7.2 (/path/to/flipperzero/crates/rt)
   Compiling ufmt-macros v0.3.0
   Compiling flipperzero-test-macros v0.1.0 (/path/to/flipperzero/crates/test/macros)
   Compiling ufmt v0.2.0
   Compiling flipperzero-sys v0.7.2 (/path/to/flipperzero/crates/sys)
   Compiling flipperzero-test v0.1.0 (/path/to/flipperzero/crates/test)
   Compiling flipperzero-alloc v0.7.2 (/path/to/flipperzero/crates/alloc)
   Compiling flipperzero v0.7.2 (/path/to/flipperzero/crates/flipperzero)
    Finished test [optimized + debuginfo] target(s) in 4.73s
     Running unittests src/lib.rs (target/thumbv7em-none-eabihf/debug/deps/flipperzero-ec498257ef00abdb)

running 3 tests
test dolphin::tests::stats ... ok
test furi::message_queue::tests::capacity ... ok
test furi::sync::tests::unshared_mutex_does_not_block ... ok

test result: ok. 3 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 2ms
leaked: 88 bytes

$ # Unplug Flipper Zero
$ cargo test
    Finished test [optimized + debuginfo] target(s) in 0.01s
     Running unittests src/lib.rs (target/thumbv7em-none-eabihf/debug/deps/flipperzero-ec498257ef00abdb)
Error: unable to find Flipper Zero
error: test failed, to rerun pass `-p flipperzero --lib`

Caused by:
  process didn't exit successfully: `/path/to/flipperzero/crates/../tools/cargo-runner.sh /path/to/flipperzero/crates/target/thumbv7em-none-eabihf/debug/deps/flipperzero-ec498257ef00abdb` (exit status: 1)

[str4d]

And if I manually cause some tests to fail:

$ cargo test
   Compiling flipperzero v0.7.2 (/path/to/flipperzero/crates/flipperzero)
    Finished test [optimized + debuginfo] target(s) in 0.28s
     Running unittests src/lib.rs (target/thumbv7em-none-eabihf/debug/deps/flipperzero-ad4e99e6ec932a9a)

running 3 tests
test dolphin::tests::stats ... ok
test furi::message_queue::tests::capacity ... FAILED

---- furi::message_queue::tests::capacity stdout ----
assertion failed: queue.len() == 1

test furi::sync::tests::unshared_mutex_does_not_block ... FAILED

---- furi::sync::tests::unshared_mutex_does_not_block stdout ----
assertion failed: * value == 6


test result: FAILED. 1 passed; 2 failed; 0 ignored; 0 measured; 0 filtered out; finished in 5ms
leaked: 88 bytes

Currently the test failures are printed in-line, to avoid requiring alloc.

[str4d]

I've adjusted the test harness so it now also works for integration tests, and to allow the manifest arguments to be set (in particular, while adding more tests in subsequent local changes I need to bump the stack size).

There are probably more usability tweaks that could be made, but I think we'll discover those as the test harness is used, so I'm not planning to make any more changes to this PR other than in response to review.

[dcoles]

Should we just apply #[no_main] unconditionally? I'm not sure if there's much of an impact on lib crates, but FAP applications are unlikely to support a standard main function.

[str4d]

Rebased on main to fix merge conflict with #59.

[dcoles]

it should now also work on Windows (untested, but Windows can execute *.py files directly if their extension is associated correctly).

Looks like runner must be an application, not just an associated file type:

PS> ./cargo-runner.py
usage: cargo-runner.py [-h] binary ...
cargo-runner.py: error: the following arguments are required: binary, arguments
PS> cargo test
    Finished test [optimized + debuginfo] target(s) in 0.03s
     Running unittests src\lib.rs (target\thumbv7em-none-eabihf\debug\deps\flipperzero-b5d0b378c3208e7a)
error: test failed, to rerun pass `-p flipperzero --lib`

Caused by:
  could not execute process `C:\Users\dcoles\workspace\flipperzero-rs\crates\./cargo-runner.py C:\Users\dcoles\workspace\flipperzero-rs\crates\target\thumbv7em-none-eabihf\debug\deps\flipperzero-b5d0b378c3208e7a` (never executed)

Caused by:
  %1 is not a valid Win32 application. (os error 193)

In this case, maybe we just go with the shell script and make this work for Windows in a future PR.

[dcoles]

For something as simple as this script, we could probably even get away with just + sys.argv.

[str4d]

it should now also work on Windows (untested, but Windows can execute *.py files directly if their extension is associated correctly).

Looks like runner must be an application, not just an associated file type:

I've now found rust-lang/cargo#8347 which indicates we just need to set the runner to python path/to/script.py. I'll update the PR to do that, and then you can retest on Windows.

[str4d]

@str4d It looks like the runner might be causing problems for GitHub Actions. Can you take a look?

I think what's going on here is that CI was configured to run all tests, but nothing had tests enabled so it did nothing. Now that we enable tests, CI tries to run them, but because the .cargo/config sets the default target to be not the host, they fail. I think the solution is to just not run any tests in CI, as there are no useful tests to be run outside the Flipper Zero.

[str4d]

Force-pushed to add missing package to CI, and hopefully fix cargo test on Windows.

[str4d]

Okay, confirmed that CI fails now because it can't find a Flipper Zero. I'll adjust CI to expect that failure, so we're still compiling everything and checking the runner.

[dcoles]

I've now found rust-lang/cargo#8347 which indicates we just need to set the runner to python path/to/script.py. I'll update the PR to do that, and then you can retest on Windows.

Looks like I hit python/cpython#99185 (Windows does not provide a python3.exe), which is totally dumb and bites me all the time. It would be possible to make the script compatible with both Python 2 and 3, but on my Ubuntu box there is no python binary unless I install the python-is-python3 package.

PEP-397 tries to resolve this situation by introducing a py.exe that can interpret the shbang line, but this isn't available on Ubuntu (not to be confused with pythonpy).

Not sure if there's an easy fix. There doesn't seem to be a cfg(host_os = "windows") and I can't figure out a way to get cmd.exe and bash to accept the same runner line.

[str4d]

Hmm. Now that I know we can have arguments inside runner, I wonder if we can just call cargo directly instead of using a wrapper script? We know that cargo should be available on the PATH. The purpose of the script was to both encapsulate the numerous arguments and enable changing working directory so that the .cargo/config won't apply. If the latter can be worked around, then this might work.

[str4d]

The purpose of the script was to both encapsulate the numerous arguments and enable changing working directory so that the .cargo/config won't apply. If the latter can be worked around, then this might work.

I cannot figure out how to work around the latter inside target.<target>.runner without adding a cd ... &&, which is not cross-platform. However, reading a bunch of issues (around trying to unset build.target (rust-lang/cargo#8112) or reset it to the host (rust-lang/cargo#8687)) led me to discover per-package-target, which seems like it would also resolve the issue. We would set each of the crates under crates/ to have a package.forced-target, and then remove the build.target from crates/.cargo/config.toml. This would mean that calling cargo run --manifest-path ../tools/Cargo.toml from inside crates/ would then compile for the host instead of the Flipper Zero. However, it would block #58 on another unstable feature.

[str4d]

Sadly per-package-target doesn't work, because even though forced-target correctly causes the test binary to be compiled for the Flipper Zero, it doesn't cause the corresponding per-target runner to be used (so instead the binary runs directly on the host). I've asked about this in the tracking issue (rust-lang/cargo#9406 (comment)).

However, I've figured out another solution: aliases! We remove the build.target (so by default we build for the host), and then add aliases that set the correct --target. I've pushed a commit that does this, providing *-flipper aliases for check, build, and test (happy to rename to something else / shorter if preferred). This does interfere with muscle memory though (so it's a trade-off between "need to remember separate commands" with this commit, and "need to make custom changes to my OS environment to get Python working" before), and it means that e.g. the rust-analyzer plugin for VSCode needs to be configured to use the correct target (I can commit that if you want).

[dcoles]

Thanks for sticking with this! Honestly I'm fine with just setting it to python3 and calling it a day (I'd prefer to have the Linux environment work well, even if it means the recommendation for developing these crates is "use WSL").

This does interfere with muscle memory though (so it's a trade-off between "need to remember separate commands" with this commit, and "need to make custom changes to my OS environment to get Python working" before), and it means that e.g. the rust-analyzer plugin for VSCode needs to be configured to use the correct target (I can commit that if you want).

It worries me a little not setting a build.target, since this crate is unlikely to support anything else (but I could be wrong—maybe someone makes a Celeron-based Flipper? 😱 ). Having flipper test run unit tests on-device is nice, but not to the point I'd want to risk breaking IDE integration out-of-the-box.

A couple of thoughts:

• Would it be possible to have an alias that does the on-device tests (e.g. cargo flipper-tests), rather than trying to force flipper test work?
• Could a build.rs build script be used to prepare the right runner/runner script for a host platform?

[str4d]

It worries me a little not setting a build.target, since this crate is unlikely to support anything else (but I could be wrong—maybe someone makes a Celeron-based Flipper? 😱 ).

This is why I added the target detection code, so that flipperzero-sys would refuse to compile except on the intended target.

A couple of thoughts:

• Would it be possible to have an alias that does the on-device tests (e.g. cargo flipper-tests), rather than trying to force flipper test work?

That is exactly what the current aliases do (it's cargo test-flipper but yes), leaving cargo test to just not work.

• Could a build.rs build script be used to prepare the right runner/runner script for a host platform?

AFAICT no; the build.rs script only influences building and linking, not running the resulting binary.

Honestly I'm fine with just setting it to python3 and calling it a day (I'd prefer to have the Linux environment work well, even if it means the recommendation for developing these crates is "use WSL").

Then let's just do this. A Windows user can copy python.exe to python3.exe in the same folder and call it a day (at least until they need to update Python).

I've edited the last commit to remove the alias changes (but left the target detection code since that is independently useful).

[str4d]

@dcoles is there anything else I need to do before this can be merged? I'd love to get back on with my other PRs like #50 that are blocked on testing.

[dcoles]

Hi @str4d. Apologies for the wait. Yes, I'm happy to merge this.

Over the weekend I got to see how a few other projects tackle this. Ferrus Systems has a tool called probe-run that can be used as a runner for a number of embedded platforms (though probably not compatible with Flipper apps). You can use the hf2-cli in a similar way via hf2 elf.

I should make flipperzero-tools into an installable package which would make it far easier to use run-fap in a similar manner.

[str4d]

Over the weekend I got to see how a few other projects tackle this. Ferrus Systems has a tool called probe-run that can be used as a runner for a number of embedded platforms (though probably not compatible with Flipper apps). You can use the hf2-cli in a similar way via hf2 elf.

Yeah, those were what made me realize we could run tests and apps in this way. But we can't use any of the existing ones because they flash firmware, and I wanted this to be as easy to use as possible, so no need to install anything ideally. Didn't quite get there on all platforms, and if we end up having an installable FAP runner then I don't mind.

[str4d]

The other thing is that an installable FAP runner will be much nicer once upstream has implement proper RPC support for FAPs, so we don't have to do the hacky "write stdout to a file and then copy it off afterwards" that really is only suitable for one-shot FAPs.

[dcoles]

The other thing is that an installable FAP runner will be much nicer once upstream has implement proper RPC support for FAPs, so we don't have to do the hacky "write stdout to a file and then copy it off afterwards" that really is only suitable for one-shot FAPs.

I'm very much looking forward to that. It's bit awful what you have to do when trying to use a serial CLI as a file transfer layer.