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

improve regress #728

Merged
merged 14 commits into from
Dec 6, 2023
Merged

improve regress #728

merged 14 commits into from
Dec 6, 2023

Conversation

Emilgardis
Copy link
Member

@Emilgardis Emilgardis commented Jun 4, 2023

Beginning of revamp for our CI

This pr adds the following new features

  • Makes it possible to specifically single out a chip and have tests/diff always be run for it
  • Allow passing any command to svd2rust via svd2rust-regress tests -- --command
  • Add alias cargo regress -> cargo run -p svd2rust-regress --
  • Allow diffing two versions of the output
    This allows for diffing between svd2rust versions and command inputs
    # diff the output between master branch without any extra args and this pr with --atomics
    svd2rust-regress diff --baseline "" --current "--atomics" --chip my-chip
    # diff the output of master branch and this pr 
    svd2rust-regress diff --baseline "@master" --current "@pr" --chip my-chip
    # shorthand for above with a well-known default chip
    svd2rust-regress diff pr
    where @pr = version from current pr, @master = version from master branch
  • Add a on_comment hook to allow diffing prs
    /ci diff pr
    
    /ci diff -c achipwewanttodiff -- --atomics
    
    this should output the result in a <details> or gist

resolves #355

@burrbull
Copy link
Member

burrbull commented Jun 4, 2023

Nice!

@burrbull
Copy link
Member

burrbull commented Jun 4, 2023

we need to add retry for curl
https://stackoverflow.com/questions/42873285/curl-retry-mechanism

@Emilgardis Emilgardis force-pushed the improve-regress branch 2 times, most recently from 78a8f10 to 1ae4393 Compare June 5, 2023 07:06
@Emilgardis Emilgardis added the no changelog no-changelog label Jun 5, 2023
@Emilgardis
Copy link
Member Author

added no changelog just to make ci happy for now

@Emilgardis Emilgardis force-pushed the improve-regress branch 2 times, most recently from 664f976 to 2c1a1ed Compare June 5, 2023 19:04
src/generate/peripheral.rs Outdated Show resolved Hide resolved
@burrbull
Copy link
Member

burrbull commented Jul 6, 2023

What status of this?

@Emilgardis
Copy link
Member Author

I still want to implement it, just don't have the time or motivation right now

@Emilgardis
Copy link
Member Author

Header text

Diff result (click to expand)
diff --git "a/.\\output\\head\\atmel_at91sam9cn11\\/src/lib.rs" "b/.\\output\\base\\atmel_at91sam9cn11\\/src/lib.rs"
index 4fe602c..b31966b 100644
--- "a/.\\output\\head\\atmel_at91sam9cn11\\/src/lib.rs"
+++ "b/.\\output\\base\\atmel_at91sam9cn11\\/src/lib.rs"
@@ -1,4 +1,4 @@
-/*!Peripheral access API for AT91SAM9CN11 microcontrollers (generated using svd2rust v0.31.2 (99e68c7 2023-11-29))
+/*!Peripheral access API for AT91SAM9CN11 microcontrollers (generated using svd2rust v0.31.2 (ef9b813 2023-11-29))

 You can find an overview of the generated API [here].

@Emilgardis
Copy link
Member Author

Emilgardis commented Nov 29, 2023

it's now possible to do

cargo regress diff --format --base "@v0.30.3" --head "@v0.31.2"
cargo regress diff --format --head "@debug"

etc.

i have one problem with this though, the diff is way to large to be always useful. I think it would be helpful to have a very small default example case that is expected to only emit few changes. Maybe we could even make a query somehow to specify something like "I only want the diff inside mod clusterb"

@Emilgardis
Copy link
Member Author

@burrbull

This comment was marked as resolved.

@Emilgardis
Copy link
Member Author

Emilgardis commented Dec 4, 2023

switched to yaml!
Any opinions on the diff? I think I want it in another way, and having it be saved elsewhere other than action logs would be nice, but I don't think a comment works for it and there's no way to create a gist using an organization account (we'd need a rust-embedded bot account)

@burrbull
Copy link
Member

burrbull commented Dec 4, 2023

Any opinions on the diff? I think I want it in another way, and having it be saved elsewhere other than action logs would be nice, but I don't think a comment works for it and there's no way to create a gist using an organization account (we'd need a rust-embedded bot account)

Need someone more powerful. cc @therealprof @adamgreig

@burrbull
Copy link
Member

burrbull commented Dec 5, 2023

@Emilgardis I've made several fixes. See #782

.cargo/config.toml Outdated Show resolved Hide resolved
summary:
runs-on: ubuntu-latest
needs: [diff]
if: always()
Copy link
Member Author

@Emilgardis Emilgardis Dec 5, 2023

Choose a reason for hiding this comment

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

this needs to check that generate.outputs.diffs was not empty, otherwise a comment will always be done: Emilgardis#1 (comment)

@burrbull
Copy link
Member

burrbull commented Dec 6, 2023

Maybe it is better to merge this PR and make future changes in new PRs?

@Emilgardis
Copy link
Member Author

I think that's fine, I'll cleanup the commits and do the last adjustments before a review to merge

burrbull
burrbull previously approved these changes Dec 6, 2023
@Emilgardis
Copy link
Member Author

managed to sneak in a bug during the rebase and implementing regress pr, should be all good now.

@burrbull burrbull added this pull request to the merge queue Dec 6, 2023
@Emilgardis
Copy link
Member Author

🥳

Merged via the queue into master with commit 4f5fa5b Dec 6, 2023
46 checks passed
@burrbull burrbull deleted the improve-regress branch December 6, 2023 19:53
@Emilgardis
Copy link
Member Author

/ci diff pr

lets try it :)

Copy link

github-actions bot commented Dec 6, 2023

Diff for comment

@Emilgardis
Copy link
Member Author

well it works, but it doesn't like the command , will fix

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no changelog no-changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

svd2rust-regress usability woes
2 participants