From 392352086e2a552c122c9f23083c56c9990222c4 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Thu, 28 Mar 2024 13:28:13 +0100 Subject: [PATCH 1/2] fix(cli-repl): properly skip update notification fetches in quiet mode MONGOSH-1751 --- packages/cli-repl/src/cli-repl.spec.ts | 25 +++++++++++++++++++++++++ packages/cli-repl/src/cli-repl.ts | 5 +++-- 2 files changed, 28 insertions(+), 2 deletions(-) diff --git a/packages/cli-repl/src/cli-repl.spec.ts b/packages/cli-repl/src/cli-repl.spec.ts index fbacb645f..4d4a10c6e 100644 --- a/packages/cli-repl/src/cli-repl.spec.ts +++ b/packages/cli-repl/src/cli-repl.spec.ts @@ -992,6 +992,31 @@ describe('CliRepl', function () { ); }); }); + + it('does not attempt to load information about new releases with --eval and no explicit --no-quiet', async function () { + cliReplOptions.shellCliOptions.eval = ['1+1']; + cliRepl = new CliRepl(cliReplOptions); + let fetchingUpdateMetadataCalls = 0; + cliRepl.bus.on( + 'mongosh:fetching-update-metadata', + () => fetchingUpdateMetadataCalls++ + ); + await startWithExpectedImmediateExit(cliRepl, ''); + expect(fetchingUpdateMetadataCalls).to.equal(0); + }); + + it('does attempt to load information about new releases in --no-quiet mode', async function () { + cliReplOptions.shellCliOptions.eval = ['1+1']; + cliReplOptions.shellCliOptions.quiet = false; + cliRepl = new CliRepl(cliReplOptions); + let fetchingUpdateMetadataCalls = 0; + cliRepl.bus.on( + 'mongosh:fetching-update-metadata', + () => fetchingUpdateMetadataCalls++ + ); + await startWithExpectedImmediateExit(cliRepl, ''); + expect(fetchingUpdateMetadataCalls).to.equal(1); + }); }); verifyAutocompletion({ diff --git a/packages/cli-repl/src/cli-repl.ts b/packages/cli-repl/src/cli-repl.ts index 069ccff93..12a3c241b 100644 --- a/packages/cli-repl/src/cli-repl.ts +++ b/packages/cli-repl/src/cli-repl.ts @@ -1187,8 +1187,9 @@ export class CliRepl implements MongoshIOProvider { } async fetchMongoshUpdateUrl() { + const { quiet } = CliRepl.getFileAndEvalInfo(this.cliOptions); if ( - this.cliOptions.quiet || + quiet || process.env.CI || process.env.IS_CI || this.isContainerizedEnvironment @@ -1224,7 +1225,7 @@ export class CliRepl implements MongoshIOProvider { } } - async getMoreRecentMongoshVersion() { + async getMoreRecentMongoshVersion(): Promise { const { version } = require('../package.json'); return await this.updateNotificationManager.getLatestVersionIfMoreRecent( process.env From 3e691f24532e72fe55bd6307d18d53ce5db9caf0 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Tue, 2 Apr 2024 19:04:58 +0200 Subject: [PATCH 2/2] fixup: properly account for CI environments, test actual http call in unit tests --- packages/cli-repl/src/cli-repl.spec.ts | 83 ++++++++++++++++++-------- packages/cli-repl/src/cli-repl.ts | 8 ++- 2 files changed, 63 insertions(+), 28 deletions(-) diff --git a/packages/cli-repl/src/cli-repl.spec.ts b/packages/cli-repl/src/cli-repl.spec.ts index 4d4a10c6e..689198a75 100644 --- a/packages/cli-repl/src/cli-repl.spec.ts +++ b/packages/cli-repl/src/cli-repl.spec.ts @@ -2,7 +2,8 @@ import { MongoshInternalError } from '@mongosh/errors'; import { bson } from '@mongosh/service-provider-core'; import { once } from 'events'; import { promises as fs } from 'fs'; -import http from 'http'; +import type { Server as HTTPServer } from 'http'; +import http, { createServer as createHTTPServer } from 'http'; import path from 'path'; import type { Duplex } from 'stream'; import { PassThrough } from 'stream'; @@ -29,6 +30,7 @@ import type { CliReplOptions } from './cli-repl'; import { CliRepl } from './cli-repl'; import { CliReplErrors } from './error-codes'; import type { DevtoolsConnectOptions } from '@mongosh/service-provider-server'; +import type { AddressInfo } from 'net'; const { EJSON } = bson; const delay = promisify(setTimeout); @@ -138,7 +140,7 @@ describe('CliRepl', function () { const content = await fs.readFile(path.join(tmpdir.path, 'config'), { encoding: 'utf8', }); - expect((EJSON.parse(content) as any).enableTelemetry).to.be.false; + expect(EJSON.parse(content).enableTelemetry).to.be.false; }); it('does not store config options on disk that have not been changed', async function () { @@ -188,7 +190,7 @@ describe('CliRepl', function () { const content = await fs.readFile(path.join(tmpdir.path, 'config'), { encoding: 'utf8', }); - expect((EJSON.parse(content) as any).inspectDepth).equal(Infinity); + expect(EJSON.parse(content).inspectDepth).equal(Infinity); }); it('emits exit when asked to, Node.js-style', async function () { @@ -993,29 +995,60 @@ describe('CliRepl', function () { }); }); - it('does not attempt to load information about new releases with --eval and no explicit --no-quiet', async function () { - cliReplOptions.shellCliOptions.eval = ['1+1']; - cliRepl = new CliRepl(cliReplOptions); - let fetchingUpdateMetadataCalls = 0; - cliRepl.bus.on( - 'mongosh:fetching-update-metadata', - () => fetchingUpdateMetadataCalls++ - ); - await startWithExpectedImmediateExit(cliRepl, ''); - expect(fetchingUpdateMetadataCalls).to.equal(0); - }); + context('showing update information', function () { + let httpServer: HTTPServer; + let httpServerUrl: string; - it('does attempt to load information about new releases in --no-quiet mode', async function () { - cliReplOptions.shellCliOptions.eval = ['1+1']; - cliReplOptions.shellCliOptions.quiet = false; - cliRepl = new CliRepl(cliReplOptions); - let fetchingUpdateMetadataCalls = 0; - cliRepl.bus.on( - 'mongosh:fetching-update-metadata', - () => fetchingUpdateMetadataCalls++ - ); - await startWithExpectedImmediateExit(cliRepl, ''); - expect(fetchingUpdateMetadataCalls).to.equal(1); + beforeEach(async function () { + httpServer = createHTTPServer((req, res) => { + res.end( + JSON.stringify({ + versions: [{ version: '2023.4.15' }], + }) + ); + }); + httpServer.listen(0); + await once(httpServer, 'listening'); + httpServerUrl = `http://127.0.0.1:${ + (httpServer.address() as AddressInfo).port + }`; + }); + + afterEach(async function () { + httpServer.close(); + await once(httpServer, 'close'); + }); + + it('does not attempt to load information about new releases with --eval and no explicit --no-quiet', async function () { + cliReplOptions.shellCliOptions.eval = ['1+1']; + cliRepl = new CliRepl(cliReplOptions); + cliRepl.fetchMongoshUpdateUrlRegardlessOfCiEnvironment = true; + cliRepl.config.updateURL = httpServerUrl; + let fetchingUpdateMetadataCalls = 0; + cliRepl.bus.on( + 'mongosh:fetching-update-metadata', + () => fetchingUpdateMetadataCalls++ + ); + await startWithExpectedImmediateExit(cliRepl, ''); + expect(fetchingUpdateMetadataCalls).to.equal(0); + }); + + it('does attempt to load information about new releases in --no-quiet mode', async function () { + cliReplOptions.shellCliOptions.eval = ['1+1']; + cliReplOptions.shellCliOptions.quiet = false; + cliRepl = new CliRepl(cliReplOptions); + cliRepl.fetchMongoshUpdateUrlRegardlessOfCiEnvironment = true; + cliRepl.config.updateURL = httpServerUrl; + let fetchingUpdateMetadataCalls = 0; + cliRepl.bus.on( + 'mongosh:fetching-update-metadata', + () => fetchingUpdateMetadataCalls++ + ); + const requestPromise = once(httpServer, 'request'); + await startWithExpectedImmediateExit(cliRepl, ''); + expect(fetchingUpdateMetadataCalls).to.equal(1); + await requestPromise; + }); }); }); diff --git a/packages/cli-repl/src/cli-repl.ts b/packages/cli-repl/src/cli-repl.ts index 12a3c241b..a86d6cac5 100644 --- a/packages/cli-repl/src/cli-repl.ts +++ b/packages/cli-repl/src/cli-repl.ts @@ -120,6 +120,7 @@ export class CliRepl implements MongoshIOProvider { isContainerizedEnvironment = false; hasOnDiskTelemetryId = false; updateNotificationManager = new UpdateNotificationManager(); + fetchMongoshUpdateUrlRegardlessOfCiEnvironment = false; // for testing /** * Instantiate the new CLI Repl. @@ -1190,9 +1191,10 @@ export class CliRepl implements MongoshIOProvider { const { quiet } = CliRepl.getFileAndEvalInfo(this.cliOptions); if ( quiet || - process.env.CI || - process.env.IS_CI || - this.isContainerizedEnvironment + (!this.fetchMongoshUpdateUrlRegardlessOfCiEnvironment && + (process.env.CI || + process.env.IS_CI || + this.isContainerizedEnvironment)) ) { // No point in telling users about new versions if we are in // a CI or Docker-like environment. or the user has explicitly