Agoric Upgrade 11 Tests -> JS #8237

iomekam · 2023-08-22T21:20:41Z

Description

Refactoring our bash scripts to look and feel more like Javascript. We've already competed agoric-upgrade-10. This change moves over agoric-upgrade-11 and agoric-upgrade-12.

Security Considerations

N/A

Scaling Considerations

N/A

Documentation Considerations

N/A

Testing Considerations

This change adds to our integration tests, which can be run with the force:integration tag.

Upgrade Considerations

N/A

dckc · 2023-08-24T20:29:26Z

about the zoe upgrade (#8121) and injecting bundleIDs - in order for the upgrade core-eval to fit inside a chain upgrade, we need to use the agoric run / writeCoreProposal approach

packages/deployment/upgrade-test/upgrade-test-scripts/agoric-upgrade-11/actions.mjs

packages/deployment/upgrade-test/upgrade-test-scripts/agoric-upgrade-12/actions.mjs

turadg · 2023-09-06T22:14:52Z

packages/deployment/upgrade-test/upgrade-test-scripts/agoric-upgrade-12/actions.mjs

+
+const dirname = dname(fileURLToPath(import.meta.url));
+
+export const installBundles = async bundlesData => {


this seems useful in more than one actions file. consider putting in coreUpgradeHelpers

#8237 (comment)

turadg · 2023-09-06T22:15:20Z

packages/deployment/upgrade-test/upgrade-test-scripts/agoric-upgrade-12/actions.mjs

+  return bundleIds;
+};
+
+export const prepForCoreEval = async (filePath, constants) => {


ditto, consider keeping in a helpers module

I'm thinking that this code may go away, as we may move this approach to the agoric run approach that was used for the wallet factory work

turadg · 2023-09-06T22:20:31Z

packages/deployment/upgrade-test/upgrade-test-scripts/agoric-upgrade-12/actions.test.js

+  t.context.bundleIds = await installBundles(bundlesData);
+});
+
+test.serial('Open Vaults', async t => {


important but for a follow-up PR:

the actions aren't test themselves. They're always in a strict order, which isn't really what Ava's .serial is meant for. (While it does execute in order, it's only guarantee is "not concurrently". Users should still be able to select any test in the file to run without the others.)

I think the actions module should just be a script without any Ava tests. In this file I see the only assertions are strict equality, which can be achieved with Node's built-in assert()

turadg · 2023-09-06T22:21:25Z

packages/deployment/upgrade-test/upgrade-test-scripts/agoric-upgrade-12/pre.test.js

+import { main } from './tools/vat-status.mjs';
+
+test.before(async () => {
+  console.log('Wait for upgrade to settle');


should the actions of the preceding upgrade be responsible for this?

I don't think so. I believe this here as we run pre.test.js shortly after performing the actual upgrade (which happens after the actions of the preceding upgrade)

what I mean is, the thing that is "performing the actual upgrade" has the job of upgrading. if the next step has to wait for the upgrade to settle, the the upgrade step hasn't finished it's job. The test should be able to assume the upgrade is complete. Knowing how much to wait for it to complete should be the responsibility of the upgrade, since it depends on the upgrade being performed.

packages/deployment/upgrade-test/upgrade-test-scripts/agoric-upgrade-12/tools/vat-status.mjs

packages/deployment/upgrade-test/upgrade-test-scripts/coreUpgradeHelpers.mjs

packages/deployment/upgrade-test/upgrade-test-scripts/agoric-upgrade-10/upgradeHelpers.mjs

turadg · 2023-09-07T17:24:02Z

packages/deployment/upgrade-test/Dockerfile

+
+WORKDIR /usr/src/agoric-sdk/upgrade-test-scripts
+RUN yarn
+RUN echo '. /usr/src/agoric-sdk/upgrade-test-scripts/env_setup.sh' >> ~/.bashrc 
+
+WORKDIR /usr/src/agoric-sdk/
+


these lines complicate the boilerplate. Without templating it's hard to see what's supposed to be part of each layer vs what's specific to the current.

Would the following work?

Suggested change

WORKDIR /usr/src/agoric-sdk/upgrade-test-scripts

RUN yarn

RUN echo '. /usr/src/agoric-sdk/upgrade-test-scripts/env_setup.sh' >> ~/.bashrc

WORKDIR /usr/src/agoric-sdk/

RUN cd upgrade-test-scripts && yarn

RUN echo '. /usr/src/agoric-sdk/upgrade-test-scripts/env_setup.sh' >> ~/.bashrc

Also why does every layer need to append to ~/.bashrc? Shouldn't doing it once at the top suffice? Hmm, ditto for the yarn install.

Each build stage is seperate from each other, so the layers we add in agoric-upgrade-10 won't be present in agoric-upgrade-11, which is why we have to do this for every stage.

Oh, I thought they were layered upon each other. I still haven't grokked this scheme

turadg · 2023-09-07T17:25:51Z

packages/deployment/upgrade-test/upgrade-test-scripts/coreUpgradeHelpers.js

"core" has a particular association with "core eval". Is it used here just to mean ones that are common to all upgrades? please choose a different adjective like "common" or "shared".

turadg · 2023-09-07T17:29:30Z

packages/deployment/upgrade-test/upgrade-test-scripts/agoric-upgrade-11/post.test.js

@@ -0,0 +1,5 @@
+import test from 'ava';
+
+test(`Replace me`, t => {


This should be replaced by the wallet factory work. The reason I have this filler here is because the runner expects to have a post.test.js file present with at least one test.

ok then please say that: replace with the walletFactory work when it's complete

packages/deployment/upgrade-test/upgrade-test-scripts/agoric-upgrade-12/post.test.js

packages/deployment/upgrade-test/upgrade-test-scripts/constants.js

turadg

Good enough to land imo. We will keep iterating.

Agoric Upgrade 11 Tests -> JS

turadg mentioned this pull request Aug 22, 2023

feat: upgrade walletFactory to go beyond vbank assets #8147

Merged

iomekam force-pushed the agoric-upgrade-11-js branch from 8eb2219 to 6fffcf2 Compare August 29, 2023 16:57

dckc mentioned this pull request Sep 1, 2023

revive zoe-full-upgrade test #8121

Closed

iomekam force-pushed the agoric-upgrade-11-js branch from 6fffcf2 to 3d611f7 Compare September 1, 2023 17:09

turadg requested changes Sep 6, 2023

View reviewed changes

iomekam force-pushed the agoric-upgrade-11-js branch from bf79c61 to 6d54530 Compare September 7, 2023 15:27

iomekam added the force:integration Force integration tests to run on PR label Sep 7, 2023

turadg reviewed Sep 7, 2023

View reviewed changes

turadg approved these changes Sep 7, 2023

View reviewed changes

iomekam force-pushed the agoric-upgrade-11-js branch from a9d117f to 25351c9 Compare September 7, 2023 21:35

iomekam marked this pull request as ready for review September 7, 2023 21:35

iomekam force-pushed the agoric-upgrade-11-js branch from 25351c9 to c270c4e Compare September 7, 2023 21:37

test: move ag11 and ag12 to JS

c5f7be4

iomekam force-pushed the agoric-upgrade-11-js branch from c270c4e to c5f7be4 Compare September 7, 2023 22:06

iomekam added the automerge:rebase Automatically rebase updates, then merge label Sep 8, 2023

mergify bot merged commit 2ee239f into master Sep 8, 2023
79 checks passed

mergify bot deleted the agoric-upgrade-11-js branch September 8, 2023 00:38

mhofman pushed a commit that referenced this pull request Nov 8, 2023

Merge pull request #8237 from Agoric/agoric-upgrade-11-js

0b7df19

Agoric Upgrade 11 Tests -> JS

mhofman pushed a commit that referenced this pull request Nov 10, 2023

Merge pull request #8237 from Agoric/agoric-upgrade-11-js

a852db9

Agoric Upgrade 11 Tests -> JS

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Agoric Upgrade 11 Tests -> JS #8237

Agoric Upgrade 11 Tests -> JS #8237

iomekam commented Aug 22, 2023 •

edited

Loading

dckc commented Aug 24, 2023

turadg Sep 6, 2023

iomekam Sep 7, 2023

turadg Sep 6, 2023

iomekam Sep 7, 2023

turadg Sep 6, 2023

turadg Sep 6, 2023

iomekam Sep 7, 2023

turadg Sep 7, 2023

turadg Sep 7, 2023

iomekam Sep 7, 2023

turadg Sep 7, 2023 •

edited

Loading

turadg Sep 7, 2023

turadg Sep 7, 2023

iomekam Sep 7, 2023

turadg Sep 7, 2023

turadg left a comment


		const dirname = dname(fileURLToPath(import.meta.url));

		export const installBundles = async bundlesData => {

		@@ -0,0 +1,5 @@
		import test from 'ava';

		test(`Replace me`, t => {

Agoric Upgrade 11 Tests -> JS #8237

Agoric Upgrade 11 Tests -> JS #8237

Conversation

iomekam commented Aug 22, 2023 • edited Loading

Description

Security Considerations

Scaling Considerations

Documentation Considerations

Testing Considerations

Upgrade Considerations

dckc commented Aug 24, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

turadg Sep 7, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

turadg left a comment

Choose a reason for hiding this comment

iomekam commented Aug 22, 2023 •

edited

Loading

turadg Sep 7, 2023 •

edited

Loading