Conversation
Member
Author
bisected this down to PR #564 |
Urhengulas
approved these changes
Sep 29, 2021
Member
Urhengulas
left a comment
Urhengulas
left a comment
There was a problem hiding this comment.
Looks good. I've left two comments. You add a bit of redundant code, but that's not a huge problem.
Member
|
bors r+ |
bors Bot
added a commit
that referenced
this pull request
Oct 1, 2021
592: xtask: add backward compability test r=Urhengulas a=japaric **UPDATE**: I have left the defmt-version as it is since we have not made a defmt 0.3.0 release. I have also tweaked the defmt-test to not exit with non-zero exit code and remove some test-specific logic from xtask. See individual commits messages for details. --- implements #506 (comment) closes #506 I tested this against c7b20d5 (I called that `defmt-version-3` locally) and found that a bitflags change broke the wire format. Not a big deal but I guess we should bump the version to 4? cc `@jonas-schievink` ``` console bitflags (dev) Error: malformed bitflags value string 'Flags::0::FLAG_0' ``` the other thing that I found out is that some of the snapshot test exit with non-zero code: ``` console defmt-test (dev) INFO (1/7) running `change_init_struct`... INFO (2/7) running `test_for_changed_init_struct`... INFO (3/7) running `assert_true`... INFO (4/7) running `assert_imported_max`... INFO (5/7) running `result`... INFO (6/7) running `should_error`... INFO (7/7) running `fail`... ERROR panicked at '`#[should_error]` test failed with outcome: Ok(this should have returned `Err`)' Timer with period zero, disabling ``` `cargo xtask test-snapshot` is not checking the exit code; it only looks at stdout so that non-zero exit code is not a problem for those tests. in the backcompat test we want to look for decoding errors, not at stdout, so I was using the exit code of `cargo run` but this `defmt-test` is problematic with that check. Co-authored-by: Jorge Aparicio <jorge.aparicio@ferrous-systems.com>
Contributor
|
Build failed: |
japaric
force-pushed
the
test-backcompat
branch
from
ef0b9ca to
96b90a4
Compare
Member
Author
|
had to bump the revision because defmt::println! broke the wire format (expected) |
Contributor
|
Build succeeded: |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
UPDATE: I have left the defmt-version as it is since we have not made a defmt 0.3.0 release. I have also tweaked the defmt-test to not exit with non-zero exit code and remove some test-specific logic from xtask. See individual commits messages for details.
implements #506 (comment)
closes #506
I tested this against c7b20d5 (I called that
defmt-version-3locally) and found that a bitflags change broke the wire format. Not a big deal but I guess we should bump the version to 4? cc @jonas-schievinkthe other thing that I found out is that some of the snapshot test exit with non-zero code:
cargo xtask test-snapshotis not checking the exit code; it only looks at stdout so that non-zero exit code is not a problem for those tests. in the backcompat test we want to look for decoding errors, not at stdout, so I was using the exit code ofcargo runbut thisdefmt-testis problematic with that check.