From cef0ed07d00f46fbc893ef41e3be84a8df8b80eb Mon Sep 17 00:00:00 2001 From: David Berlin Date: Wed, 1 May 2024 11:03:06 +0300 Subject: [PATCH 1/7] fix: don't prompt for input when running in CI --- src/commands/base-command.ts | 10 +++++++ src/utils/build-info.ts | 10 +++++++ .../framework-detection.test.js.snap | 6 +++++ .../dev-miscellaneous.test.js.snap | 3 +++ .../commands/dev/dev-miscellaneous.test.js | 27 ++++++++++++++++++- tests/integration/framework-detection.test.js | 27 +++++++++++++++++++ 6 files changed, 82 insertions(+), 1 deletion(-) create mode 100644 tests/integration/commands/dev/__snapshots__/dev-miscellaneous.test.js.snap diff --git a/src/commands/base-command.ts b/src/commands/base-command.ts index 53114cfaaa3..f23186e36f4 100644 --- a/src/commands/base-command.ts +++ b/src/commands/base-command.ts @@ -1,3 +1,5 @@ +import { isCI } from 'ci-info' + import { existsSync } from 'fs' import { join, relative, resolve } from 'path' import process from 'process' @@ -103,6 +105,14 @@ async function selectWorkspace(project: Project, filter?: string): Promise pkg.name || pkg.path) + .join(', ')}.`, + ) + } + const { result } = await inquirer.prompt({ name: 'result', // @ts-expect-error TS(2769) FIXME: No overload matches this call. diff --git a/src/utils/build-info.ts b/src/utils/build-info.ts index 9250493949b..6facc9ef172 100644 --- a/src/utils/build-info.ts +++ b/src/utils/build-info.ts @@ -1,4 +1,5 @@ import { Settings } from '@netlify/build-info' +import { isCI } from 'ci-info' import fuzzy from 'fuzzy' import inquirer from 'inquirer' @@ -77,6 +78,15 @@ export const detectFrameworkSettings = async ( } if (settings.length > 1) { + if (isCI) { + log(`Multiple possible ${type} commands found`) + throw new Error( + `Update your settings to specify which to use. Detected commands for: ${settings + .map((setting) => setting.framework.name) + .join(', ')}.`, + ) + } + // multiple matching detectors, make the user choose const scriptInquirerOptions = formatSettingsArrForInquirer(settings, type) const { chosenSettings } = await inquirer.prompt<{ chosenSettings: Settings }>({ diff --git a/tests/integration/__snapshots__/framework-detection.test.js.snap b/tests/integration/__snapshots__/framework-detection.test.js.snap index 95c47bd12ac..5bce118c420 100644 --- a/tests/integration/__snapshots__/framework-detection.test.js.snap +++ b/tests/integration/__snapshots__/framework-detection.test.js.snap @@ -29,6 +29,12 @@ exports[`frameworks/framework-detection > should detect a known framework 1`] = ◈ Command failed with exit code *: npm run start. Shutting down Netlify Dev server" `; +exports[`frameworks/framework-detection > should fail in CI when multiple frameworks are detected 1`] = ` +"◈ Netlify Dev ◈ +Multiple possible dev commands found +◈ Update your settings to specify which to use. Detected commands for: Gatsby, Create React App." +`; + exports[`frameworks/framework-detection > should filter frameworks with no dev command 1`] = ` "◈ Netlify Dev ◈ ◈ No app server detected. Using simple static server diff --git a/tests/integration/commands/dev/__snapshots__/dev-miscellaneous.test.js.snap b/tests/integration/commands/dev/__snapshots__/dev-miscellaneous.test.js.snap new file mode 100644 index 00000000000..cce0005b2ed --- /dev/null +++ b/tests/integration/commands/dev/__snapshots__/dev-miscellaneous.test.js.snap @@ -0,0 +1,3 @@ +// Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html + +exports[`commands/dev-miscellaneous > should fail in CI with multiple projects 1`] = `"› Error: Configure the site you want to work with and try again. Sites detected: package1, package2."`; diff --git a/tests/integration/commands/dev/dev-miscellaneous.test.js b/tests/integration/commands/dev/dev-miscellaneous.test.js index e25ebc1a1b1..a373d59f43e 100644 --- a/tests/integration/commands/dev/dev-miscellaneous.test.js +++ b/tests/integration/commands/dev/dev-miscellaneous.test.js @@ -3,16 +3,19 @@ import path from 'path' import { fileURLToPath } from 'url' import { setProperty } from 'dot-prop' +import execa from 'execa' import getAvailablePort from 'get-port' import jwt from 'jsonwebtoken' import fetch from 'node-fetch' import { describe, test } from 'vitest' -import { withDevServer } from '../../utils/dev-server.ts' +import { cliPath } from '../../utils/cli-path.js' +import { getExecaOptions, withDevServer } from '../../utils/dev-server.ts' import got from '../../utils/got.js' import { withMockApi } from '../../utils/mock-api.js' import { pause } from '../../utils/pause.js' import { withSiteBuilder } from '../../utils/site-builder.ts' +import { normalize } from '../../utils/snapshots.js' // eslint-disable-next-line no-underscore-dangle const __dirname = path.dirname(fileURLToPath(import.meta.url)) @@ -1318,4 +1321,26 @@ describe.concurrent('commands/dev-miscellaneous', () => { ) }) }) + + test('should fail in CI with multiple projects', async (t) => { + await withSiteBuilder('site-with-multiple-packages', async (builder) => { + await builder + .withPackageJson({ packageJson: { name: 'main', workspaces: ['*'] } }) + .withPackageJson({ packageJson: { name: 'package1' }, pathPrefix: 'package1' }) + .withPackageJson({ packageJson: { name: 'package2' }, pathPrefix: 'package2' }) + .buildAsync() + + const asyncErrorBlock = async () => { + const childProcess = execa( + cliPath, + ['dev', '--offline'], + getExecaOptions({ cwd: builder.directory, env: { CI: true } }), + ) + await childProcess + } + const error = await asyncErrorBlock().catch((error_) => error_) + t.expect(normalize(error.stderr, { duration: true, filePath: true })).toMatchSnapshot() + t.expect(error.exitCode).toBe(1) + }) + }) }) diff --git a/tests/integration/framework-detection.test.js b/tests/integration/framework-detection.test.js index cbdcbc61e35..e5060c797cf 100644 --- a/tests/integration/framework-detection.test.js +++ b/tests/integration/framework-detection.test.js @@ -252,6 +252,33 @@ describe.concurrent('frameworks/framework-detection', () => { }) }) + test('should fail in CI when multiple frameworks are detected', async (t) => { + await withSiteBuilder('site-with-multiple-frameworks', async (builder) => { + await builder + .withPackageJson({ + packageJson: { + dependencies: { 'react-scripts': '1.0.0', gatsby: '^3.0.0' }, + scripts: { start: 'react-scripts start', develop: 'gatsby develop' }, + }, + }) + .withContentFile({ path: 'gatsby-config.js', content: '' }) + .buildAsync() + + // a failure is expected since this is not a true framework project + const asyncErrorBlock = async () => { + const childProcess = execa( + cliPath, + ['dev', '--offline'], + getExecaOptions({ cwd: builder.directory, env: { CI: true } }), + ) + await childProcess + } + const error = await asyncErrorBlock().catch((error_) => error_) + t.expect(normalize(error.stdout, { duration: true, filePath: true })).toMatchSnapshot() + t.expect(error.exitCode).toBe(1) + }) + }) + test('should not run framework detection if command and targetPort are configured', async (t) => { await withSiteBuilder('site-with-hugo-config', async (builder) => { await builder.withContentFile({ path: 'config.toml', content: '' }).buildAsync() From 2bf5ff8ee65b347e039ef64245bdf129ff8ccdbb Mon Sep 17 00:00:00 2001 From: David Berlin Date: Sun, 5 May 2024 11:35:03 +0300 Subject: [PATCH 2/7] fix: improve log messages --- src/commands/base-command.ts | 6 ++++-- src/utils/build-info.ts | 6 ++++-- .../__snapshots__/framework-detection.test.js.snap | 2 +- .../dev/__snapshots__/dev-miscellaneous.test.js.snap | 2 +- 4 files changed, 10 insertions(+), 6 deletions(-) diff --git a/src/commands/base-command.ts b/src/commands/base-command.ts index f23186e36f4..796a27e4df3 100644 --- a/src/commands/base-command.ts +++ b/src/commands/base-command.ts @@ -107,9 +107,11 @@ async function selectWorkspace(project: Project, filter?: string): Promise pkg.name || pkg.path) - .join(', ')}.`, + .join( + ', ', + )}. Configure the site you want to work with and try again. See docs at: https://docs.netlify.com/configure-builds/monorepos/#manual-configuration`, ) } diff --git a/src/utils/build-info.ts b/src/utils/build-info.ts index 6facc9ef172..08492683d76 100644 --- a/src/utils/build-info.ts +++ b/src/utils/build-info.ts @@ -81,9 +81,11 @@ export const detectFrameworkSettings = async ( if (isCI) { log(`Multiple possible ${type} commands found`) throw new Error( - `Update your settings to specify which to use. Detected commands for: ${settings + `Detected commands for: ${settings .map((setting) => setting.framework.name) - .join(', ')}.`, + .join( + ', ', + )}. Update your settings to specify which to use. See docs at: https://docs.netlify.com/configure-builds/file-based-configuration/#netlify-dev`, ) } diff --git a/tests/integration/__snapshots__/framework-detection.test.js.snap b/tests/integration/__snapshots__/framework-detection.test.js.snap index 5bce118c420..641779748ac 100644 --- a/tests/integration/__snapshots__/framework-detection.test.js.snap +++ b/tests/integration/__snapshots__/framework-detection.test.js.snap @@ -32,7 +32,7 @@ exports[`frameworks/framework-detection > should detect a known framework 1`] = exports[`frameworks/framework-detection > should fail in CI when multiple frameworks are detected 1`] = ` "◈ Netlify Dev ◈ Multiple possible dev commands found -◈ Update your settings to specify which to use. Detected commands for: Gatsby, Create React App." +◈ Detected commands for: Gatsby, Create React App. Update your settings to specify which to use. See docs at: https://docs.netlify.com/configure-builds/file-based-configuration/#netlify-dev" `; exports[`frameworks/framework-detection > should filter frameworks with no dev command 1`] = ` diff --git a/tests/integration/commands/dev/__snapshots__/dev-miscellaneous.test.js.snap b/tests/integration/commands/dev/__snapshots__/dev-miscellaneous.test.js.snap index cce0005b2ed..c2a94a508b0 100644 --- a/tests/integration/commands/dev/__snapshots__/dev-miscellaneous.test.js.snap +++ b/tests/integration/commands/dev/__snapshots__/dev-miscellaneous.test.js.snap @@ -1,3 +1,3 @@ // Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html -exports[`commands/dev-miscellaneous > should fail in CI with multiple projects 1`] = `"› Error: Configure the site you want to work with and try again. Sites detected: package1, package2."`; +exports[`commands/dev-miscellaneous > should fail in CI with multiple projects 1`] = `"› Error: Sites detected: package1, package2. Configure the site you want to work with and try again. See docs at: https://docs.netlify.com/configure-builds/monorepos/#manual-configuration"`; From 913aaa128178e910f07f9a0056654e0251c7ac38 Mon Sep 17 00:00:00 2001 From: David Berlin <47757213+davbree@users.noreply.github.com> Date: Mon, 6 May 2024 18:56:05 +0300 Subject: [PATCH 3/7] Update src/utils/build-info.ts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Eduardo Bouças --- src/utils/build-info.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/utils/build-info.ts b/src/utils/build-info.ts index 08492683d76..5bc3a5caf6b 100644 --- a/src/utils/build-info.ts +++ b/src/utils/build-info.ts @@ -85,7 +85,7 @@ export const detectFrameworkSettings = async ( .map((setting) => setting.framework.name) .join( ', ', - )}. Update your settings to specify which to use. See docs at: https://docs.netlify.com/configure-builds/file-based-configuration/#netlify-dev`, + )}. Update your settings to specify which to use. Refer to https://ntl.fyi/dev-monorepo for more information.`, ) } From dacaed9a7519038e83885a89f83942727bc5b9a2 Mon Sep 17 00:00:00 2001 From: David Berlin <47757213+davbree@users.noreply.github.com> Date: Mon, 6 May 2024 18:56:23 +0300 Subject: [PATCH 4/7] Update src/commands/base-command.ts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Eduardo Bouças --- src/commands/base-command.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/commands/base-command.ts b/src/commands/base-command.ts index f5051aea645..1f95e0b54e1 100644 --- a/src/commands/base-command.ts +++ b/src/commands/base-command.ts @@ -111,7 +111,7 @@ async function selectWorkspace(project: Project, filter?: string): Promise pkg.name || pkg.path) .join( ', ', - )}. Configure the site you want to work with and try again. See docs at: https://docs.netlify.com/configure-builds/monorepos/#manual-configuration`, + )}. Configure the site you want to work with and try again. Refer to https://ntl.fyi/configure-site for more information.`, ) } From e9e598595ef49c23a77af97d6fcc1d4ffa225f58 Mon Sep 17 00:00:00 2001 From: David Berlin Date: Mon, 6 May 2024 19:11:55 +0300 Subject: [PATCH 5/7] fix: snapshots --- .../integration/__snapshots__/framework-detection.test.js.snap | 2 +- .../commands/dev/__snapshots__/dev-miscellaneous.test.js.snap | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/integration/__snapshots__/framework-detection.test.js.snap b/tests/integration/__snapshots__/framework-detection.test.js.snap index 641779748ac..968f20716f9 100644 --- a/tests/integration/__snapshots__/framework-detection.test.js.snap +++ b/tests/integration/__snapshots__/framework-detection.test.js.snap @@ -32,7 +32,7 @@ exports[`frameworks/framework-detection > should detect a known framework 1`] = exports[`frameworks/framework-detection > should fail in CI when multiple frameworks are detected 1`] = ` "◈ Netlify Dev ◈ Multiple possible dev commands found -◈ Detected commands for: Gatsby, Create React App. Update your settings to specify which to use. See docs at: https://docs.netlify.com/configure-builds/file-based-configuration/#netlify-dev" +◈ Detected commands for: Gatsby, Create React App. Update your settings to specify which to use. Refer to https://ntl.fyi/dev-monorepo for more information." `; exports[`frameworks/framework-detection > should filter frameworks with no dev command 1`] = ` diff --git a/tests/integration/commands/dev/__snapshots__/dev-miscellaneous.test.js.snap b/tests/integration/commands/dev/__snapshots__/dev-miscellaneous.test.js.snap index c2a94a508b0..604294edef5 100644 --- a/tests/integration/commands/dev/__snapshots__/dev-miscellaneous.test.js.snap +++ b/tests/integration/commands/dev/__snapshots__/dev-miscellaneous.test.js.snap @@ -1,3 +1,3 @@ // Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html -exports[`commands/dev-miscellaneous > should fail in CI with multiple projects 1`] = `"› Error: Sites detected: package1, package2. Configure the site you want to work with and try again. See docs at: https://docs.netlify.com/configure-builds/monorepos/#manual-configuration"`; +exports[`commands/dev-miscellaneous > should fail in CI with multiple projects 1`] = `"› Error: Sites detected: package1, package2. Configure the site you want to work with and try again. Refer to https://ntl.fyi/configure-site for more information."`; From e1909caf1fc4aa692d8ad324bd78e5e0a959ae62 Mon Sep 17 00:00:00 2001 From: David Berlin Date: Mon, 6 May 2024 20:37:01 +0300 Subject: [PATCH 6/7] fix: override CI for our tests --- tests/integration/framework-detection.test.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tests/integration/framework-detection.test.js b/tests/integration/framework-detection.test.js index e5060c797cf..7a610ce28b5 100644 --- a/tests/integration/framework-detection.test.js +++ b/tests/integration/framework-detection.test.js @@ -236,7 +236,11 @@ describe.concurrent('frameworks/framework-detection', () => { // a failure is expected since this is not a true framework project const asyncErrorBlock = async () => { - const childProcess = execa(cliPath, ['dev', '--offline'], getExecaOptions({ cwd: builder.directory })) + const childProcess = execa( + cliPath, + ['dev', '--offline'], + getExecaOptions({ cwd: builder.directory, env: { CI: 'false' } }), + ) handleQuestions(childProcess, [ { From b0330cf67a45869163e35a5b620f5067c8fa1171 Mon Sep 17 00:00:00 2001 From: David Berlin Date: Mon, 6 May 2024 21:25:54 +0300 Subject: [PATCH 7/7] fix: continued work on tests --- .../__snapshots__/framework-detection.test.js.snap | 6 ------ .../dev/__snapshots__/dev-miscellaneous.test.js.snap | 3 --- tests/integration/commands/dev/dev-miscellaneous.test.js | 6 +++++- tests/integration/framework-detection.test.js | 6 +++++- 4 files changed, 10 insertions(+), 11 deletions(-) delete mode 100644 tests/integration/commands/dev/__snapshots__/dev-miscellaneous.test.js.snap diff --git a/tests/integration/__snapshots__/framework-detection.test.js.snap b/tests/integration/__snapshots__/framework-detection.test.js.snap index 968f20716f9..95c47bd12ac 100644 --- a/tests/integration/__snapshots__/framework-detection.test.js.snap +++ b/tests/integration/__snapshots__/framework-detection.test.js.snap @@ -29,12 +29,6 @@ exports[`frameworks/framework-detection > should detect a known framework 1`] = ◈ Command failed with exit code *: npm run start. Shutting down Netlify Dev server" `; -exports[`frameworks/framework-detection > should fail in CI when multiple frameworks are detected 1`] = ` -"◈ Netlify Dev ◈ -Multiple possible dev commands found -◈ Detected commands for: Gatsby, Create React App. Update your settings to specify which to use. Refer to https://ntl.fyi/dev-monorepo for more information." -`; - exports[`frameworks/framework-detection > should filter frameworks with no dev command 1`] = ` "◈ Netlify Dev ◈ ◈ No app server detected. Using simple static server diff --git a/tests/integration/commands/dev/__snapshots__/dev-miscellaneous.test.js.snap b/tests/integration/commands/dev/__snapshots__/dev-miscellaneous.test.js.snap deleted file mode 100644 index 604294edef5..00000000000 --- a/tests/integration/commands/dev/__snapshots__/dev-miscellaneous.test.js.snap +++ /dev/null @@ -1,3 +0,0 @@ -// Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html - -exports[`commands/dev-miscellaneous > should fail in CI with multiple projects 1`] = `"› Error: Sites detected: package1, package2. Configure the site you want to work with and try again. Refer to https://ntl.fyi/configure-site for more information."`; diff --git a/tests/integration/commands/dev/dev-miscellaneous.test.js b/tests/integration/commands/dev/dev-miscellaneous.test.js index a373d59f43e..ba6f03dbb07 100644 --- a/tests/integration/commands/dev/dev-miscellaneous.test.js +++ b/tests/integration/commands/dev/dev-miscellaneous.test.js @@ -1339,7 +1339,11 @@ describe.concurrent('commands/dev-miscellaneous', () => { await childProcess } const error = await asyncErrorBlock().catch((error_) => error_) - t.expect(normalize(error.stderr, { duration: true, filePath: true })).toMatchSnapshot() + t.expect( + normalize(error.stderr, { duration: true, filePath: true }).includes( + 'Sites detected: package1, package2. Configure the site you want to work with and try again. Refer to https://ntl.fyi/configure-site for more information.', + ), + ) t.expect(error.exitCode).toBe(1) }) }) diff --git a/tests/integration/framework-detection.test.js b/tests/integration/framework-detection.test.js index 7a610ce28b5..1ab1ce5a12b 100644 --- a/tests/integration/framework-detection.test.js +++ b/tests/integration/framework-detection.test.js @@ -278,7 +278,11 @@ describe.concurrent('frameworks/framework-detection', () => { await childProcess } const error = await asyncErrorBlock().catch((error_) => error_) - t.expect(normalize(error.stdout, { duration: true, filePath: true })).toMatchSnapshot() + t.expect( + normalize(error.stdout, { duration: true, filePath: true }).includes( + 'Detected commands for: Gatsby, Create React App. Update your settings to specify which to use. Refer to https://ntl.fyi/dev-monorepo for more information.', + ), + ) t.expect(error.exitCode).toBe(1) }) })