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.

Sign up for GitHub

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(integration-test): use dapp-offer-up as target for getting-start… #8829

Merged
merged 15 commits into from
Mar 13, 2024
Merged

feat(integration-test): use dapp-offer-up as target for getting-start… #8829

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
e4f65ce
feat(integration-test): use dapp-offer-up as target for getting-start…
LuqiPan Jan 26, 2024
8f19ada
fix(integration-test): bump node version from 16.x to 18.x
LuqiPan Jan 26, 2024
a033926
debug(integration-test): try node-version 18.18
LuqiPan Jan 26, 2024
a573b6f
debug(integration-test): comment out tests related to agoric deploy
LuqiPan Jan 26, 2024
c66fdc6
fix(integration-test): update UI test case
LuqiPan Jan 27, 2024
184994c
debug(integration-test): use 5173 as port number explicitly
LuqiPan Jan 27, 2024
5e5e348
fix(integration-test): remove another test that's based on testDeploy
LuqiPan Jan 27, 2024
210c4fe
chore: remove unused variable
LuqiPan Mar 11, 2024
9e65b2c
ci(integration-test): run a3p container via service container
LuqiPan Mar 12, 2024
d687300
test(integration-test): update the tests to follow current getting-st…
LuqiPan Mar 12, 2024
3a3711c
ci(integration-test): run yarn start:docker via getting-started.js
LuqiPan Mar 12, 2024
ba76a1e
ci(integration-test): remove service container
LuqiPan Mar 12, 2024
5a4b29c
chore(integration-test): fix linting errors
LuqiPan Mar 12, 2024
93435bc
format(integration-test): let prettier run
LuqiPan Mar 12, 2024
794d8fe
chore(integration-test): update comments and variable name
LuqiPan Mar 12, 2024
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
2 changes: 1 addition & 1 deletion .github/workflows/integration.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ jobs:
# Prerequisites
- uses: ./.github/actions/restore-node
with:
node-version: 16.x
node-version: 18.18
Copy link
Member

Choose a reason for hiding this comment

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

It looks like the getting started test still clones agoric-sdk. That's no longer part of the workflow.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think we still need to clone agoric-sdk, because with yarn create @agoric/dapp ... we're implicitly testing the behavior of init subcommand of agoric-cli package: https://github.com/Agoric/agoric-sdk/blob/master/packages/agoric-cli/src/main.js#L124-L140

Copy link
Member

Choose a reason for hiding this comment

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

Manual testing has confirmed that we don't need to clone agoric-sdk. yarn create @agoric/dapp gets code to run agoric init from npm, not from an agoric-sdk clone. I sure hope we can do likewise in ci.

This ci job says something about a local registry. That's perhaps the way yarn create @agoric/dapp ... gets access to the code under test.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This ci job says something about a local registry. That's perhaps the way yarn create @agoric/dapp ... gets access to the code under test.

Yeah I think we start a local NPM registry before running getting-started tests and we publish via a CI specific tag here.

...actually I'm not so sure anymore, just pushed a debug commit to weed out what's using local code and what's using npmjs.com

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK I just confirmed that yarn create @agoric/dapp ... will always pull from npmjs.com and I was not able to find a way to make it pull from local registry with my very limited yarn knowledge. I also confirmed that agoric init ... will use the local registry and code.

Here's what I propose:

  1. in agoric-sdk repo, we use agoric init ... to initialize dapp-offer-up (this PR)
    1. this way, we ensure integration tests in agoric-sdk repo are testing the behavior of agoric init ... at git HEAD, for example, if I change DEFAULT_DAPP_TEMPLATE to something erroneous, integration tests will fail right away
    2. merging this PR would also unblock @turadg's PRs to deprecate support for node 16, add support for node 20, and migrate from yarn classic to yarn modern
  2. in dapp-offer-up repo, we use yarn create @agoric/dapp ... to initialize dapp-offer-up followed by integration tests
    1. this way, we ensure integration tests in dapp-offer-up repo are testing the behavior of agoric init ... that's published on npmjs.com
    2. this work is not scheduled, and IMO is not high priority

@dckc sounds like a plan?

Copy link
Member

Choose a reason for hiding this comment

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

yes. sounds good.

keep-endo: 'true'

# Select a branch on dapp to test against by adding text to the body of the
Expand Down
169 changes: 33 additions & 136 deletions packages/agoric-cli/tools/getting-started.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/* global process setTimeout clearTimeout setInterval clearInterval */
/* global process setTimeout setInterval clearInterval */

import fs from 'fs';
import path from 'path';
Expand All @@ -10,32 +10,21 @@ import { spawn } from 'child_process';

import { makePspawn } from '../src/helpers.js';

const TIMEOUT_SECONDS = 20 * 60;
const TIMEOUT_SECONDS = 3 * 60;

const dirname = new URL('./', import.meta.url).pathname;

// To keep in sync with https://agoric.com/documentation/getting-started/
// To keep in sync with https://docs.agoric.com/guides/getting-started/

// Note that we currently only test:
// agoric init dapp-foo
// agoric install
// agoric start --reset
// agoric deploy ./contract/deploy.js ./api/deploy.js
// (For simple-exchange and autoswap, the above also makes and accepts offers)
// cd ui && yarn install
// cd ui && yarn start
// yarn install
// yarn start:docker
// yarn start:contract
// yarn start:ui

export const gettingStartedWorkflowTest = async (t, options = {}) => {
const {
init: initOptions = [],
install: installOptions = [],
start: startOptions = [],
testUnsafePlugins = false,
} = options;
// FIXME: Do a search for an unused port or allow specification.
const PORT = '7999';
process.env.PORT = PORT;

const { init: initOptions = [] } = options;
const pspawn = makePspawn({ spawn });

// Kill an entire process group.
Expand All @@ -61,7 +50,6 @@ export const gettingStartedWorkflowTest = async (t, options = {}) => {
const { AGORIC_CMD = JSON.stringify(defaultAgoricCmd()) } = process.env;
const agoricCmd = JSON.parse(AGORIC_CMD);
function myMain(args, opts = {}) {
// console.error('running agoric-cli', ...extraArgs, ...args);
return pspawnStdout(agoricCmd[0], [...agoricCmd.slice(1), ...args], {
stdio: ['ignore', 'pipe', 'inherit'],
env: { ...process.env, DEBUG: 'agoric:debug' },
Expand All @@ -70,6 +58,14 @@ export const gettingStartedWorkflowTest = async (t, options = {}) => {
});
}

function yarn(args) {
return pspawnStdout('yarn', args, {
stdio: ['ignore', 'pipe', 'inherit'],
env: { ...process.env },
detached: true,
});
}

const olddir = process.cwd();
const { name } = tmp.dirSync({
unsafeCleanup: true,
Expand All @@ -83,7 +79,7 @@ export const gettingStartedWorkflowTest = async (t, options = {}) => {
try {
f();
} catch (e) {
// console.log(e);
console.log(e);
}
}
if (sig) {
Expand All @@ -105,122 +101,32 @@ export const gettingStartedWorkflowTest = async (t, options = {}) => {
initOptions.push(...opts);
}
t.is(
await myMain([
'init',
'--dapp-template',
'dapp-fungible-faucet',
...initOptions,
'dapp-foo',
]),
await myMain(['init', ...initOptions, 'dapp-foo']),
0,
Copy link
Member

Choose a reason for hiding this comment

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

As of last Christmas, getting started no longer uses the agoric CLI.

This step should probably be yarn create @agoric/dapp --dapp-template dapp-offer-up dapp-foo

'init dapp-foo works',
);
process.chdir('dapp-foo');

// ==============
// agoric install
if (process.env.AGORIC_INSTALL_OPTIONS) {
const opts = JSON.parse(process.env.AGORIC_INSTALL_OPTIONS);
installOptions.push(...opts);
}
t.is(await myMain(['install', ...installOptions]), 0, 'install works');
// yarn install
t.is(await yarn(['install']), 0, 'yarn install works');

// ==============
// agoric start --reset
const startResult = makePromiseKit();

if (process.env.AGORIC_START_OPTIONS) {
const opts = JSON.parse(process.env.AGORIC_START_OPTIONS);
startOptions.push(...opts);
}

// TODO: Allow this to work even if the port is already used.
const startP = myMain(['start', '--reset', ...startOptions]);
finalizers.push(() => pkill(startP.childProcess, 'SIGINT'));

let stdoutStr = '';
if (startP.childProcess.stdout) {
startP.childProcess.stdout.on('data', chunk => {
// console.log('stdout:', chunk.toString());
stdoutStr += chunk.toString();
if (stdoutStr.match(/(^|:\s+)swingset running$/m)) {
startResult.resolve(true);
}
});
}
startP.childProcess.on('close', code =>
startResult.reject(Error(`early termination: ${code}`)),
);
// yarn start:docker
t.is(await yarn(['start:docker']), 0, 'yarn start:docker works');

const timeout = setTimeout(
startResult.reject,
TIMEOUT_SECONDS * 1000,
'timeout',
);
t.is(await startResult.promise, true, `swingset running before timeout`);
clearTimeout(timeout);

const openP = myMain(['open', '--no-browser'], {
stdio: ['ignore', 'pipe', 'inherit'],
});
t.is(await openP, 0, `open --no-browser exits successfully`);

const testDeploy = async (deployCmd, opts = {}) => {
const deployResult = makePromiseKit();
const deployP = myMain(
['deploy', `--hostport=127.0.0.1:${PORT}`, ...deployCmd],
{
stdio: [opts.stdin ? 'pipe' : 'ignore', 'pipe', 'inherit'],
},
);

if (opts.stdin) {
// Write the input to stdin.
deployP.childProcess.stdin.write(opts.stdin);
deployP.childProcess.stdin.end();
}

finalizers.push(() => pkill(deployP.childProcess, 'SIGINT'));
const to = setTimeout(
deployResult.resolve,
TIMEOUT_SECONDS * 1000,
'timeout',
);
const done = await Promise.race([deployResult.promise, deployP]);
t.is(done, 0, `deploy ${deployCmd.join(' ')} successful before timeout`);
clearTimeout(to);
};
// XXX: use abci_info endpoint to get block height
// sleep to let contract start
await new Promise(resolve => setTimeout(resolve, TIMEOUT_SECONDS));

// ==============
// agoric deploy ./contract/deploy.js ./api/deploy.js
await testDeploy(['./contract/deploy.js', './api/deploy.js']);

for (const [suffix, code] of [
['/notthere', 404],
['', 200],
]) {
let urlResolve;
const url = `http://127.0.0.1:${PORT}${suffix}`;
const urlP = new Promise(resolve => (urlResolve = resolve));
const urlReq = request(url, res => urlResolve(res.statusCode));
urlReq.setTimeout(2000);
urlReq.on('error', err => urlResolve(`Cannot connect to ${url}: ${err}`));
urlReq.end();
const urlTimeout = setTimeout(urlResolve, 3000, 'timeout');
const urlDone = await urlP;
clearTimeout(urlTimeout);
t.is(urlDone, code, `${url} gave status ${code}`);
}
// yarn start:contract
t.is(await yarn(['start:contract']), 0, 'yarn start:contract works');

// ==============
// cd ui && yarn start
const uiStartP = pspawn(`yarn`, ['start'], {
stdio: ['ignore', 'inherit', 'inherit'],
cwd: 'ui',
env: { ...process.env, PORT: '3000' },
detached: true,
});
finalizers.push(() => pkill(uiStartP.childProcess, 'SIGINT'));
// yarn start:ui
const startUiP = yarn(['start:ui']);
finalizers.push(() => pkill(startUiP.childProcess, 'SIGINT'));
const uiListening = makePromiseKit();
let retries = 0;
const ival = setInterval(() => {
Expand All @@ -236,7 +142,7 @@ export const gettingStartedWorkflowTest = async (t, options = {}) => {
return;
}

const req = request('http://localhost:3000/', _res => {
const req = request('http://localhost:5173/', _res => {
resolve('listening');
});
req.setTimeout(2000);
Expand All @@ -251,20 +157,11 @@ export const gettingStartedWorkflowTest = async (t, options = {}) => {
}
}, 3000);
t.is(
await Promise.race([uiStartP, uiListening.promise]),
await Promise.race([startUiP, uiListening.promise]),
'listening',
`cd ui && yarn start succeeded`,
`yarn start:ui succeeded`,
);
clearInterval(ival);

// Test that the Node.js `-r esm`-dependent plugin works.
await (testUnsafePlugins &&
Copy link
Contributor Author

Choose a reason for hiding this comment

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

remark

I don't know what these ESM plugin tests are about so I removed them

Copy link
Member

Choose a reason for hiding this comment

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

I think we can forget about resm-plugin

testDeploy(
['--allow-unsafe-plugins', `${dirname}/resm-plugin/deploy.js`],
{ stdin: 'yes\n' },
));

// TODO: When it exists, Test that the Node.js native ESM plugin works.
} finally {
runFinalizers();
process.off('SIGINT', runFinalizers);
Expand Down
Loading