Skip to content

Commit

Permalink
chore(e2e-tests): consistently wait for successful exits of shell
Browse files Browse the repository at this point in the history
Do not use unqualified `shell.waitForExit()` in cases where we
do not validate the exit code or check that no errors are displayed
in the output, just assuming that the shell exited successfully,
but instead intentionally always validate that these conditions are
true in the cases in which we expect them to be.
  • Loading branch information
addaleax committed Dec 12, 2024
1 parent f9a0631 commit f11536f
Show file tree
Hide file tree
Showing 9 changed files with 59 additions and 67 deletions.
6 changes: 3 additions & 3 deletions packages/e2e-tests/test/e2e-auth.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -998,7 +998,7 @@ describe('Auth e2e', function () {
],
});
if (
(await preTestShell.waitForExit()) === 1 &&
(await preTestShell.waitForAnyExit()) === 1 &&
preTestShell.output.match(
/digital envelope routines::unsupported|SSL routines::library has no ciphers/
)
Expand All @@ -1022,7 +1022,7 @@ describe('Auth e2e', function () {
'SCRAM-SHA-1',
],
});
await shell.waitForExit();
await shell.waitForAnyExit();
try {
shell.assertContainsOutput(
'Auth mechanism SCRAM-SHA-1 is not supported in FIPS mode'
Expand Down Expand Up @@ -1109,7 +1109,7 @@ describe('Auth e2e', function () {
'GSSAPI',
],
});
await shell.waitForExit();
await shell.waitForAnyExit();
// Failing to auth with kerberos fails with different error messages on each OS.
// Sometimes in CI, it also fails because the server received kerberos
// credentials, most likely because of a successful login by another
Expand Down
2 changes: 1 addition & 1 deletion packages/e2e-tests/test/e2e-direct.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -344,7 +344,7 @@ describe('e2e direct connection', function () {
const shell = this.startTestShell({
args: ['--host', hostlist, 'admin'],
});
await shell.waitForExit();
await shell.waitForAnyExit();
shell.assertContainsOutput('MongoServerSelectionError');
});

Expand Down
10 changes: 5 additions & 5 deletions packages/e2e-tests/test/e2e-oidc.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,7 @@ describe('OIDC auth e2e', function () {
'--browser=false',
],
});
await shell.waitForExit();
await shell.waitForAnyExit();
shell.assertContainsOutput(
'Consider specifying --oidcFlows=auth-code,device-auth if you are running mongosh in an environment without browser access'
);
Expand Down Expand Up @@ -405,7 +405,7 @@ describe('OIDC auth e2e', function () {
MONGOSH_E2E_TEST_CURL_ALLOW_INVALID_TLS: '1',
},
});
await shell.waitForExit();
await shell.waitForAnyExit();
// We cannot make the mongod server accept the mock IdP's certificate,
// so the best we can verify here is that auth failed *on the server*
shell.assertContainsOutput(/MongoServerError: Authentication failed/);
Expand Down Expand Up @@ -435,7 +435,7 @@ describe('OIDC auth e2e', function () {
},
});

await shell.waitForExit();
await shell.waitForAnyExit();
// We cannot make the mongod server accept the mock IdP's certificate,
// so the best we can verify here is that auth failed *on the server*
shell.assertContainsOutput(/MongoServerError: Authentication failed/);
Expand Down Expand Up @@ -499,7 +499,7 @@ describe('OIDC auth e2e', function () {
'--eval=42',
],
});
await shell.waitForExit();
await shell.waitForSuccessfulExit();

shell.assertContainsOutput('BEGIN OIDC TOKEN DUMP');
shell.assertContainsOutput('"tokenType": "Bearer"');
Expand All @@ -519,7 +519,7 @@ describe('OIDC auth e2e', function () {
'--eval=42',
],
});
await shell.waitForExit();
await shell.waitForSuccessfulExit();

shell.assertContainsOutput('BEGIN OIDC TOKEN DUMP');
shell.assertContainsOutput('"tokenType": "Bearer"');
Expand Down
4 changes: 2 additions & 2 deletions packages/e2e-tests/test/e2e-proxy.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -271,7 +271,7 @@ describe('e2e proxy support', function () {
},
});

const code = await shell.waitForExit();
const code = await shell.waitForAnyExit();
expect(code).to.equal(1);
});

Expand Down Expand Up @@ -622,7 +622,7 @@ describe('e2e proxy support', function () {
},
});

await shell.waitForExit();
await shell.waitForAnyExit();
// We cannot make the mongod server accept the mock IdP's certificate,
// so the best we can verify here is that auth failed *on the server*
shell.assertContainsOutput(/MongoServerError: Authentication failed/);
Expand Down
2 changes: 1 addition & 1 deletion packages/e2e-tests/test/e2e-snippet.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ describe('snippet integration tests', function () {
'config.set("snippetIndexSourceURLs", "http://localhost:1/")'
);
shell.writeInputLine('exit');
await shell.waitForExit();
await shell.waitForSuccessfulExit();

shell = makeTestShell();
await shell.waitForPrompt();
Expand Down
8 changes: 4 additions & 4 deletions packages/e2e-tests/test/e2e-tls.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -92,8 +92,8 @@ describe('e2e TLS', function () {
});
await shell.waitForPrompt();
await shell.executeLine('db.shutdownServer({ force: true })');
shell.kill();
await shell.waitForExit();
shell.writeInputLine('exit');
await shell.waitForAnyExit(); // closing the server may lead to an error being displayed
});

const server = startTestServer('e2e-tls-no-cli-valid-srv', {
Expand Down Expand Up @@ -399,8 +399,8 @@ describe('e2e TLS', function () {
});
await shell.waitForPrompt();
await shell.executeLine('db.shutdownServer({ force: true })');
shell.kill();
await shell.waitForExit();
shell.writeInputLine('exit');
await shell.waitForAnyExit(); // closing the server may lead to an error being displayed
});

const server = startTestServer('e2e-tls-valid-cli-valid-srv', {
Expand Down
74 changes: 30 additions & 44 deletions packages/e2e-tests/test/e2e.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,19 +35,17 @@ describe('e2e', function () {
describe('--version', function () {
it('shows version', async function () {
const shell = this.startTestShell({ args: ['--version'] });
await shell.waitForExit();
await shell.waitForSuccessfulExit();

shell.assertNoErrors();
shell.assertContainsOutput(require('../package.json').version);
});
});

describe('--build-info', function () {
it('shows build info in JSON format', async function () {
const shell = this.startTestShell({ args: ['--build-info'] });
await shell.waitForExit();
await shell.waitForSuccessfulExit();

shell.assertNoErrors();
const data = JSON.parse(shell.output);
expect(Object.keys(data)).to.deep.equal([
'version',
Expand Down Expand Up @@ -101,7 +99,7 @@ describe('e2e', function () {
'process.report.getReport()',
],
});
await shell.waitForExit();
await shell.waitForSuccessfulExit();
processReport = JSON.parse(shell.output);
}
expect(data.runtimeGlibcVersion).to.equal(
Expand All @@ -118,8 +116,7 @@ describe('e2e', function () {
'--nodb',
],
});
await shell.waitForExit();
shell.assertNoErrors();
await shell.waitForSuccessfulExit();
const deps = JSON.parse(shell.output);
expect(deps.nodeDriverVersion).to.be.a('string');
expect(deps.libmongocryptVersion).to.be.a('string');
Expand Down Expand Up @@ -176,34 +173,28 @@ describe('e2e', function () {
});
});
it('closes the shell when "exit" is entered', async function () {
const onExit = shell.waitForExit();
shell.writeInputLine('exit');
expect(await onExit).to.equal(0);
await shell.waitForSuccessfulExit();
});
it('closes the shell when "quit" is entered', async function () {
const onExit = shell.waitForExit();
shell.writeInputLine('quit');
expect(await onExit).to.equal(0);
await shell.waitForSuccessfulExit();
});
it('closes the shell with the specified exit code when "exit(n)" is entered', async function () {
const onExit = shell.waitForExit();
shell.writeInputLine('exit(42)');
expect(await onExit).to.equal(42);
expect(await shell.waitForAnyExit()).to.equal(42);
});
it('closes the shell with the specified exit code when "quit(n)" is entered', async function () {
const onExit = shell.waitForExit();
shell.writeInputLine('quit(42)');
expect(await onExit).to.equal(42);
expect(await shell.waitForAnyExit()).to.equal(42);
});
it('closes the shell with the pre-specified exit code when "exit" is entered', async function () {
const onExit = shell.waitForExit();
shell.writeInputLine('process.exitCode = 42; exit()');
expect(await onExit).to.equal(42);
expect(await shell.waitForAnyExit()).to.equal(42);
});
it('closes the shell with the pre-specified exit code when "quit" is entered', async function () {
const onExit = shell.waitForExit();
shell.writeInputLine('process.exitCode = 42; quit()');
expect(await onExit).to.equal(42);
expect(await shell.waitForAnyExit()).to.equal(42);
});
it('decorates internal errors with bug reporting information', async function () {
const err = await shell.executeLine(
Expand Down Expand Up @@ -300,10 +291,8 @@ describe('e2e', function () {
'sleep(100);print([1,2,3,4,5,6,7,8,9,10].reduce(\n(a,b) => { return a*b; }, 1))\n\n\n\n',
{ end: true }
);
const exitCode = await shell.waitForExit();
expect(exitCode).to.equal(0);
await shell.waitForSuccessfulExit();
shell.assertContainsOutput('3628800');
shell.assertNoErrors();
});
});

Expand Down Expand Up @@ -1095,9 +1084,8 @@ describe('e2e', function () {
'[db.hello()].reduce(\n() => { return 11111*11111; },0)\n\n\n',
{ end: true }
);
await shell.waitForExit();
await shell.waitForSuccessfulExit();
shell.assertContainsOutput('123454321');
shell.assertNoErrors();
});
});

Expand Down Expand Up @@ -1162,8 +1150,7 @@ describe('e2e', function () {
// when run under coverage, as we currently specify the location of
// coverage files via a relative path and nyc fails to write to that
// when started from a changed cwd.
await shell.waitForExit();
shell.assertNoErrors();
await shell.waitForSuccessfulExit();
});

if (!jsContextFlags.includes('--jsContext=plain-vm')) {
Expand Down Expand Up @@ -1202,7 +1189,7 @@ describe('e2e', function () {
await eventually(() => {
shell.assertContainsOutput('Error: uh oh');
});
expect(await shell.waitForExit()).to.equal(1);
expect(await shell.waitForAnyExit()).to.equal(1);
});
}
});
Expand All @@ -1216,8 +1203,7 @@ describe('e2e', function () {
await eventually(() => {
shell.assertContainsOutput('hello one');
});
expect(await shell.waitForExit()).to.equal(0);
shell.assertNoErrors();
await shell.waitForSuccessfulExit();
});

if (!jsContextFlags.includes('--jsContext=plain-vm')) {
Expand All @@ -1244,7 +1230,7 @@ describe('e2e', function () {
await eventually(() => {
shell.assertContainsOutput('Error: uh oh');
});
expect(await shell.waitForExit()).to.equal(1);
expect(await shell.waitForAnyExit()).to.equal(1);
});

it('fails with the error if the loaded script throws asynchronously (setImmediate)', async function () {
Expand All @@ -1259,7 +1245,7 @@ describe('e2e', function () {
await eventually(() => {
shell.assertContainsOutput('Error: uh oh');
});
expect(await shell.waitForExit()).to.equal(
expect(await shell.waitForAnyExit()).to.equal(
jsContextFlags.includes('--jsContext=repl') ? 0 : 1
);
});
Expand All @@ -1276,7 +1262,7 @@ describe('e2e', function () {
await eventually(() => {
shell.assertContainsOutput('Error: uh oh');
});
expect(await shell.waitForExit()).to.equal(
expect(await shell.waitForAnyExit()).to.equal(
jsContextFlags.includes('--jsContext=repl') ? 0 : 1
);
});
Expand All @@ -1299,7 +1285,7 @@ describe('e2e', function () {
script,
],
});
expect(await shell.waitForExit()).to.equal(0);
await shell.waitForSuccessfulExit();

// Check that:
// - the script runs in the expected environment
Expand Down Expand Up @@ -1512,7 +1498,7 @@ describe('e2e', function () {
}
await shell.executeLine('a = 42');
shell.writeInput('.exit\n');
await shell.waitForExit();
await shell.waitForSuccessfulExit();

shell = await startTestShell();
// Arrow up twice to skip the .exit line
Expand All @@ -1521,7 +1507,7 @@ describe('e2e', function () {
expect(shell.output).to.include('a = 42');
});
shell.writeInput('\n.exit\n');
await shell.waitForExit();
await shell.waitForSuccessfulExit();

expect(await fs.readFile(historyPath, 'utf8')).to.match(/^a = 42$/m);
});
Expand All @@ -1533,7 +1519,7 @@ describe('e2e', function () {

await shell.executeLine('a = 42');
shell.writeInput('.exit\n');
await shell.waitForExit();
await shell.waitForSuccessfulExit();

expect((await fs.stat(historyPath)).mode & 0o077).to.equal(0);
});
Expand All @@ -1544,7 +1530,7 @@ describe('e2e', function () {
await shell.executeLine('a = 42');
await shell.executeLine('foo = "bar"');
shell.writeInput('.exit\n');
await shell.waitForExit();
await shell.waitForAnyExit(); // db.auth() call fails because of --nodb

const contents = await fs.readFile(historyPath, 'utf8');
expect(contents).to.not.match(/mypassword/);
Expand Down Expand Up @@ -1603,7 +1589,7 @@ describe('e2e', function () {
`config.set("updateURL", ${JSON.stringify(httpServerUrl)})`
);
shell.writeInputLine('exit');
await shell.waitForExit();
await shell.waitForSuccessfulExit();
}

delete env.CI;
Expand All @@ -1623,13 +1609,13 @@ describe('e2e', function () {
).to.be.a('string');
});
shell.writeInputLine('exit');
await shell.waitForExit();
await shell.waitForSuccessfulExit();
}

{
const shell = await startTestShell();
shell.writeInputLine('exit');
await shell.waitForExit();
await shell.waitForSuccessfulExit();
shell.assertContainsOutput(
'mongosh 2023.4.15 is available for download: https://www.mongodb.com/try/download/shell'
);
Expand Down Expand Up @@ -1799,7 +1785,7 @@ describe('e2e', function () {
'.mongodb.net/',
],
});
const exitCode = await shell.waitForExit();
const exitCode = await shell.waitForAnyExit();
expect(exitCode).to.equal(1);
});

Expand All @@ -1813,7 +1799,7 @@ describe('e2e', function () {
'.mongodb.net/',
],
});
const exitCode = await shell.waitForExit();
const exitCode = await shell.waitForAnyExit();
expect(exitCode).to.equal(1);
});

Expand Down Expand Up @@ -1904,7 +1890,7 @@ describe('e2e', function () {
);

shell.writeInputLine('exit');
await shell.waitForExit();
await shell.waitForSuccessfulExit();
shell.assertNoErrors();
});
});
Expand Down Expand Up @@ -1937,7 +1923,7 @@ describe('e2e', function () {
args: [filename],
env: { ...process.env, MONGOSH_RUN_NODE_SCRIPT: '1' },
});
expect(await shell.waitForExit()).to.equal(0);
await shell.waitForSuccessfulExit();
shell.assertContainsOutput('610');
});
});
Expand Down
Loading

0 comments on commit f11536f

Please sign in to comment.