This repository has been archived by the owner on Jan 30, 2024. It is now read-only.
Simplify snapshot tests #334
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Urhengulas
changed the title
Urhengulas
force-pushed
the
update-hardware-in-loop-tests
branch
from
0f49115
to
2b902d5
Compare
|
bors try |
Urhengulas
changed the title
Urhengulas
commented
Jul 26, 2022
tryBuild succeeded: |
|
bors r=justahero |
|
Build succeeded: |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR simplifies the probe-run snapshot tests by using the
rstestmacro.Additionally we update the dependencies used in the tests.How to test the tests
These tests are ignored by default, since they require the hardware (
nrf52840dk) to be present. You can run them locally by connecting your computer to the nrf52 and executingcargo test -- --ignored.Reviewer notes
Please don't get intimidated by the number of files and lines changed. Most of the files are just renamed to work with
rstestand the new name of the test file. The many deleted lines are because the stack overflow test got simplified in the past, but the output wasn't adapted yet. You should mainly focus ontests/snapshot.rs.Remarks
Note that the
panic_verbosetest will fail in most cases. It's because this test output contains timing related numbers, which will slightly change from run to run. This also happened before this PR, therefore isn't a regression, but should be fixed in the future.Note that updatingserial_testpulls in manyfutures-*libraries, which seems unnecessary since we are not using any async functions. I've asked if that can be avoided: palfrey/serial_test#73Edit(1): Drop dependency update from this PR, in order to unblock it.
Edit(2): Add reviewer notes.
Edit(3): Add section on how to tests the tests and structure the PR description.