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

feat: proposal pre/post test steps #210

Merged
merged 31 commits into from
Feb 7, 2025
Merged
Show file tree
Hide file tree
Changes from 29 commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
2660fec
hooks
usmanmani1122 Jan 1, 2025
8e3f3e4
Merge branch "usman/prepare-test-hook" into branch "usman/before-afte…
usmanmani1122 Jan 1, 2025
df80864
nit
usmanmani1122 Jan 1, 2025
0554920
fixes
usmanmani1122 Jan 3, 2025
5d4e465
Merge branch 'main' into usman/before-after-test-scripts
usmanmani1122 Jan 3, 2025
2c94fef
document prepare-test hook
usmanmani1122 Jan 16, 2025
a689ecc
address comments
usmanmani1122 Jan 22, 2025
61371f5
Merge branch 'main' into usman/before-after-test-scripts
usmanmani1122 Jan 22, 2025
9f24947
increase test timeout
usmanmani1122 Jan 22, 2025
3d6780c
increase test timeout 2.0
usmanmani1122 Jan 22, 2025
5b4c029
fix for spawn
usmanmani1122 Jan 22, 2025
608e2da
:)
usmanmani1122 Jan 22, 2025
0ee2a77
increase test timeout 3.0
usmanmani1122 Jan 22, 2025
7ad7d92
revert
usmanmani1122 Jan 22, 2025
f55ef1f
address comments
usmanmani1122 Jan 23, 2025
8b22861
Empty
usmanmani1122 Jan 23, 2025
b89fe4f
Empty
usmanmani1122 Jan 23, 2025
6d5c227
Empty
usmanmani1122 Jan 23, 2025
f121f67
Merge branch 'main' into usman/before-after-test-scripts
usmanmani1122 Jan 24, 2025
679a083
Address comments
usmanmani1122 Jan 24, 2025
f983826
lint
usmanmani1122 Jan 24, 2025
36a2a20
use tmp directory
usmanmani1122 Jan 27, 2025
5d54940
nit
usmanmani1122 Jan 28, 2025
ef3d10b
address comments + fix bug
usmanmani1122 Jan 30, 2025
963e59f
lint
usmanmani1122 Jan 30, 2025
1a137a4
nit
usmanmani1122 Jan 31, 2025
12c5042
nit
usmanmani1122 Feb 4, 2025
ac2c07b
Merge branch 'main' into usman/before-after-test-scripts
usmanmani1122 Feb 4, 2025
b34bdb6
Merge branch 'main' into usman/before-after-test-scripts
usmanmani1122 Feb 7, 2025
73a9c4b
add test + host folder notes
usmanmani1122 Feb 7, 2025
9d67bc1
lint
usmanmani1122 Feb 7, 2025
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
4 changes: 4 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,10 @@ If the proposal is _pending_ and does not yet have a number, use a letter. The p
- `package.json` specifies what kind of proposal it is in a `agoricProposal` field. If it's a "Software Upgrade Proposal" it also includes additional parameters.
- `use.sh` is the script that will be run in the USE stage of the build
- `test.sh` is the script that will be _included_ in the TEST stage of the build, and run in CI
- `setup-test.sh` is an optional script which can be used to run setup steps _inside_ the container. It runs _before_ the chain starts.
- `teardown-test.sh` is an optional script which can be used to run teardown steps _inside_ the container. It runs _after_ the chain stops.
- `before-test-run.sh` is an optional script which can be used to run setup steps on the _host_ (like starting a follower). It runs _before_ the container launches. It needs to be placed in the **host** folder.
- `after-test-run.sh` is an optional script which can be used to run teardown steps on the _host_ (like stopping a follower). It runs _after_ the container exits. It needs to be placed in the **host** folder.
Copy link
Member

Choose a reason for hiding this comment

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

Let's be more explicit about what the host folder is for. There has been some confusion: Agoric/agoric-sdk#10947 (comment)

Suggested change
- `before-test-run.sh` is an optional script which can be used to run setup steps on the _host_ (like starting a follower). It runs _before_ the container launches. It needs to be placed in the **host** folder.
- `after-test-run.sh` is an optional script which can be used to run teardown steps on the _host_ (like stopping a follower). It runs _after_ the container exits. It needs to be placed in the **host** folder.
- `host` folder is for ___
- `host/before-test-run.sh` is an optional script which can be used to run setup steps on the _host_ (like starting a follower). It runs _before_ the container launches.
- `host/after-test-run.sh` is an optional script which can be used to run teardown steps on the _host_ (like stopping a follower). It runs _after_ the container exits.

While you're at it a change like this would help,

 - `test` folder is for files that should be included only in the `test` image

This is not blocking feedback. If something is delayed by this PR the docs improvements can come later.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added


## Development

Expand Down
4 changes: 3 additions & 1 deletion packages/synthetic-chain/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,15 @@
"chalk": "^5.4.1",
"cosmjs-types": "^0.9.0",
"execa": "^9.5.2",
"glob": "^11.0.1"
"glob": "^11.0.1",
"tmp": "0.2.3"
},
"devDependencies": {
"@agoric/cosmic-proto": "0.5.0-u18.5",
"@types/better-sqlite3": "^7.6.12",
"@types/glob": "^8.1.0",
"@types/node": "^18.19.50",
"@types/tmp": "0.2.6",
"ava": "^6.2.0",
"ts-blank-space": "^0.5.1",
"tsup": "^8.3.6",
Expand Down
12 changes: 9 additions & 3 deletions packages/synthetic-chain/public/upgrade-test-scripts/run_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,10 @@ source ./env_setup.sh

cd /usr/src/proposals/"$PROPOSAL/" || fail "Proposal $PROPOSAL does not exist"

if test -f prepare-test.sh
if test -f setup-test.sh
then
echo "[$PROPOSAL] Running prepare-test.sh"
./prepare-test.sh
echo "[$PROPOSAL] Running setup-test.sh"
./setup-test.sh
fi

echo "[$PROPOSAL] Starting agd"
Expand All @@ -44,3 +44,9 @@ echo "[$PROPOSAL] Running test.sh."
echo "[$PROPOSAL] Testing completed."

killAgd

if test -f teardown-test.sh
then
echo "[$PROPOSAL] Running teardown-test.sh"
./teardown-test.sh
fi
2 changes: 1 addition & 1 deletion packages/synthetic-chain/src/cli/cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ switch (cmd) {
console.log(chalk.cyan.bold(`Testing ${proposal.proposalName}`));
const image = imageNameForProposal(proposal, 'test');
bakeTarget(image.target, values.dry);
runTestImage(proposal);
runTestImage({ proposal });
// delete the image to reclaim disk space. The next build
// will use the build cache.
execSync('docker system df', { stdio: 'inherit' });
Expand Down
8 changes: 4 additions & 4 deletions packages/synthetic-chain/src/cli/dockerfileGen.ts
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ ENV \
SKIP_PROPOSAL_VALIDATION=${skipProposalValidation}

${createCopyCommand(
['node_modules', 'test', 'test.sh'],
['host', 'node_modules', 'test', 'test.sh'],
`./proposals/${path}`,
`/usr/src/proposals/${path}`,
)}
Expand Down Expand Up @@ -116,7 +116,7 @@ WORKDIR /usr/src/upgrade-test-scripts

# base is a fresh sdk image so set up the proposal and its dependencies
${createCopyCommand(
['node_modules', 'test', 'test.sh'],
['host', 'node_modules', 'test', 'test.sh'],
`./proposals/${path}`,
`/usr/src/proposals/${path}`,
)}
Expand Down Expand Up @@ -149,7 +149,7 @@ RUN ./run_execute.sh ${planName}
FROM use-${lastProposal.proposalName} as eval-${proposalName}

${createCopyCommand(
['node_modules', 'test', 'test.sh'],
['host', 'node_modules', 'test', 'test.sh'],
`./proposals/${path}`,
`/usr/src/proposals/${path}`,
)}
Expand Down Expand Up @@ -215,7 +215,7 @@ FROM use-${proposalName} as test-${proposalName}
# to copy only missing files, but there may be none. Fortunately, copying extra
# does not invalidate other images because nothing depends on this layer.
${createCopyCommand(
['node_modules'],
['host', 'node_modules'],
`./proposals/${path}`,
`/usr/src/proposals/${path}`,
)}
Expand Down
123 changes: 103 additions & 20 deletions packages/synthetic-chain/src/cli/run.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,29 @@
import { execSync } from 'node:child_process';
import { realpathSync } from 'node:fs';
import { spawnSync } from 'node:child_process';
import { existsSync, realpathSync } from 'node:fs';
import { resolve as resolvePath } from 'node:path';
import { fileSync as createTempFile } from 'tmp';
import { ProposalInfo, imageNameForProposal } from './proposals.js';

const createMessageFile = (proposal: ProposalInfo) =>
createTempFile({ prefix: proposal.proposalName });

const executeHostScriptIfPresent = (
extraEnv: typeof process.env,
proposal: ProposalInfo,
scriptName: string,
) => {
const scriptPath = `${resolvePath('.')}/proposals/${proposal.path}/host/${scriptName}`;
if (existsSync(scriptPath)) {
console.log(
`Running script ${scriptName} for proposal ${proposal.proposalName}`,
);
spawnSync(scriptPath, {
env: { ...process.env, ...extraEnv },
stdio: 'inherit',
});
}
};

/**
* Used to propagate a SLOGFILE environment variable into Docker containers.
* Any file identified by such a variable will be created if it does not already
Expand All @@ -14,40 +36,101 @@ const propagateSlogfile = env => {
const { SLOGFILE } = env;
if (!SLOGFILE) return [];

execSync('touch "$SLOGFILE"');
return ['-e', 'SLOGFILE', '-v', `"$SLOGFILE:${realpathSync(SLOGFILE)}"`];
spawnSync('touch', [SLOGFILE]);

return [
'--env',
`SLOGFILE=${SLOGFILE}`,
'--volume',
`${SLOGFILE}:${realpathSync(SLOGFILE)}`,
];
};

export const runTestImage = (proposal: ProposalInfo) => {
console.log(`Running test image for proposal ${proposal.proposalName}`);
const { name } = imageNameForProposal(proposal, 'test');
const slogOpts = propagateSlogfile(process.env);
// 'rm' to remove the container when it exits
const cmd = `docker run ${slogOpts.join(' ')} --rm ${name}`;
execSync(cmd, { stdio: 'inherit' });
export const runTestImage = ({
extraDockerArgs = [],
proposal,
removeContainerOnExit = true,
}: {
extraDockerArgs?: Array<string>;
proposal: ProposalInfo;
removeContainerOnExit?: boolean;
}) => {
const { name: messageFilePath, removeCallback: removeTempFileCallback } =
createMessageFile(proposal);

const containerFilePath = '/root/message-file-path';
Copy link
Member

Choose a reason for hiding this comment

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

Nit:

Suggested change
const containerFilePath = '/root/message-file-path';
const containerFilePath = '/root/host-test-message';


try {
executeHostScriptIfPresent(
{
MESSAGE_FILE_PATH: messageFilePath,
},
proposal,
'before-test-run.sh',
);

console.log(`Running test image for proposal ${proposal.proposalName}`);
const { name } = imageNameForProposal(proposal, 'test');
spawnSync(
'docker',
[
'run',
'--env',
`MESSAGE_FILE_PATH=${containerFilePath}`,
'--mount',
`source=${messageFilePath},target=${containerFilePath},type=bind`,
...(removeContainerOnExit ? ['--rm'] : []),
...propagateSlogfile(process.env),
...extraDockerArgs,
name,
],
{ stdio: 'inherit' },
);

executeHostScriptIfPresent(

Choose a reason for hiding this comment

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

So at this point our tests are being executed inside container, right?

{
MESSAGE_FILE_PATH: messageFilePath,
},
proposal,
'after-test-run.sh',
);
} finally {
removeTempFileCallback();
}
};

export const debugTestImage = (proposal: ProposalInfo) => {
const { name } = imageNameForProposal(proposal, 'test');
console.log(
`
Starting chain of test image for proposal ${proposal.proposalName}

To get an interactive shell in the container, use an IDE feature like "Attach Shell" or this command:'

docker exec -ti $(docker ps -q -f ancestor=${name}) bash

And within that shell:
cd /usr/src/proposals/${proposal.path} && ./test.sh

To edit files you can use terminal tools like vim, or mount the container in your IDE.
In VS Code the command is:
Dev Containers: Attach to Running Container...
`,
);

const slogOpts = propagateSlogfile(process.env);
// start the chain with ports mapped
const cmd = `docker run ${slogOpts.join(' ')} --publish 26657:26657 --publish 1317:1317 --publish 9090:9090 --interactive --tty --entrypoint /usr/src/upgrade-test-scripts/start_agd.sh ${name}`;
execSync(cmd, { stdio: 'inherit' });
return runTestImage({
extraDockerArgs: [
'--entrypoint',
'/usr/src/upgrade-test-scripts/start_agd.sh',
'--interactive',
'--publish',
'1317:1317',
'--publish',
'9090:9090',
'--publish',
'26657:26657',
'--tty',
],
proposal,
removeContainerOnExit: false,
});
};
22 changes: 19 additions & 3 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,14 @@ __metadata:
"@types/better-sqlite3": "npm:^7.6.12"
"@types/glob": "npm:^8.1.0"
"@types/node": "npm:^18.19.50"
"@types/tmp": "npm:0.2.6"
ava: "npm:^6.2.0"
better-sqlite3: "npm:^11.8.1"
chalk: "npm:^5.4.1"
cosmjs-types: "npm:^0.9.0"
execa: "npm:^9.5.2"
glob: "npm:^11.0.1"
tmp: "npm:0.2.3"
ts-blank-space: "npm:^0.5.1"
tsup: "npm:^8.3.6"
typescript: "npm:^5.7.3"
Expand Down Expand Up @@ -626,11 +628,18 @@ __metadata:
linkType: hard

"@types/node@npm:^18.19.50":
version: 18.19.74
resolution: "@types/node@npm:18.19.74"
version: 18.19.50
resolution: "@types/node@npm:18.19.50"
dependencies:
undici-types: "npm:~5.26.4"
checksum: 10c0/365d9cc2af934965aa6a8471e24ae80add815c15dc094e42a320c57c1ea5416032f0b7ef6f23e32174c34811fbb8d89ea8eaa1396548610fbb8ba317b6e93fbf
checksum: 10c0/36e6bc9eb47213ce94a868dad9504465ad89fba6af9f7954e22bb27fb17a32ac495f263d0cf4fdaee74becd7b2629609a446ec8c2b59b7a07bd587567c8a4782
languageName: node
linkType: hard

"@types/tmp@npm:0.2.6":
version: 0.2.6
resolution: "@types/tmp@npm:0.2.6"
checksum: 10c0/a11bfa2cd8eaa6c5d62f62a3569192d7a2c28efdc5c17af0b0551db85816b2afc8156f3ca15ac76f0b142ae1403f04f44279871424233a1f3390b2e5fc828cd0
languageName: node
linkType: hard

Expand Down Expand Up @@ -3326,6 +3335,13 @@ __metadata:
languageName: node
linkType: hard

"tmp@npm:0.2.3":
version: 0.2.3
resolution: "tmp@npm:0.2.3"
checksum: 10c0/3e809d9c2f46817475b452725c2aaa5d11985cf18d32a7a970ff25b568438e2c076c2e8609224feef3b7923fa9749b74428e3e634f6b8e520c534eef2fd24125
languageName: node
linkType: hard

"to-regex-range@npm:^5.0.1":
version: 5.0.1
resolution: "to-regex-range@npm:5.0.1"
Expand Down