Skip to content
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

Rust unit test dependency and initialization issues #4189

Open
cepetr opened this issue Sep 18, 2024 · 1 comment
Open

Rust unit test dependency and initialization issues #4189

cepetr opened this issue Sep 18, 2024 · 1 comment
Labels
tests Automated integration tests

Comments

@cepetr
Copy link
Contributor

cepetr commented Sep 18, 2024

This issue report addresses two system problem with our Rust unit tests (make test_rust):

1. Dependency on make build_unix:

The make test_rust target currently relies on build artifacts from make build_unix. If the emulator hasn't been built beforehand, the test build will fail. Ideally, these two targets should be independent, so that running tests does not require the emulator to be built first.

2. Improper Initialization of C Functions in Rust Tests:

In Rust unit tests, functions from the C world (e.g., trezorhal) can be invoked without proper prior initialization. While tests can call any function defined in the ffi module, there is currently no mechanism to initialize trezorhal. Normally, this is handled in main.c, which isn't included or called during test execution.

This problem became evident with the introduction of the system module, specifically with the new system_init() function. In the firmware/bootloader/emulator, system_init() is the first function called in main() to initialize system services (e.g., systick). Without this initialization, functions like systick_ms() will always return 0. Since systick_ms() is used in other trezorhal functions (as well as in storage), these functions do not work correctly without the necessary initialization.

We’ve applied a temporary fix for this issue by automatically initializing the systick module whenever any of its functions are called. However, similar initialization issues could arise in other parts of the system in the future.

To prevent these issues, systematic approach to properly initialize trezorhal drivers in the unit test binary before any test is executed should be introduced.

@cepetr cepetr added the tests Automated integration tests label Sep 18, 2024
@matejcik
Copy link
Contributor

Ideally, these two targets should be independent, so that running tests does not require the emulator to be built first.

this seemed impractical in the past: the cargo mechanism can't easily produce the micropython and trezorhal obj files that it requires, because their build is itself rather complicated

istm we can just add build_unix to dependencies of test_rust to patch over this problem 🤷‍♀️

  1. Improper Initialization of C Functions in Rust Tests:

the easiest solution is to expose a test_setup() function from C that we'll stick into a lazy static on the Rust side, and test writers will be responsible for running it at start

more complex solution is replacing the Rust test harness with something that supports global setup, but that doesn't seem that easy

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tests Automated integration tests
Projects
Status: No status
Development

No branches or pull requests

2 participants