Skip to content

chore: test cli upgrade#12506

Merged
Maddiaa0 merged 9 commits intomasterfrom
mt/test-upgrade-script
Mar 12, 2025
Merged

chore: test cli upgrade#12506
Maddiaa0 merged 9 commits intomasterfrom
mt/test-upgrade-script

Conversation

@just-mitch
Copy link
Collaborator

Ensures we have coverage on the primary means we currently have to upgrade the rollup.

@just-mitch just-mitch requested a review from charlielye as a code owner March 6, 2025 00:14
@just-mitch just-mitch added the ci-full Run all master checks. label Mar 6, 2025
@just-mitch just-mitch linked an issue Mar 6, 2025 that may be closed by this pull request
@just-mitch
Copy link
Collaborator Author

See passing run of the test here

@just-mitch just-mitch enabled auto-merge (squash) March 6, 2025 13:58

set -eu

NAMESPACE=$1
Copy link
Member

@Maddiaa0 Maddiaa0 Mar 11, 2025

Choose a reason for hiding this comment

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

non blocking / for future: This is just a duplication of what we have in jest, I'd much rather we have one code path and run the upgrade_rollup_with_lock script from there and not have two places to debug things

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I agree there are kind of two entrypoints right now- CLI and jest.

Are you suggesting we should invoke the CLI from within jest?

@Maddiaa0 Maddiaa0 disabled auto-merge March 11, 2025 10:06
;;
"test-cli-upgrade-with-lock")
env
FRESH_INSTALL=false INSTALL_METRICS=false \
Copy link
Member

Choose a reason for hiding this comment

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

INSTALL_METRICS=false however 1-validators.yaml runs with telemetry enabled, might want to override that to not pollute log output

Copy link
Member

Choose a reason for hiding this comment

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

also with 1-validators.yaml this test will use full slot / block times and will take longer than it needs to

@Maddiaa0
Copy link
Member

note: i ran this locally and it never cleaned up the cluster

@just-mitch just-mitch force-pushed the mt/test-upgrade-script branch from 697fbc0 to e40bc3a Compare March 11, 2025 21:15
@just-mitch
Copy link
Collaborator Author

Passing run of CLI via jest http://ci.aztec-labs.com/d0a90cac404a869a

@just-mitch just-mitch removed the ci-full Run all master checks. label Mar 11, 2025
});
if (json) {
log(JSON.stringify({ proposalId }, null, 2));
log(JSON.stringify({ proposalId: Number(proposalId) }, null, 2));
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Great example of why CLI tests are good haha. Had a "don't know how to serialize bigint" here.

Copy link
Member

Choose a reason for hiding this comment

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

you can also use the jsonStringify function in foundation

@Maddiaa0 Maddiaa0 merged commit fc728f3 into master Mar 12, 2025
6 checks passed
@Maddiaa0 Maddiaa0 deleted the mt/test-upgrade-script branch March 12, 2025 10:14
const config = setupEnvironment(process.env);

// technically it doesn't require a k8s env, but it doesn't seem we're keeping the "local" versions of the spartan scripts up to date
// and I didn't want to plumb through the config and test this with the local scripts.
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need the local scripts or would it make sense if anyway stale to kill em?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good question. @ludamad ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

That's a team call, I asked recently and Sean gave feedback about using them, but I certainly don't right now. I'm a bit surprised that people don't find the interactive usage useful, but again your guy's call

forwardProcesses.push(process);
ETHEREUM_HOSTS = `http://127.0.0.1:${port}`;
}
pxe = await createCompatibleClient(PXE_URL, debugLogger);
Copy link
Contributor

Choose a reason for hiding this comment

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

Unrelated to this pr, but the createCompatibleClient always makes me chuggle because it indicates there is a createIncompatibleClient

sklppy88 pushed a commit that referenced this pull request Mar 12, 2025
🤖 I have created a new Aztec Packages release
---


##
[0.80.0](v0.79.0...v0.80.0)
(2025-03-12)


### ⚠ BREAKING CHANGES

* disable PXE concurrency
([#12637](#12637))

### Features

* add inspect command to bootstrap
([#12641](#12641))
([2f0c527](2f0c527))
* disable PXE concurrency
([#12637](#12637))
([2716898](2716898))
* remove public bytecode from ts ContractArtifact
([#12653](#12653))
([6345158](6345158))


### Bug Fixes

* **cli:** fix tag resolution for `aztec update --contract`
([#12657](#12657))
([b58c123](b58c123))
* **docs:** Fix link to source code after code snippets
([#12658](#12658))
([8bc5daf](8bc5daf))
* fix yarn-project typos in release
([#12652](#12652))
([93a6f4e](93a6f4e))
* handling undefined block number in sync data provider txe
([#12605](#12605))
([b764a9a](b764a9a))
* Inject blob sink client config to archiver start
([#12675](#12675))
([e2e857b](e2e857b))
* Load config from env on archiver start
([#12662](#12662))
([79579f3](79579f3))


### Miscellaneous

* aztec start should use pxe proving by default
([#12676](#12676))
([80cd840](80cd840)),
closes
[#12677](#12677)
* downgrade undici so its engine spec is compatible
([#12659](#12659))
([4f815ea](4f815ea)),
closes
[#12645](#12645)
* fix warning in aztec-nr
([#12647](#12647))
([3831bab](3831bab))
* Merge is part of verification queue
([#12612](#12612))
([e324582](e324582))
* Remove roots arg on __compute_fracs in blob lib
([#12663](#12663))
([8ec910b](8ec910b))
* replace relative paths to noir-protocol-circuits
([f684528](f684528))
* test cli upgrade
([#12506](#12506))
([fc728f3](fc728f3))


### Documentation

* **feat:** Aztec js intro page
([#11804](#11804))
([12d8f3f](12d8f3f))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

test: cli upgrade

4 participants