chore(e2e-tests): consistently wait for successful exits of shell MONGOSH-1943 (#2301)

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.

Author: addaleax

packages/e2e-tests/test/e2e-auth.spec.ts

@@ -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/
             )
@@ -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'
@@ -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

packages/e2e-tests/test/e2e-direct.spec.ts

@@ -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');
         });
 

packages/e2e-tests/test/e2e-oidc.spec.ts

@@ -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'
     );
@@ -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/);
@@ -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/);
@@ -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"');
@@ -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"');

packages/e2e-tests/test/e2e-proxy.spec.ts

@@ -271,7 +271,7 @@ describe('e2e proxy support', function () {
       },
     });
 
-    const code = await shell.waitForExit();
+    const code = await shell.waitForAnyExit();
     expect(code).to.equal(1);
   });
 
@@ -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/);

packages/e2e-tests/test/e2e-snippet.spec.ts

@@ -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();

packages/e2e-tests/test/e2e-tls.spec.ts

@@ -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', {
@@ -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', {

packages/e2e-tests/test/e2e.spec.ts

@@ -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',
@@ -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(
@@ -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');
@@ -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(
@@ -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();
     });
   });
 
@@ -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();
     });
   });
 
@@ -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')) {
@@ -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);
           });
         }
       });
@@ -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')) {
@@ -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 () {
@@ -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
           );
         });
@@ -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
           );
         });
@@ -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
@@ -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
@@ -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);
         });
@@ -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);
         });
@@ -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/);
@@ -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;
@@ -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'
             );
@@ -1799,7 +1785,7 @@ describe('e2e', function () {
             '.mongodb.net/',
         ],
       });
-      const exitCode = await shell.waitForExit();
+      const exitCode = await shell.waitForAnyExit();
       expect(exitCode).to.equal(1);
     });
 
@@ -1813,7 +1799,7 @@ describe('e2e', function () {
             '.mongodb.net/',
         ],
       });
-      const exitCode = await shell.waitForExit();
+      const exitCode = await shell.waitForAnyExit();
       expect(exitCode).to.equal(1);
     });
 
@@ -1904,7 +1890,7 @@ describe('e2e', function () {
       );
 
       shell.writeInputLine('exit');
-      await shell.waitForExit();
+      await shell.waitForSuccessfulExit();
       shell.assertNoErrors();
     });
   });
@@ -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');
     });
   });