From b6173e643c6671a8d9bc3186b052373bbdd14786 Mon Sep 17 00:00:00 2001 From: Luna Wei Date: Thu, 15 Feb 2024 11:18:05 -0800 Subject: [PATCH] Remove process.exit calls from publish-npm and throw errors instead (#43039) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/43039 Changelog: [Internal] - `publish-npm.js` is a [script we call in our CI](https://www.internalfb.com/code/fbsource/[c0b8566ac0d66c2c0282eeb597bfb54bedf757c6]/xplat/js/react-native-github/.circleci/configurations/jobs.yml?lines=1243) to publish the react-native package and others. Currently, the script leverages `exit/process.exit` to terminate early in a couple of places which makes the code hard to test because our tests don't truly early exit when `exit/process.exit` is called. This change removes any explicit `exit` calls and instead leverages the uncaught error to terminate the process and set the non-zero exit code. This makes our tests more accurate to the real control flow of the script. I've also updated the tests to better capture what we're actually testing by mocking at a higher level. Reviewed By: cipolleschi Differential Revision: D53792754 fbshipit-source-id: 9293bb9a95430c50052db36c0e6f6c1ba348107f --- .../releases-ci/__tests__/publish-npm-test.js | 418 +++++++++++------- scripts/releases-ci/publish-npm.js | 38 +- .../utils/__tests__/version-utils-test.js | 8 + 3 files changed, 278 insertions(+), 186 deletions(-) diff --git a/scripts/releases-ci/__tests__/publish-npm-test.js b/scripts/releases-ci/__tests__/publish-npm-test.js index 78de2089177083..27c6cb31b7fdc5 100644 --- a/scripts/releases-ci/__tests__/publish-npm-test.js +++ b/scripts/releases-ci/__tests__/publish-npm-test.js @@ -4,313 +4,407 @@ * This source code is licensed under the MIT license found in the * LICENSE file in the root directory of this source tree. * + * @flow strict-local * @format * @oncall react_native */ const execMock = jest.fn(); -const echoMock = jest.fn(); -const exitMock = jest.fn(); -const consoleErrorMock = jest.fn(); +const consoleLogMock = jest.fn(); const isTaggedLatestMock = jest.fn(); const setVersionMock = jest.fn(); const setReactNativeVersionMock = jest.fn(); const publishAndroidArtifactsToMavenMock = jest.fn(); const removeNewArchFlags = jest.fn(); const env = process.env; - const publishPackageMock = jest.fn(); const getNpmInfoMock = jest.fn(); - -jest - .mock('shelljs', () => ({ - exec: execMock, - echo: echoMock, - exit: exitMock, - })) - .mock('./../../scm-utils', () => ({ - exitIfNotOnGit: command => command(), - getCurrentCommit: () => 'currentco_mmit', - isTaggedLatest: isTaggedLatestMock, - })) - .mock('../../releases/utils/release-utils', () => ({ - generateAndroidArtifacts: jest.fn(), - publishAndroidArtifactsToMaven: publishAndroidArtifactsToMavenMock, - })) - .mock('../../releases/set-version', () => setVersionMock) - .mock('../../releases/set-rn-version', () => ({ - setReactNativeVersion: setReactNativeVersionMock, - })) - .mock('../../releases/remove-new-arch-flags', () => ({ - removeNewArchFlags, - })); - -const date = new Date('2023-04-20T23:52:39.543Z'); +const generateAndroidArtifactsMock = jest.fn(); +const getPackagesMock = jest.fn(); const {publishNpm} = require('../publish-npm'); const path = require('path'); const REPO_ROOT = path.resolve(__filename, '../../../..'); -let consoleError; +let consoleLog; describe('publish-npm', () => { beforeAll(() => { - jest.setSystemTime(date); + jest + .mock('shelljs', () => ({ + exec: execMock, + })) + .mock('./../../scm-utils', () => ({ + exitIfNotOnGit: command => command(), + getCurrentCommit: () => 'currentco_mmit', + isTaggedLatest: isTaggedLatestMock, + })) + .mock('../../releases/utils/release-utils', () => ({ + generateAndroidArtifacts: generateAndroidArtifactsMock, + publishAndroidArtifactsToMaven: publishAndroidArtifactsToMavenMock, + })) + .mock('../../releases/set-version', () => setVersionMock) + .mock('../../releases/set-rn-version', () => ({ + setReactNativeVersion: setReactNativeVersionMock, + })) + .mock('../../releases/remove-new-arch-flags', () => ({ + removeNewArchFlags, + })) + .mock('../../npm-utils', () => ({ + ...jest.requireActual('../../npm-utils'), + publishPackage: publishPackageMock, + getNpmInfo: getNpmInfoMock, + })); + }); + + afterAll(() => { + jest.clearAllMocks(); }); beforeEach(() => { - consoleError = console.error; - console.error = consoleErrorMock; + consoleLog = console.log; + // $FlowExpectedError[cannot-write] + console.log = consoleLogMock; }); afterEach(() => { process.env = env; - console.error = consoleError; + // $FlowExpectedError[cannot-write] + console.log = consoleLog; jest.resetModules(); jest.resetAllMocks(); }); describe('publish-npm.js', () => { - it('Fails when invalid build type is passed', async () => { - await expect(publishNpm('invalid')).rejects.toThrow( - 'Unsupported build type: invalid', - ); + it('should fail when invalid build type is passed', async () => { + // Call actual function + // $FlowExpectedError[underconstrained-implicit-instantiation] + const npmUtils = jest.requireActual('../../npm-utils'); + getNpmInfoMock.mockImplementation(npmUtils.getNpmInfo); + + await expect(async () => { + // $FlowExpectedError[incompatible-call] + await publishNpm('invalid'); + }).rejects.toThrow('Unsupported build type: invalid'); }); }); - describe('dry-run', () => { + describe("publishNpm('dry-run')", () => { it('should set version and not publish', async () => { + const version = '1000.0.0-currentco'; + getNpmInfoMock.mockReturnValueOnce({ + version, + tag: null, + }); + await publishNpm('dry-run'); expect(removeNewArchFlags).not.toHaveBeenCalled(); - expect(exitMock).toHaveBeenCalledWith(0); - expect(isTaggedLatestMock.mock.calls).toHaveLength(0); - expect(echoMock).toHaveBeenCalledWith( - 'Skipping `npm publish` because --dry-run is set.', - ); + + expect(setVersionMock).not.toBeCalled(); expect(setReactNativeVersionMock).toBeCalledWith( - '1000.0.0-currentco', + version, null, 'dry-run', ); - expect(setVersionMock).not.toBeCalled(); + + expect(generateAndroidArtifactsMock).toBeCalledWith(version); + expect(consoleLogMock).toHaveBeenCalledWith( + 'Skipping `npm publish` because --dry-run is set.', + ); + + // Expect termination + expect(publishAndroidArtifactsToMavenMock).not.toHaveBeenCalled(); + expect(publishPackageMock).not.toHaveBeenCalled(); }); }); - describe('nightly', () => { - let consoleLog; + describe("publishNpm('nightly')", () => { beforeAll(() => { - consoleLog = console.log; - console.log = jest.fn(); - jest.mock('../../npm-utils', () => ({ - ...jest.requireActual('../../npm-utils'), - publishPackage: publishPackageMock, - getNpmInfo: getNpmInfoMock, + jest.mock('../../releases/utils/monorepo', () => ({ + ...jest.requireActual('../../releases/utils/monorepo'), + getPackages: getPackagesMock, })); }); afterAll(() => { - console.log = consoleLog; - jest.unmock('../../npm-utils'); - }); - - beforeEach(() => { - jest.resetAllMocks(); + jest.unmock('../../releases/utils/monorepo'); }); it('should publish', async () => { - publishPackageMock.mockImplementation(() => ({ - code: 0, + const expectedVersion = '0.82.0-nightly-20230420-currentco'; + getPackagesMock.mockImplementation(() => ({ + 'monorepo/pkg-a': { + name: 'monorepo/pkg-a', + path: 'path/to/monorepo/pkg-a', + packageJson: {version: expectedVersion}, + }, + 'monorepo/pkg-b': { + name: 'monorepo/pkg-b', + path: 'path/to/monorepo/pkg-b', + packageJson: {version: expectedVersion}, + }, })); + getNpmInfoMock.mockImplementation(() => ({ version: expectedVersion, tag: 'nightly', })); - const expectedVersion = '0.82.0-nightly-20230420-currentco'; + publishPackageMock.mockImplementation(() => ({ + code: 0, + })); await publishNpm('nightly'); expect(removeNewArchFlags).not.toHaveBeenCalled(); expect(setVersionMock).toBeCalledWith(expectedVersion); + expect(generateAndroidArtifactsMock).toHaveBeenCalled(); + expect(publishPackageMock.mock.calls).toEqual([ + ['path/to/monorepo/pkg-a', {otp: undefined, tags: ['nightly']}], + ['path/to/monorepo/pkg-b', {otp: undefined, tags: ['nightly']}], + [ + path.join(REPO_ROOT, 'packages', 'react-native'), + {otp: undefined, tags: ['nightly']}, + ], + ]); expect(publishAndroidArtifactsToMavenMock).toHaveBeenCalledWith( expectedVersion, 'nightly', ); - publishPackageMock.mock.calls.forEach(params => { - expect(params[1]).toEqual({ - tags: ['nightly'], - otp: undefined, - }); - }); - expect(publishPackageMock).toHaveBeenCalledWith( - path.join(REPO_ROOT, 'packages/react-native'), - {otp: undefined, tags: ['nightly']}, - ); - expect(echoMock).toHaveBeenCalledWith( - `Published to npm ${expectedVersion}`, - ); - expect(exitMock).toHaveBeenCalledWith(0); + expect(consoleLogMock.mock.calls).toEqual([ + ['Publishing monorepo/pkg-a...'], + [`Published monorepo/pkg-a@${expectedVersion} to npm`], + ['Publishing monorepo/pkg-b...'], + [`Published monorepo/pkg-b@${expectedVersion} to npm`], + [`Published react-native@${expectedVersion} to npm`], + ]); }); - it('should fail to set version', async () => { + it('should not publish when setting version fails', async () => { const expectedVersion = '0.82.0-nightly-20230420-currentco'; - publishPackageMock.mockImplementation(() => ({ - code: 0, - })); getNpmInfoMock.mockImplementation(() => ({ version: expectedVersion, tag: 'nightly', })); + publishPackageMock.mockImplementation(() => ({ + code: 0, + })); setVersionMock.mockImplementation(() => { - throw new Error('something went wrong'); + throw new Error('something went wrong with setVersion'); }); - await publishNpm('nightly'); + await expect(async () => { + await publishNpm('nightly'); + }).rejects.toThrow('something went wrong with setVersion'); expect(removeNewArchFlags).not.toHaveBeenCalled(); + expect(publishPackageMock).not.toBeCalled(); + expect(generateAndroidArtifactsMock).not.toHaveBeenCalled(); expect(publishAndroidArtifactsToMavenMock).not.toBeCalled(); - expect(consoleErrorMock).toHaveBeenCalledWith( - `Failed to set version number to ${expectedVersion}`, - ); - expect(exitMock).toHaveBeenCalledWith(1); }); + it('should fail to publish react-native if some monorepo packages fail', async () => { - publishPackageMock.mockImplementation(packagePath => ({ - code: 1, + const expectedVersion = '0.82.0-nightly-20230420-currentco'; + + getPackagesMock.mockImplementation(() => ({ + 'monorepo/pkg-a': { + name: 'monorepo/pkg-a', + path: 'path/to/monorepo/pkg-a', + packageJson: {version: expectedVersion}, + }, + 'monorepo/pkg-b': { + name: 'monorepo/pkg-b', + path: 'path/to/monorepo/pkg-b', + packageJson: {version: expectedVersion}, + }, + 'monorepo/pkg-c': { + name: 'monorepo/pkg-c', + path: 'path/to/monorepo/pkg-c', + packageJson: {version: expectedVersion}, + }, })); + publishPackageMock.mockImplementation(packagePath => { + if (packagePath === 'path/to/monorepo/pkg-b') { + return {code: 1}; + } + return {code: 0}; + }); getNpmInfoMock.mockImplementation(() => ({ version: expectedVersion, tag: 'nightly', })); - const expectedVersion = '0.82.0-nightly-20230420-currentco'; - - await publishNpm('nightly'); + // We expect publish to fail on monorepo/pkg-b, and not publish anything-beyond + await expect(async () => { + await publishNpm('nightly'); + }).rejects.toThrow( + `Failed to publish monorepo/pkg-b@${expectedVersion} to npm. Stopping all nightly publishes`, + ); expect(removeNewArchFlags).not.toHaveBeenCalled(); expect(setVersionMock).toBeCalledWith(expectedVersion); - expect(publishAndroidArtifactsToMavenMock).toHaveBeenCalledWith( - expectedVersion, - 'nightly', - ); - expect(exitMock).toHaveBeenCalledWith(1); - publishPackageMock.mock.calls.forEach(params => { - expect(params[1]).toEqual({ - tags: ['nightly'], - otp: undefined, - }); - }); - expect(echoMock).toHaveBeenCalledWith('Failed to publish package to npm'); - }); - }); - describe('release', () => { - it('should fail with invalid release version', async () => { - process.env.CIRCLE_TAG = '1.0.1'; - await expect(publishNpm('release')).rejects.toThrow( - 'Version 1.0.1 is not valid for Release', - ); - expect(publishAndroidArtifactsToMavenMock).not.toBeCalled(); + expect(generateAndroidArtifactsMock).not.toHaveBeenCalled(); + + // Note that we don't call `publishPackage` for react-native, or monorepo/pkg-c + expect(publishPackageMock.mock.calls).toEqual([ + ['path/to/monorepo/pkg-a', {otp: undefined, tags: ['nightly']}], + ['path/to/monorepo/pkg-b', {otp: undefined, tags: ['nightly']}], + ]); + + expect(consoleLogMock.mock.calls).toEqual([ + ['Publishing monorepo/pkg-a...'], + ['Published monorepo/pkg-a@0.82.0-nightly-20230420-currentco to npm'], + ['Publishing monorepo/pkg-b...'], + ]); + expect(publishAndroidArtifactsToMavenMock).not.toHaveBeenCalled(); }); + }); + describe("publishNpm('release')", () => { it('should publish non-latest', async () => { - execMock.mockReturnValueOnce({code: 0}); - isTaggedLatestMock.mockReturnValueOnce(false); - process.env.CIRCLE_TAG = '0.81.1'; + const expectedVersion = '0.81.1'; + getNpmInfoMock.mockImplementation(() => ({ + version: expectedVersion, + tag: '0.81-stable', + })); + publishPackageMock.mockImplementation(() => ({ + code: 0, + })); + process.env.NPM_CONFIG_OTP = 'otp'; await publishNpm('release'); expect(removeNewArchFlags).not.toHaveBeenCalled(); - const expectedVersion = '0.81.1'; + expect(setReactNativeVersionMock).not.toHaveBeenCalled(); + expect(setVersionMock).not.toBeCalled(); + expect(generateAndroidArtifactsMock).toHaveBeenCalled(); expect(publishAndroidArtifactsToMavenMock).toHaveBeenCalledWith( expectedVersion, 'release', ); - expect(execMock).toHaveBeenCalledWith( - `npm publish --tag 0.81-stable --otp otp`, - {cwd: path.join(REPO_ROOT, 'packages/react-native')}, - ); - expect(echoMock).toHaveBeenCalledWith( - `Published to npm ${expectedVersion}`, - ); - expect(exitMock).toHaveBeenCalledWith(0); - expect(execMock.mock.calls).toHaveLength(1); + + expect(publishPackageMock.mock.calls).toEqual([ + [ + path.join(REPO_ROOT, 'packages', 'react-native'), + {otp: process.env.NPM_CONFIG_OTP, tags: ['0.81-stable']}, + ], + ]); + + expect(consoleLogMock.mock.calls).toEqual([ + [`Published react-native@${expectedVersion} to npm`], + ]); }); it('should publish latest stable', async () => { - execMock.mockReturnValueOnce({code: 0}); - isTaggedLatestMock.mockReturnValueOnce(true); - process.env.CIRCLE_TAG = '0.81.1'; + const expectedVersion = '0.81.1'; + getNpmInfoMock.mockImplementation(() => ({ + version: expectedVersion, + tag: 'latest', + })); + publishPackageMock.mockImplementation(() => ({ + code: 0, + })); + process.env.NPM_CONFIG_OTP = 'otp'; await publishNpm('release'); expect(removeNewArchFlags).not.toHaveBeenCalled(); - const expectedVersion = '0.81.1'; + expect(setVersionMock).not.toBeCalled(); + expect(setReactNativeVersionMock).not.toBeCalled(); + expect(generateAndroidArtifactsMock).toHaveBeenCalled(); expect(publishAndroidArtifactsToMavenMock).toHaveBeenCalledWith( expectedVersion, 'release', ); - expect(execMock).toHaveBeenCalledWith( - `npm publish --tag latest --otp ${process.env.NPM_CONFIG_OTP}`, - {cwd: path.join(REPO_ROOT, 'packages/react-native')}, - ); - expect(echoMock).toHaveBeenCalledWith( - `Published to npm ${expectedVersion}`, - ); - expect(exitMock).toHaveBeenCalledWith(0); - expect(execMock.mock.calls).toHaveLength(1); + + expect(publishPackageMock.mock.calls).toEqual([ + [ + path.join(REPO_ROOT, 'packages', 'react-native'), + {otp: process.env.NPM_CONFIG_OTP, tags: ['latest']}, + ], + ]); + + expect(consoleLogMock.mock.calls).toEqual([ + [`Published react-native@${expectedVersion} to npm`], + ]); }); it('should fail to publish latest stable', async () => { + const expectedVersion = '0.81.1'; + getNpmInfoMock.mockImplementation(() => ({ + version: expectedVersion, + tag: 'latest', + })); + publishPackageMock.mockImplementation(() => ({ + code: 1, + })); + execMock.mockReturnValueOnce({code: 1}); isTaggedLatestMock.mockReturnValueOnce(true); - process.env.CIRCLE_TAG = '0.81.1'; + process.env.NPM_CONFIG_OTP = 'otp'; - await publishNpm('release'); + await expect(async () => { + await publishNpm('release'); + }).rejects.toThrow( + `Failed to publish react-native@${expectedVersion} to npm.`, + ); expect(removeNewArchFlags).not.toHaveBeenCalled(); - const expectedVersion = '0.81.1'; + expect(setVersionMock).not.toBeCalled(); + expect(setReactNativeVersionMock).not.toHaveBeenCalled(); + expect(generateAndroidArtifactsMock).toHaveBeenCalled(); expect(publishAndroidArtifactsToMavenMock).toHaveBeenCalledWith( expectedVersion, 'release', ); - expect(execMock).toHaveBeenCalledWith( - `npm publish --tag latest --otp ${process.env.NPM_CONFIG_OTP}`, - {cwd: path.join(REPO_ROOT, 'packages/react-native')}, - ); - expect(echoMock).toHaveBeenCalledWith(`Failed to publish package to npm`); - expect(exitMock).toHaveBeenCalledWith(1); - expect(execMock.mock.calls).toHaveLength(1); + + expect(publishPackageMock.mock.calls).toEqual([ + [ + path.join(REPO_ROOT, 'packages', 'react-native'), + {otp: process.env.NPM_CONFIG_OTP, tags: ['latest']}, + ], + ]); + expect(consoleLogMock).not.toHaveBeenCalled(); }); it('should publish next', async () => { - execMock.mockReturnValueOnce({code: 0}); - isTaggedLatestMock.mockReturnValueOnce(true); - process.env.CIRCLE_TAG = '0.81.0-rc.4'; + const expectedVersion = '0.81.0-rc.4'; + getNpmInfoMock.mockImplementation(() => ({ + version: expectedVersion, + tag: 'next', + })); + publishPackageMock.mockImplementation(() => ({ + code: 0, + })); + process.env.NPM_CONFIG_OTP = 'otp'; await publishNpm('release'); expect(removeNewArchFlags).not.toHaveBeenCalled(); - const expectedVersion = '0.81.0-rc.4'; + expect(setReactNativeVersionMock).not.toHaveBeenCalled(); + expect(setVersionMock).not.toBeCalled(); + expect(generateAndroidArtifactsMock).toHaveBeenCalled(); expect(publishAndroidArtifactsToMavenMock).toHaveBeenCalledWith( expectedVersion, 'release', ); - expect(execMock).toHaveBeenCalledWith( - `npm publish --tag next --otp ${process.env.NPM_CONFIG_OTP}`, - {cwd: path.join(REPO_ROOT, 'packages/react-native')}, - ); - expect(echoMock).toHaveBeenCalledWith( - `Published to npm ${expectedVersion}`, - ); - expect(exitMock).toHaveBeenCalledWith(0); - expect(execMock.mock.calls).toHaveLength(1); + + expect(publishPackageMock.mock.calls).toEqual([ + [ + path.join(REPO_ROOT, 'packages', 'react-native'), + {otp: process.env.NPM_CONFIG_OTP, tags: ['next']}, + ], + ]); + expect(consoleLogMock.mock.calls).toEqual([ + [`Published react-native@${expectedVersion} to npm`], + ]); }); }); }); diff --git a/scripts/releases-ci/publish-npm.js b/scripts/releases-ci/publish-npm.js index 53511a2b9c05ca..a50d4211148066 100755 --- a/scripts/releases-ci/publish-npm.js +++ b/scripts/releases-ci/publish-npm.js @@ -25,7 +25,6 @@ const { publishAndroidArtifactsToMaven, } = require('../releases/utils/release-utils'); const path = require('path'); -const {echo, exit} = require('shelljs'); const yargs = require('yargs'); const REPO_ROOT = path.resolve(__dirname, '../..'); @@ -85,11 +84,11 @@ async function publishMonorepoPackages(tag /*: ?string */) { const spec = `${packageInfo.name}@${packageInfo.packageJson.version}`; if (result.code) { - echo(`Failed to publish ${spec} to npm. Stopping all nightly publishes`); - exit(1); - } else { - echo(`Published ${spec} to npm`); + throw new Error( + `Failed to publish ${spec} to npm. Stopping all nightly publishes`, + ); } + console.log(`Published ${spec} to npm`); } } @@ -102,26 +101,20 @@ async function publishNpm(buildType /*: BuildType */) /*: Promise */ { // For stable releases, CircleCI job `prepare_package_for_release` handles this if (['dry-run', 'nightly', 'prealpha'].includes(buildType)) { - try { - if (buildType === 'nightly') { - // Set same version for all monorepo packages - await setVersion(version); - await publishMonorepoPackages(tag); - } else { - await setReactNativeVersion(version, null, buildType); - } - } catch (e) { - console.error(`Failed to set version number to ${version}`); - console.error(e); - return exit(1); + if (buildType === 'nightly') { + // Set same version for all monorepo packages + await setVersion(version); + await publishMonorepoPackages(tag); + } else { + await setReactNativeVersion(version, null, buildType); } } generateAndroidArtifacts(version); if (buildType === 'dry-run') { - echo('Skipping `npm publish` because --dry-run is set.'); - return exit(0); + console.log('Skipping `npm publish` because --dry-run is set.'); + return; } // We first publish on Maven Central all the necessary artifacts. @@ -136,12 +129,9 @@ async function publishNpm(buildType /*: BuildType */) /*: Promise */ { }); if (result.code) { - echo('Failed to publish package to npm'); - return exit(1); - } else { - echo(`Published to npm ${version}`); - return exit(0); + throw new Error(`Failed to publish react-native@${version} to npm.`); } + console.log(`Published react-native@${version} to npm`); } module.exports = { diff --git a/scripts/releases/utils/__tests__/version-utils-test.js b/scripts/releases/utils/__tests__/version-utils-test.js index 1dba46a6235361..044619b31e1580 100644 --- a/scripts/releases/utils/__tests__/version-utils-test.js +++ b/scripts/releases/utils/__tests__/version-utils-test.js @@ -51,6 +51,7 @@ describe('version-utils', () => { `"Unsupported build type: invalid_build_type"`, ); }); + it('should throw error if invalid match with release', () => { function testInvalidVersion() { parseVersion('', 'release'); @@ -59,6 +60,7 @@ describe('version-utils', () => { `"You must pass a correctly formatted version; couldn't parse "`, ); }); + it('should throw error if invalid match with dry-run', () => { function testInvalidVersion() { parseVersion('', 'dry-run'); @@ -353,6 +355,12 @@ describe('version-utils', () => { '"Version 1.0.0-2023100416 is not valid for prealphas"', ); }); + + it('should reject stable releases with major > 0', () => { + expect(() => parseVersion('1.0.1', 'release')).toThrow( + 'Version 1.0.1 is not valid for Release', + ); + }); }); describe('isNightly', () => {