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

8605 a3p integration #8635

Merged
merged 7 commits into from
Jan 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 1 addition & 8 deletions .eslintrc.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,7 @@ module.exports = {
{
files: ['*.ts'],
rules: {
'jsdoc/require-param-type': 'off',
// TS has this covered and eslint gets it wrong
'no-undef': 'off',
},
Expand All @@ -156,14 +157,6 @@ module.exports = {
project: false,
},
},
{
files: ['packages/**/upgrade-test-scripts/**/*.*js'],
rules: {
// NOTE: This rule is enabled for the repository in general. We turn it
// off for test code for now.
'@jessie.js/safe-await-separator': 'off',
},
},
{
// Types files have no promises to lint and that linter chokes on the .d.ts twin.
// Maybe due to https://github.com/typescript-eslint/typescript-eslint/issues/7435
Expand Down
20 changes: 11 additions & 9 deletions .github/workflows/integration.yml
Original file line number Diff line number Diff line change
Expand Up @@ -231,15 +231,17 @@ jobs:
# Produces ghcr.io/agoric/agoric-sdk:latest used in the following upgrade test.
# run: cd packages/deployment && ./scripts/test-docker-build.sh | $TEST_COLLECT
# XXX skip TAP test output and collection for now; it hides the output from the logs
Comment on lines 231 to 233
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this comment obsolete?

Copy link
Member Author

Choose a reason for hiding this comment

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

Nope. That script is a wrapper for test-docker-build which is still part of CI for building the SDK image. This PR doesn't have anything do with that.

run: cd packages/deployment && make docker-build-sdk
- name: docker build upgrade test
run: |
cd packages/deployment/upgrade-test && \
docker build \
--build-arg DEST_IMAGE=ghcr.io/agoric/agoric-sdk:latest \
-t docker-upgrade-test:latest -f Dockerfile upgrade-test-scripts
- name: docker run upgrade final stage
run: docker run --env "DEST=0" docker-upgrade-test:latest
run: make docker-build-sdk
working-directory: packages/deployment
- name: setup a3p-integration
run: yarn install
working-directory: a3p-integration
- name: build proposals tests
run: yarn synthetic-chain append
working-directory: a3p-integration
- name: run proposals tests
run: yarn synthetic-chain test
working-directory: a3p-integration
- name: notify on failure
if: failure() && github.event_name != 'pull_request'
uses: ./.github/actions/notify-status
Expand Down
12 changes: 12 additions & 0 deletions a3p-integration/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
# build in CI
Dockerfile
upgrade-test-scripts

# Yarn (https://yarnpkg.com/getting-started/qa#which-files-should-be-gitignored)
proposals/*/.pnp.*
proposals/*/.yarn/*
proposals/*/!.yarn/patches
proposals/*/!.yarn/plugins
proposals/*/!.yarn/releases
proposals/*/!.yarn/sdks
proposals/*/!.yarn/versions
50 changes: 50 additions & 0 deletions a3p-integration/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
# Integration with agoric-3 synthetic test chain

The test runner is `@agoric/synthetic-chain`. This package depends on that so that you can run,
```
yarn synthetic-chain append
yarn synthetic-chain test
yarn synthetic-chain test --debug
```

# Package management

This directory hierarchy, while it contains packages, is not part of the agoric-sdk workspace. This is to isolate it from tooling that expects a public package published to NPM.

For each proposal, their package.json is also separate but it can't access the SDK code. Instead you must either source a published version of `@agoric/synthetic-chain` (e.g. a `dev` version published on each master commit) or pack a tarball and source that.

```
cd packages/synthetic-chain
yarn pack
TARBALL=`ls *.tgz`
cd -
mv packages/synthetic-chain/$TARBALL a3p-integration/proposals/c:myproposal/
# .tgz are gitignored at the root but a closer .gitignore makes an exception for this package's tarball
git add a3p-integration/proposals/c:myproposal/$TARBALL
yarn add @agoric/synthetic-chain@file:$TARBALL
```

# Troubleshooting

## no match for platform

If you get an error like this,
```
ERROR: failed to solve: ghcr.io/agoric/agoric-3-proposals:main: no match for platform in manifest sha256:83321abda66fa94915f1ae20d651b66870f2d1aac17b71449c04ecd46b6b1b96: not found
```
it's because our CI only builds x64 yet and you're on some other machine, probably a Mac.

There is some effort to make CI build multiplatform: https://github.com/Agoric/agoric-3-proposals/pull/32

Meanwhile you can build the `main` image locally:

```sh
cd agoric-3-proposals
./node_modules/.bin/synthetic-chain build

# build the default entrypoint and tag it so the `append` command finds it
docker buildx build --tag ghcr.io/agoric/agoric-3-proposals:main .
```
12 changes: 12 additions & 0 deletions a3p-integration/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
{
"private": true,
"scripts": {
"build": "echo Use synthetic-chain to build proposal images",
"test": "echo Use synthetic-chain to test proposal images"
Copy link
Member

Choose a reason for hiding this comment

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

can't this run synthetic-chain test ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, I just thought it better to train people on the tool rather than creating another level of abstraction.

},
"dependencies": {
"@agoric/synthetic-chain": "^0.0.2-0",
"tsx": "^4.7.0"
},
"license": "Apache-2.0"
}
1 change: 1 addition & 0 deletions a3p-integration/proposals/a:upgrade-next/.yarnrc.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
nodeLinker: node-modules
20 changes: 20 additions & 0 deletions a3p-integration/proposals/a:upgrade-next/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
{
"agoricProposal": {
"releaseNotes": "TBD",
"sdkImageTag": "latest",
"planName": "UNRELEASED_UPGRADE",
"type": "Software Upgrade Proposal"
},
"type": "module",
"license": "Apache-2.0",
"dependencies": {
"@agoric/synthetic-chain": "^0.0.2-0",
"ava": "^5.3.1",
"better-sqlite3": "^9.2.2",
"execa": "^7.2.0"
},
"scripts": {
"agops": "yarn --cwd /usr/src/agoric-sdk/ --silent agops"
},
"packageManager": "[email protected]"
}
26 changes: 26 additions & 0 deletions a3p-integration/proposals/a:upgrade-next/sanity.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
/**
* @file Sanity checks of master branch
*
* This 'upgrade-next' test is a dummy on this branch since there is no upgrade to be tested.
* This file serves as a starting point for tests on release branchs.
*/
import test from 'ava';

import { agd } from '@agoric/synthetic-chain/src/lib/cliHelper.js';
import { getIncarnation } from '@agoric/synthetic-chain/src/lib/vat-status.js';

test('MaxBytes param', async t => {
const { value: rawParams } = await agd.query(
'params',
'subspace',
'baseapp',
'BlockParams',
);
const blockParams = JSON.parse(rawParams);
t.is(blockParams.max_bytes, '5242880');
});

test(`Ensure Zoe Vat is at 0`, async t => {
const incarnation = await getIncarnation('zoe');
t.is(incarnation, 0);
});
4 changes: 4 additions & 0 deletions a3p-integration/proposals/a:upgrade-next/test.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
#!/bin/bash
source /usr/src/upgrade-test-scripts/env_setup.sh

yarn ava
5 changes: 5 additions & 0 deletions a3p-integration/proposals/a:upgrade-next/use.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
#!/bin/bash

# UNTIl this is upstream https://github.com/Agoric/agoric-3-proposals/issues/40
# Set to zero so tests don't have to pay gas (we're not testing that)
sed --in-place=.bak s/'minimum-gas-prices = ""'/'minimum-gas-prices = "0ubld,0uist"'/ ~/.agoric/config/app.toml
Comment on lines +3 to +5
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
# UNTIl this is upstream https://github.com/Agoric/agoric-3-proposals/issues/40
# Set to zero so tests don't have to pay gas (we're not testing that)
sed --in-place=.bak s/'minimum-gas-prices = ""'/'minimum-gas-prices = "0ubld,0uist"'/ ~/.agoric/config/app.toml
# UNTIL this is upstream https://github.com/Agoric/agoric-3-proposals/issues/40
# Set to zero so tests don't have to pay gas (we're not testing that)
sed -i .bak -e 's/minimum-gas-prices = ""/minimum-gas-prices = "0ubld,0uist"/' ~/.agoric/config/app.toml

-e says the next arg is an expression. Single quotes can capture the entire substitution expression, which can then include spaces and doublequotes. --in-place=.bak didn't work on my version of sed. does -i work for you?

Copy link
Member Author

Choose a reason for hiding this comment

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

-e says the next arg is an expression. Single quotes can capture the entire substitution expression, which can then include spaces and doublequotes.

Sound like you think that would read better. I'll make those changes.

--in-place=.bak didn't work on my version of sed. does -i work for you?

Your version like on your Mac? IIRC it didn't work in Ubuntu, where this line runs.

Copy link
Contributor

@Chris-Hibbert Chris-Hibbert Dec 15, 2023

Choose a reason for hiding this comment

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

Your version like on your Mac?

Yeah. nothing special, just what's here.

Loading
Loading