-
Notifications
You must be signed in to change notification settings - Fork 6
Added Westend para workflow to CI and justfile #152
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
Conversation
examples/justfile
Outdated
| # Check out the required branch | ||
| echo " Checking out branch bko-bulletin-para-support..." | ||
| git fetch origin | ||
| # TODO: later, before merging, switch to the `bko-bulletin-support` https://github.com/paritytech/polkadot-sdk/pull/10662 (not now) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shall this TODO be fixed in this PR?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shall this TODO be fixed in this PR?
Let's better do a follow-up, but you need to add this runtime API to the bulletin-westend runtime: https://github.com/paritytech/polkadot-sdk/pull/10656/files#diff-443b1be7b55963d65b470281941835067391449385b417b01a5ab5183951f253 (or bump polkadot sdk master if that PR is merged).
examples/justfile
Outdated
| echo " Using zombienet: $ZOMBIENET_BIN" | ||
|
|
||
| # Handle different runtimes | ||
| if [ "{{ runtime }}" = "bulletin-westend-runtime" ]; then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@x3c41a I would rather keep bulletin-solo-zombienet-start as it is and instead create bulletin-westend-para-zombienet-start and move this IF to the setup-services where we decide what zombienet to use, it will be easier to review and we don't need to change anything for bulletin-solo-zombienet-start
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
reasonable! this function started to grow substantially.
I'll remember to make this refactor
…ts and kill processes from old runs -- very useful when you run test locally
|
State of the art:
Improvements:
Left to do:
|
hmm, it worked for me locally for the first run: |
Ehh, we do need a comprehensive docker setup to avoid things working on one OS and not working on another one :( |
# Conflicts: # .github/workflows/integration-test.yml # examples/justfile
…eanup - Add fallback to commit hash when bko-bulletin-para-support branch doesn't exist - Clean up zombienet directory before starting to prevent interactive prompts - Fixes 'Directory already exists' prompt that was blocking CI runs
When bko-bulletin-para-support branch doesn't exist, now falls back to origin/master to get the latest stable polkadot-sdk instead of an old pinned commit.
examples/justfile
Outdated
| # Setup prerequisites for Westend runtime (polkadot and polkadot-parachain binaries) | ||
| # This recipe clones polkadot-sdk, builds required binaries, and copies them to ~/local_bulletin_testing/bin | ||
| setup-westend-prerequisites: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know the function is long but splitting it into sub-functions will make the file even longer...
Not sure which one of two devils to choose?
examples/justfile
Outdated
| echo " Waiting for chain to start (15 seconds)..." | ||
| sleep 15 | ||
| echo " Waiting for chain to start (120 seconds)..." | ||
| sleep 120 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be nice to replace the fixed sleep 120 with a bounded readiness probe (e.g. polling RPC or logs) to make startup more reliable and avoid unnecessary CI delays.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, we already have it in our backlog.
The only purpose of this PR was to make those two flows work (and they do work!)
All the refactors and improvements will be handled separately
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@x3c41a add here please TODO or better let's fix it here now :)
| # Setup prerequisites for Westend runtime (polkadot and polkadot-parachain binaries) | ||
| # This recipe clones polkadot-sdk, builds required binaries, and copies them to ~/local_bulletin_testing/bin | ||
| setup-westend-prerequisites: | ||
| #!/usr/bin/env bash |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please extract this whole logic into separate *.sh file and execute it here, right now its a mix of just-style variables and bash variables and its hard to read
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
extracted into scripts/setup_westend_prerequisites.sh
examples/justfile
Outdated
| #!/usr/bin/env bash | ||
| set -e | ||
|
|
||
| BIN_DIR=~/local_bulletin_testing/bin |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please avoid using ~/$HOME in general its bad practice to permanently modify people system files outside of scope of this project files - I would use /tmp instead if needed, but in this case maybe polkadot-sdk git submodule would be good (as mentioned in other comment)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
neat! I'll remember about ~($HOME) vs /tmp!
Added all integration tests for Polkadot solochain and Westend parachain
Also:
justfilefunctionschainspecpathcommand line argument to examples/authorize_and_store_papi_smoldot.js