Skip to content
This repository has been archived by the owner on Jan 30, 2024. It is now read-only.

add first, user-triggered snapshot tests #218

Merged
merged 1 commit into from
Jun 15, 2021
Merged

add first, user-triggered snapshot tests #218

merged 1 commit into from
Jun 15, 2021

Conversation

Lotterleben
Copy link

@Lotterleben Lotterleben commented May 31, 2021

to try them out locally, connect an nrf52840 and run cargo test -- --ignored

these test cases are pretty basic to give us some sort of baseline and can be expanded in subsequent PRs. In the future it'd be cool to have them set up as hardware in the loop tests but I think they're already helpful during development as-is, even if we don't catch regressions automatically (yet)

@Lotterleben Lotterleben requested a review from japaric May 31, 2021 14:53
Copy link
Member

@Urhengulas Urhengulas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Love it! 😸

Cargo.toml Outdated Show resolved Hide resolved
tests/README.md Outdated Show resolved Hide resolved
tests/README.md Outdated Show resolved Hide resolved
tests/README.md Outdated Show resolved Hide resolved
tests/test.rs Outdated Show resolved Hide resolved
tests/test.rs Outdated Show resolved Hide resolved
tests/test.rs Show resolved Hide resolved
tests/test.rs Show resolved Hide resolved
tests/test.rs Outdated Show resolved Hide resolved
tests/test.rs Outdated Show resolved Hide resolved
@Urhengulas
Copy link
Member

I am wondering if these tests will break with a next compiler release, because this might impact the program size, which is encoded e.g. here

(HOST) INFO flashing program (5.82 KiB)

But it seems that it at least shows the same for the current nightly, so I guess this is a not-problem 😬

@Lotterleben
Copy link
Author

I am wondering if these tests will break with a next compiler release, because this might impact the program size, which is encoded e.g. here

(HOST) INFO flashing program (5.82 KiB)

But it seems that it at least shows the same for the current nightly, so I guess this is a not-problem 😬

Since we're not re-compiling but running precompiled binaries that should be ok. If we ever recompile them then yes, we'll have to update the tests :D But I think that's actually a useful canary

tests/test.rs Outdated Show resolved Hide resolved
tests/test.rs Outdated Show resolved Hide resolved
tests/test.rs Show resolved Hide resolved
Copy link
Member

@japaric japaric left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

other than the inline comments, this looks good!

@Lotterleben Lotterleben force-pushed the local-tests branch 2 times, most recently from c4b6d90 to e69de8c Compare June 4, 2021 15:27
bors bot added a commit that referenced this pull request Jun 4, 2021
222: refactor the huge "main" function into smaller functions + module r=jonas-schievink a=japaric

what the PR title says. I ran the ELFs from PR #218 and they print the expected output.

This is the most non-trivial change that may have changed semantics
- before we were extracting *two* RAM regions from the probe-rs registry. The "first one" (returned by the iterator) and the "one that contains the initial stack pointer (initial_sp)". we installed the canary on "the first one" but only if it contained the initial_sp
- now, we only extract one RAM region, the one that contains initial_sp, and install the canary there

I think this still does the same, in practice, but not 100% sure.

Co-authored-by: Jorge Aparicio <jorge.aparicio@ferrous-systems.com>
Co-authored-by: Lotte Steenbrink <lotte.steenbrink@ferrous-systems.com>
@japaric
Copy link
Member

japaric commented Jun 4, 2021

@Lotterleben feel free to send this to bors after a rebase

@Lotterleben Lotterleben force-pushed the local-tests branch 2 times, most recently from 3e08f6d to 9b87ae3 Compare June 9, 2021 08:41
@Lotterleben
Copy link
Author

bors r+

@bors bors bot merged commit e667864 into main Jun 15, 2021
@bors bors bot deleted the local-tests branch June 15, 2021 11:46
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants