feat(ci): Check that generated bindings are up to date #1955
Conversation
| @@ -4,9 +4,10 @@ function generate_did() { | |||
| local canister=$1 | |||
| canister_root="src/$canister" | |||
There was a problem hiding this comment.
Recompile the backend only if the Wasm is not there already.
scripts/generate.sh
Outdated
| # Create local .did files | ||
| ./scripts/did.sh | ||
| # Download .did files without an explicit url in dfx.json | ||
| ./scripts/download.icp.sh |
There was a problem hiding this comment.
We probably don't require download icp, ckbtc and cketh as we do no generate their types. Any particular reason why you download those here?
There was a problem hiding this comment.
Running a blanket dfx generate, as in the original package.json, will fail if some candid files are missing, like this:
Generating type declarations for canister icp_ledger:
Error: Failed while trying to generate type declarations for 'icp_ledger'.
Caused by: Failed while trying to generate type declarations for 'icp_ledger'.
Candid file: /home/max/dfn/oisy-wallet/branches/check-generate-in-ci/target/ic/icp_ledger.did doesn't exist.
I guess one could generate only the canisters for which there are directories in declarations, like this:
for canister in $(ls src/declarations/) ; do echo "Generating bindings for $canister" ; dfx generate "$canister" ; done
Is that what you mean? I think it's a good idea. Less files to download => faster execution. :-)
There was a problem hiding this comment.
I did not knew. I think we should just get rid of dfx generate and directly use candid-extractor.
That's what I do in Juno. e.g. https://github.com/junobuild/juno/blob/main/scripts/did.sh
I can provide a PR to do so once we have merged this PR.
| # | ||
| # .did files are placed in the local .dfx directory, where the .did files are expected by `dfx generate` and other commands. | ||
| function install_did_files() { | ||
| jq -r '.canisters | to_entries | .[] | select(.value.candid != null) | "\(.key) \(.value.candid)"' dfx.json | |
There was a problem hiding this comment.
should we add a check that assert if jq is installed at the top of the script? "You don't have jq, install jq to execute this script"
There was a problem hiding this comment.
Yes, can do. Added to the --help and added a check.
bitdivine
force-pushed
the
check-generate-in-ci
branch
from
4d521ec
to
de2b716
Compare
|
Merged but happy to revisit if any of the changes were not as imagined. |
Motivation
There is no check in CI that candid files are up to date.
Secondary (& real!!) motivation: Frustration with
dfx generaterequiring deployment. Really annoying when trying to iterate an API. But might as well use the fast command for something useful, now that we have it.Changes
Tests