Skip to content

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
@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

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
@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