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

feat(test): Improve kurtosis workflow #57

Merged
merged 29 commits into from
Jan 31, 2025
Merged

Conversation

janjakubnanista
Copy link
Collaborator

@janjakubnanista janjakubnanista commented Jan 29, 2025

In this PR

The biggest change is the ability to filter the kurtosis setups by any glob - users can type in local, remote, no_op_geth etc and the job will only run the matching setups. Besides this a lot of improvements were introduced:

  • Improve kurtosis tests visibility
    • DRY the code first by introducing a shell script that dynamically checks the block creation on all EL clients
    • DRY the pipeline since now the code does not need to be duplicated & slightly modified for different args files
    • Improve the debug output by grouping the kurtosis logs on the CI
    • Always dump the logs, no matter whether the run failed or succeeded
    • Run all local/remote kurtosis configurations in a matrix job
  • Remove the notify-on-error job that never worked since it's not connected to Slack
  • Use latest as the default tag for op-node instead of develop
  • Fix a small typo (op-reth image was specified as op-reth:latest which does not exist)
  • Show the repositories and refs from which the local images are built
  • Don't make users type repositories like https://github.com/org/repo.git - let them just type org/repo

This is an example run where the block creation failed. Check the dump logs job - it has the kurtosis logs arranged into collapsible UI groups for ease of use.

This is an example of a job with no filter

This is an example of a job with no_op_geth filter

This is an example of a job with filter that does not match anything. spoiler alert: it fails

Fixes #53

@janjakubnanista janjakubnanista self-assigned this Jan 30, 2025
@janjakubnanista janjakubnanista marked this pull request as ready for review January 30, 2025 05:35
@janjakubnanista janjakubnanista changed the title feat: Improve kurtosis workflow output feat(ci): Improve kurtosis workflow output Jan 30, 2025
@janjakubnanista janjakubnanista changed the title feat(ci): Improve kurtosis workflow output feat(test): Improve kurtosis workflow output Jan 30, 2025
@janjakubnanista janjakubnanista changed the title feat(test): Improve kurtosis workflow output feat(test): Improve kurtosis workflow Jan 30, 2025
Copy link
Member

@emhane emhane left a comment

Choose a reason for hiding this comment

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

nice! some comments:

i) since it takes such a long time to build op-reth, it would be best if we can run all the local setups every time like it was before, i.e.

  • op-reth<>op-node<>op-geth(sequencer)
  • op-reth(sequencer)<>op-node<>op-geth
  • op-reth<>op-node
  • op-node<>op-geth

it would make more sense if the drop down menu just has the options local or remote rather, and in the case of local, that it continue to provide the specification of fork and branch.

ii) the possibility to collapse logs for a certain component is great. can we always dump logs, i.e. also on positive outcome? that we we can see in the output what kind of transactions and blocks are being built and gossiped. this is possibly out of scope of this pr.

@janjakubnanista janjakubnanista force-pushed the jan/workflow--007 branch 13 times, most recently from ef0ba76 to a268ef8 Compare January 30, 2025 22:30
Copy link
Member

@emhane emhane left a comment

Choose a reason for hiding this comment

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

the new filter is lit!

only remaining comment, is if the names of the jobs can be a bit more human readable friendly, i.e. instead of having the jobs be called the file name like
Screenshot 2025-01-31 at 13 49 34
display the names:

  • multiclient, op-geth sequencer
  • op-reth
  • op-geth
  • multiclient op-reth sequencer

@janjakubnanista
Copy link
Collaborator Author

@emhane updated:

Screenshot 2025-01-31 at 9 53 37 AM

@janjakubnanista janjakubnanista merged commit a95802e into optimism Jan 31, 2025
37 of 42 checks passed
@janjakubnanista janjakubnanista deleted the jan/workflow--007 branch January 31, 2025 17:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add branch and fork to job name for local kurtosis
2 participants