Fix the Ci to always dump simulation artifacts#3372
Conversation
This commit adds yaml rules to always dump simulation artifacts after a CI execution. The current CI does not dump artifacts when a job fails so it can be very hard to identify, reproduce and fix a bug that occurs in the CI execution. So this commit forces the CI to always dump simulation outputs even if a job fails or encounters an error.
|
Hi @pulsar2105, thanks for your contribution. I am not convinced that this update is a good idea. The whole purpose of a CI regression is to create a simple (but not too simple), fast regression to provide a "thumbs up" or "thumbs down" gate for accepting pull-requests. Typically, if one of my pull-requests fails a CI test, I will re-run it locally with waves and increased verbosity. |
|
Hi, @MikeOpenHWGroup in this case I don't understand at all why the CVA6 CI dump artifacts when tests success. I'm currently working on a 128 bits version of the CVA6 at TIMA lab and I encounter some bugs about environment setup that only occurs in the CI environment and are very hard to reproduce, so I am very interested to recover artifacts when CI crash to understand why it crashes. I added this feature in my fork and actively use it and thought it has a good idea to share it in the main project. |
|
I agree with @pulsar2105, artifacts are most useful when jobs fail, and the environment is sometimes hard to reproduce accurately. I don't know how big artifacts are but if we can afford their upload when the thumb is up, so we should when the thumb is down. |
| - name: Run Tests | ||
| run: | | ||
| set -x | ||
| mkdir -p "verif/sim/out_$(date +%F)" |
There was a problem hiding this comment.
Isn't this folder created during the bash script? Do we need to add this line?
There was a problem hiding this comment.
I figured that since we're forcing the dumping of artifacts, it would be safer if the CI could dump something even if the directory is empty. But if you think it useless I'm good to go delete it.
| DV_SIMULATORS=${{matrix.simulator}},spike DV_TARGET=${{matrix.config}} bash verif/regress/${{matrix.testcase}}.sh | ||
|
|
||
| - name: Upload Lint Report to Github | ||
| if: always() |
There was a problem hiding this comment.
You mentioned that you are using this in your own fork so I will not double-check that this is the correct way to proceed ;)
As I say, this is not my decision. I would not dumb anything on a CI,beyond pass/fail. But that's me. |
|
For information, the plan is to superseed this CI by a new CI during Summer. I propose to close this PR, and maybe have a talk in the coming months about the support this feature on the new CI. |
|
Thank you @JeanRochCoulon for clarifying that. I think indeed that if nobody have seen this problem of artifacts dumping since May 2024 that is a sign that the role of the CI need to be rethought. I would be happy to help in this direction in the future and even provide a Dockerfile to run the CVA6 when the time will be right. |
4 participants
Add yaml rules to dump CI artifacts in case of failure
The current CI does not dump artifacts when a job fails
so it can be very hard to identify, reproduce and fix a bug that occurs
during CI execution. With this patch, we force dumping
simulation outputs when a CI error occurs.