diff --git a/.github/actions/javascript/markPullRequestsAsDeployed/index.js b/.github/actions/javascript/markPullRequestsAsDeployed/index.js index 2cd5e7203fff..f380b153cd90 100644 --- a/.github/actions/javascript/markPullRequestsAsDeployed/index.js +++ b/.github/actions/javascript/markPullRequestsAsDeployed/index.js @@ -12707,7 +12707,7 @@ function wrappy (fn, cb) { /***/ }), /***/ 2483: -/***/ (function(module, exports, __nccwpck_require__) { +/***/ (function(__unused_webpack_module, exports, __nccwpck_require__) { "use strict"; @@ -12748,7 +12748,7 @@ var __importDefault = (this && this.__importDefault) || function (mod) { return (mod && mod.__esModule) ? mod : { "default": mod }; }; Object.defineProperty(exports, "__esModule", ({ value: true })); -/* eslint-disable @typescript-eslint/naming-convention, import/no-import-module-exports */ +/* eslint-disable @typescript-eslint/naming-convention */ const ActionUtils = __importStar(__nccwpck_require__(6981)); const CONST_1 = __importDefault(__nccwpck_require__(9873)); const GithubUtils_1 = __importDefault(__nccwpck_require__(9296)); @@ -12937,7 +12937,7 @@ async function run() { if (require.main === require.cache[eval('__filename')]) { run(); } -module.exports = run; +exports["default"] = run; /***/ }), diff --git a/.github/actions/javascript/markPullRequestsAsDeployed/markPullRequestsAsDeployed.ts b/.github/actions/javascript/markPullRequestsAsDeployed/markPullRequestsAsDeployed.ts index e0d07d1341ab..cf5b0baedd45 100644 --- a/.github/actions/javascript/markPullRequestsAsDeployed/markPullRequestsAsDeployed.ts +++ b/.github/actions/javascript/markPullRequestsAsDeployed/markPullRequestsAsDeployed.ts @@ -1,4 +1,4 @@ -/* eslint-disable @typescript-eslint/naming-convention, import/no-import-module-exports */ +/* eslint-disable @typescript-eslint/naming-convention */ import * as ActionUtils from '@github/libs/ActionUtils'; import CONST from '@github/libs/CONST'; import GithubUtils from '@github/libs/GithubUtils'; @@ -219,4 +219,4 @@ if (require.main === module) { run(); } -module.exports = run; +export default run; diff --git a/.github/workflows/bunTests.yml b/.github/workflows/bunTests.yml index 2df7576e070d..22e4047f025b 100644 --- a/.github/workflows/bunTests.yml +++ b/.github/workflows/bunTests.yml @@ -1,12 +1,19 @@ name: Bun tests on: + workflow_call: pull_request: types: [opened, synchronize] branches-ignore: [staging, production] paths: - 'server/**' - 'src/**' + - 'tests/tooling/**' + - 'tests/utils/**' + - '.github/actions/javascript/**' + - '.github/libs/**' + - '.github/scripts/**' + - 'scripts/**' - 'bunfig.toml' - '.bun-version' - 'package.json' @@ -14,7 +21,7 @@ on: - '.github/workflows/bunTests.yml' concurrency: - group: ${{ github.ref }}-bun-tests + group: ${{ github.ref == 'refs/heads/main' && format('{0}-{1}', github.ref, github.sha) || github.ref }}-bun-tests cancel-in-progress: true jobs: diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index 0627965accd5..3237ac966baa 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -6,7 +6,21 @@ on: types: [opened, synchronize] branches-ignore: [staging, production] paths: - ['**.js', '**.ts', '**.tsx', '**.json', '**.mjs', '**.cjs', 'config/.editorconfig', 'config/eslint/**', 'scripts/lint.sh', 'scripts/lintChanged.sh', '.watchmanconfig', '.imgbotconfig'] + [ + '**.js', + '**.ts', + '**.tsx', + '**.json', + '**.mjs', + '**.cjs', + 'config/.editorconfig', + 'config/eslint/**', + 'scripts/lint.sh', + 'scripts/lintChanged.sh', + '.watchmanconfig', + '.imgbotconfig', + '.github/workflows/lint.yml', + ] concurrency: group: ${{ github.ref == 'refs/heads/main' && format('{0}-{1}', github.ref, github.sha) || github.ref }}-lint @@ -16,10 +30,9 @@ jobs: lint: name: ESLint check if: ${{ github.event.head_commit.author.name != 'OSBotify' || github.event_name == 'push' }} - # A cold-cache run lints the whole repo with type-aware rules, which loads the full TypeScript - # program in every worker (~12GB of heap each, regardless of how files are split between workers). - # 2 workers with a 14GB heap cap need ~30GB of memory, so this requires the 32GB (8vcpu) runner. - runs-on: blacksmith-8vcpu-ubuntu-2404 + # A cold-cache run loads the full TypeScript program into every worker (~12GB each), plus any extra + # tsconfig project a file is mapped to. 2 workers need more than the 32GB an 8vcpu runner has. + runs-on: blacksmith-16vcpu-ubuntu-2404 steps: - name: Checkout uses: useblacksmith/checkout@1c9394c220d293645707b625ba9d79685f093a8f # v1 diff --git a/.github/workflows/preDeploy.yml b/.github/workflows/preDeploy.yml index 15183af70f87..80121fa9cdb3 100644 --- a/.github/workflows/preDeploy.yml +++ b/.github/workflows/preDeploy.yml @@ -21,18 +21,21 @@ jobs: uses: ./.github/workflows/test.yml secrets: inherit + bunTests: + uses: ./.github/workflows/bunTests.yml + confirmPassingBuild: runs-on: blacksmith-4vcpu-ubuntu-2404 - needs: [typecheck, lint, test] + needs: [typecheck, lint, test, bunTests] if: ${{ always() }} steps: - uses: useblacksmith/checkout@1c9394c220d293645707b625ba9d79685f093a8f # v1 - name: Exit failed workflow - if: ${{ needs.typecheck.result == 'failure' || needs.lint.result == 'failure' || needs.test.result == 'failure' }} + if: ${{ needs.typecheck.result == 'failure' || needs.lint.result == 'failure' || needs.test.result == 'failure' || needs.bunTests.result == 'failure' }} run: | - echo "Checks failed, exiting ~ typecheck: ${{ needs.typecheck.result }}, lint: ${{ needs.lint.result }}, test: ${{ needs.test.result }}" + echo "Checks failed, exiting ~ typecheck: ${{ needs.typecheck.result }}, lint: ${{ needs.lint.result }}, test: ${{ needs.test.result }}, bunTests: ${{ needs.bunTests.result }}" exit 1 chooseDeployActions: diff --git a/.github/workflows/typecheck.yml b/.github/workflows/typecheck.yml index 8bf164abf6b5..90cdf1780ecb 100644 --- a/.github/workflows/typecheck.yml +++ b/.github/workflows/typecheck.yml @@ -5,7 +5,7 @@ on: pull_request: types: [opened, synchronize] branches-ignore: [staging, production] - paths: ['**.js', '**.ts', '**.tsx', 'package.json', 'package-lock.json', 'tsconfig.json'] + paths: ['**.js', '**.ts', '**.tsx', 'package.json', 'package-lock.json', '**/tsconfig.json'] concurrency: group: ${{ github.ref == 'refs/heads/main' && format('{0}-{1}', github.ref, github.sha) || github.ref }}-typecheck diff --git a/.oxfmtrc.json b/.oxfmtrc.json index 46e9af39ccf0..edc832fe2b5e 100644 --- a/.oxfmtrc.json +++ b/.oxfmtrc.json @@ -11,8 +11,8 @@ "ignoreCase": true, "customGroups": [ { - "groupName": "jest", - "elementNamePattern": ["@jest/globals", "@testing-library/**"] + "groupName": "test-runner", + "elementNamePattern": ["@jest/globals", "@testing-library/**", "bun:test"] }, {"groupName": "assets", "elementNamePattern": ["@assets/**"]}, {"groupName": "components", "elementNamePattern": ["@components/**"]}, @@ -28,7 +28,7 @@ {"groupName": "src", "elementNamePattern": ["@src/**"]} ], "groups": [ - "jest", + "test-runner", "assets", "components", "github", diff --git a/CLAUDE.md b/CLAUDE.md index cc49e7bef0a4..16c01b162bd1 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -231,7 +231,8 @@ React Compiler auto-memoizes object literals, callbacks, JSX, and derived values ### Testing -- **Unit Tests**: Jest with React Native Testing Library +- **Unit Tests**: Jest with React Native Testing Library. Tests for `.github/` and `scripts/` live in + `tests/tooling/` and run under `bun:test` (`npm run test:bun`) — see `tests/tooling/README.md`. - **Performance Tests**: Reassure framework ## Special Considerations @@ -292,6 +293,9 @@ npm run fmt # Testing npm run test + +# Bun tests: server/ plus the repo's own tooling (.github/ and scripts/) +npm run test:bun ``` ### Platform Builds diff --git a/README.md b/README.md index a46e1b31cc5a..e69813155bb0 100644 --- a/README.md +++ b/README.md @@ -94,6 +94,7 @@ Often times in order to write a unit test, you may need to mock data, a componen to help run our Unit tests. * To run the **Jest unit tests**: `npm run test` +* To run the **Bun tests** — `server/` plus the `.github/` and `scripts/` tooling tests: `npm run test:bun` — see [tests/tooling/README.md](tests/tooling/README.md) * UI tests guidelines can be found [here](tests/ui/README.md) ## Performance tests diff --git a/bunfig.toml b/bunfig.toml index 781d91320191..db27eb44682b 100644 --- a/bunfig.toml +++ b/bunfig.toml @@ -2,7 +2,8 @@ # `import` and `require()`, so scripts don't load the real Flow-typed RN packages at runtime). preload = ["./scripts/stubReactNative.js"] -# Bun test discovery for server-side tooling (Jest owns src/ and tests/). +# Where a bare `bun test` looks. Both suites are actually run through the `test:bun` npm script, which passes +# their roots explicitly along with the --parallel and --preload flags they need (see tests/tooling/README.md). [test] root = "server" pathIgnorePatterns = ["**/fixtures/**", "**/dist/**", "**/.dev/**", "**/__output__/**", "**/__golden__/**"] diff --git a/config/eslint/eslint.config.mjs b/config/eslint/eslint.config.mjs index e9d5af4950e5..69c9bf8d12c7 100644 --- a/config/eslint/eslint.config.mjs +++ b/config/eslint/eslint.config.mjs @@ -699,6 +699,29 @@ const config = defineConfig([ }, }, + { + // CIGitLogic is excluded from the root tsconfig because it needs @types/bun, so type-aware rules have to + // be pointed at the project that does own it. See tests/tooling/README.md. + files: ['tests/tooling/CIGitLogic.test.ts'], + languageOptions: { + parserOptions: { + project: path.resolve(projectRoot, 'tests/tooling/tsconfig.json'), + projectService: false, + }, + }, + }, + + { + files: ['tests/tooling/**/*.ts'], + rules: { + // bun-types declares `expect(...).resolves`/`.rejects` matchers as returning `void` even though Bun's + // own docs recommend (and its runtime requires) awaiting them, so this rule reports every correct use + // of that pattern here. See https://github.com/oven-sh/bun/pull/23425. The cost of turning it off is + // that a *missing* await on `.rejects` also lints clean, so check those by hand in review. + '@typescript-eslint/await-thenable': 'off', + }, + }, + { files: ['server/victory-chart-renderer/**/*.ts', 'server/victory-chart-renderer/**/*.tsx'], languageOptions: { diff --git a/cspell.json b/cspell.json index b03d9b793f0e..84e54cfb5bb2 100644 --- a/cspell.json +++ b/cspell.json @@ -57,6 +57,7 @@ "Bronn", "Buildscript", "Bunq", + "bunfig", "Bushwick", "CARDFROZEN", "CARDUNFROZEN", @@ -250,6 +251,7 @@ "Nonfinancial", "Nonmortgage", "Nonnull", + "nothrow", "Nonstore", "Nonupholstered", "Noto", diff --git a/jest.config.js b/jest.config.js index eed0ea91fec1..7b9a523b566f 100644 --- a/jest.config.js +++ b/jest.config.js @@ -26,7 +26,10 @@ module.exports = { // Prevent Babel from transforming worklets in this file so they are treated as normal functions, otherwise FormatSelectionUtilsTest won't run. '/node_modules/@expensify/react-native-live-markdown/lib/commonjs/parseExpensiMark.js', ], - testPathIgnorePatterns: ['/node_modules'], + // tests/tooling/ covers .github/ and scripts/ and runs under `bun test` instead (see the `test:bun` npm + // script), so those files import `bun:test` rather than Jest's globals. They aren't in testMatch above, and + // this keeps them out even if a future testMatch entry broadens to all of tests/. + testPathIgnorePatterns: ['/node_modules', '/tests/tooling/'], // .worktrees/ and .claude/worktrees/ hold parallel git worktrees a developer may check out locally. // Each one carries its own modules/hybrid-app/package.json, which trips // jest-haste-map's "duplicate package name" assertion. Skip them entirely. diff --git a/knip.json b/knip.json index 14e17865496e..225d716d71b9 100644 --- a/knip.json +++ b/knip.json @@ -10,6 +10,7 @@ "web/proxy.ts", "config/rsbuild/**/*.{js,mjs,cjs,ts}", ".github/scripts/**/*.ts", + "tests/tooling/**/*.ts", ".github/actions/javascript/**/*.ts", ".storybook/**/*.{js,ts,tsx}", "metro.config.js", diff --git a/package-lock.json b/package-lock.json index 4653612c9507..f262001a91ce 100644 --- a/package-lock.json +++ b/package-lock.json @@ -174,6 +174,7 @@ "@octokit/core": "4.0.4", "@octokit/plugin-paginate-rest": "3.1.0", "@octokit/plugin-throttling": "4.1.0", + "@octokit/request-error": "3.0.3", "@octokit/webhooks-types": "^7.5.1", "@react-native-community/cli": "20.1.0", "@react-native-community/cli-platform-android": "20.1.0", @@ -205,7 +206,6 @@ "@types/geojson": "^7946.0.16", "@types/howler": "^2.2.12", "@types/jest": "^29.5.14", - "@types/jest-when": "^3.5.2", "@types/js-yaml": "^4.0.5", "@types/lodash-es": "4.17.12", "@types/mapbox-gl": "^2.7.13", @@ -258,7 +258,6 @@ "jest-environment-jsdom": "^29.7.0", "jest-expo": "57.0.2", "jest-transformer-svg": "^2.0.1", - "jest-when": "^3.5.2", "knip": "^6.14.0", "lefthook": "2.1.9", "link": "^2.1.1", @@ -11986,6 +11985,8 @@ }, "node_modules/@octokit/request-error": { "version": "3.0.3", + "resolved": "https://registry.npmjs.org/@octokit/request-error/-/request-error-3.0.3.tgz", + "integrity": "sha512-crqw3V5Iy2uOU5Np+8M/YexTlT8zxCfI+qu+LxUB7SZpje4Qmx3mub5DfEKSO8Ylyk0aogi6TYdf6kxzh2BguQ==", "dev": true, "license": "MIT", "dependencies": { @@ -18545,14 +18546,6 @@ "pretty-format": "^29.0.0" } }, - "node_modules/@types/jest-when": { - "version": "3.5.2", - "dev": true, - "license": "MIT", - "dependencies": { - "@types/jest": "*" - } - }, "node_modules/@types/js-yaml": { "version": "4.0.5", "dev": true, @@ -31119,14 +31112,6 @@ "node": ">=8" } }, - "node_modules/jest-when": { - "version": "3.5.2", - "dev": true, - "license": "MIT", - "peerDependencies": { - "jest": ">= 25" - } - }, "node_modules/jest-worker": { "version": "27.5.1", "license": "MIT", diff --git a/package.json b/package.json index 150e1e78fbdb..8bb37aced12d 100644 --- a/package.json +++ b/package.json @@ -46,8 +46,8 @@ "test:verbose": "TZ=utc NODE_OPTIONS=\"--experimental-vm-modules --max_old_space_size=4096\" JEST_VERBOSE=true jest", "test:debug": "TZ=utc NODE_OPTIONS='--inspect-brk --experimental-vm-modules' jest --runInBand", "perf-test": "NODE_OPTIONS=--experimental-vm-modules npx reassure", - "typecheck": "NODE_OPTIONS=--max_old_space_size=8192 tsc && NODE_OPTIONS=--max_old_space_size=8192 tsc -p server/tsconfig.json && NODE_OPTIONS=--max_old_space_size=8192 tsc -p server/victory-chart-renderer/tsconfig.json", - "typecheck-tsgo": "tsgo --noEmit --incremental --tsBuildInfoFile tsconfig.tsgo.tsbuildinfo && tsgo --noEmit -p server/tsconfig.json --incremental --tsBuildInfoFile server/tsconfig.tsgo.tsbuildinfo && tsgo --noEmit -p server/victory-chart-renderer/tsconfig.json --incremental --tsBuildInfoFile server/victory-chart-renderer/tsconfig.tsgo.tsbuildinfo", + "typecheck": "NODE_OPTIONS=--max_old_space_size=8192 tsc && NODE_OPTIONS=--max_old_space_size=8192 tsc -p tests/tooling/tsconfig.json && NODE_OPTIONS=--max_old_space_size=8192 tsc -p server/tsconfig.json && NODE_OPTIONS=--max_old_space_size=8192 tsc -p server/victory-chart-renderer/tsconfig.json", + "typecheck-tsgo": "tsgo --noEmit --incremental --tsBuildInfoFile tsconfig.tsgo.tsbuildinfo && tsgo --noEmit -p tests/tooling/tsconfig.json --incremental --tsBuildInfoFile tests/tooling/tsconfig.tsgo.tsbuildinfo && tsgo --noEmit -p server/tsconfig.json --incremental --tsBuildInfoFile server/tsconfig.tsgo.tsbuildinfo && tsgo --noEmit -p server/victory-chart-renderer/tsconfig.json --incremental --tsBuildInfoFile server/victory-chart-renderer/tsconfig.tsgo.tsbuildinfo", "lint": "./scripts/lint.sh", "lint-changed": "./scripts/lintChanged.sh", "lint-watch": "onchange '**/*.{js,jsx,ts,tsx,mjs,cjs}' -- ./scripts/lint.sh {{changed}}", @@ -81,7 +81,7 @@ "compress-svg": "bun scripts/compressSvg.ts --dir assets/images && bun scripts/compressSvg.ts --dir docs/assets/images", "server:vcr:dev": "npm run dev -w @expensify/victory-chart-renderer --", "server:vcr:test": "npm run test -w @expensify/victory-chart-renderer --", - "test:bun": "bun test --concurrent --max-concurrency 7", + "test:bun": "TZ=utc bun test --parallel --preload ./scripts/stubReactNative.js --preload ./tests/tooling/setup.ts ./server ./tests/tooling", "server:vcr:build:linux": "npm run build:linux -w @expensify/victory-chart-renderer --", "server:vcr:build:linux-arm": "npm run build:linux-arm -w @expensify/victory-chart-renderer --", "server:vcr:build:macos": "npm run build:macos -w @expensify/victory-chart-renderer --" @@ -248,6 +248,7 @@ "@octokit/core": "4.0.4", "@octokit/plugin-paginate-rest": "3.1.0", "@octokit/plugin-throttling": "4.1.0", + "@octokit/request-error": "3.0.3", "@octokit/webhooks-types": "^7.5.1", "@react-native-community/cli": "20.1.0", "@react-native-community/cli-platform-android": "20.1.0", @@ -279,7 +280,6 @@ "@types/geojson": "^7946.0.16", "@types/howler": "^2.2.12", "@types/jest": "^29.5.14", - "@types/jest-when": "^3.5.2", "@types/js-yaml": "^4.0.5", "@types/lodash-es": "4.17.12", "@types/mapbox-gl": "^2.7.13", @@ -332,7 +332,6 @@ "jest-environment-jsdom": "^29.7.0", "jest-expo": "57.0.2", "jest-transformer-svg": "^2.0.1", - "jest-when": "^3.5.2", "knip": "^6.14.0", "lefthook": "2.1.9", "link": "^2.1.1", diff --git a/server/victory-chart-renderer/tests/log.test.ts b/server/victory-chart-renderer/tests/log.test.ts index 73615eca0ec4..ab175b17f962 100644 --- a/server/victory-chart-renderer/tests/log.test.ts +++ b/server/victory-chart-renderer/tests/log.test.ts @@ -1,6 +1,7 @@ -import Log from '@server/libs/log'; import {afterEach, beforeEach, describe, expect, test} from 'bun:test'; +import Log from '@server/libs/log'; + import vcrLog from '../src/log'; // VCR_LOG_DESTINATION=stderr forces the fallback path so these tests are deterministic regardless diff --git a/server/victory-chart-renderer/tests/render.test.ts b/server/victory-chart-renderer/tests/render.test.ts index 73b52ce099da..9f7ee7782022 100644 --- a/server/victory-chart-renderer/tests/render.test.ts +++ b/server/victory-chart-renderer/tests/render.test.ts @@ -1,4 +1,5 @@ import {afterAll, beforeAll, describe, expect, test} from 'bun:test'; + import {spawnSync} from 'node:child_process'; import {chmodSync, copyFileSync, existsSync, mkdtempSync, readFileSync, rmSync} from 'node:fs'; import {tmpdir} from 'node:os'; diff --git a/server/victory-chart-renderer/tests/resolveCanvasSize.test.ts b/server/victory-chart-renderer/tests/resolveCanvasSize.test.ts index c0972c4244c6..56cd4c86569b 100644 --- a/server/victory-chart-renderer/tests/resolveCanvasSize.test.ts +++ b/server/victory-chart-renderer/tests/resolveCanvasSize.test.ts @@ -1,6 +1,7 @@ +import {describe, expect, test} from 'bun:test'; + import type {TNode} from 'react-native-render-html'; -import {describe, expect, test} from 'bun:test'; import createMock from 'tests/utils/createMock'; import resolveCanvasSize from '../src/resolveCanvasSize'; diff --git a/server/victory-chart-renderer/tests/stubExports.test.ts b/server/victory-chart-renderer/tests/stubExports.test.ts index f137f6d6fcf9..a3c15b6302aa 100644 --- a/server/victory-chart-renderer/tests/stubExports.test.ts +++ b/server/victory-chart-renderer/tests/stubExports.test.ts @@ -1,5 +1,6 @@ -import * as telemetryActiveSpansStub from '@server/stubs/telemetry-activeSpans'; import {describe, expect, test} from 'bun:test'; + +import * as telemetryActiveSpansStub from '@server/stubs/telemetry-activeSpans'; import {readFileSync} from 'node:fs'; import {join} from 'node:path'; diff --git a/server/victory-chart-renderer/tests/testUtils.ts b/server/victory-chart-renderer/tests/testUtils.ts index 849568497faa..6a662b12341c 100644 --- a/server/victory-chart-renderer/tests/testUtils.ts +++ b/server/victory-chart-renderer/tests/testUtils.ts @@ -1,4 +1,5 @@ import {expect} from 'bun:test'; + import {readdirSync, readFileSync} from 'node:fs'; import {arch, platform} from 'node:os'; import {join} from 'node:path'; diff --git a/tests/README.md b/tests/README.md index 657650d809d0..d26b7b17e683 100644 --- a/tests/README.md +++ b/tests/README.md @@ -2,6 +2,10 @@ [Jest](https://jestjs.io/) is a testing framework we use to ensure our most mission critical libraries are as stable as possible. Here are a few things to consider with regards to our app's architecture when testing in Jest. +> Jest covers everything except `tests/tooling/`, which tests the repo's own build and deploy tooling and runs under +> `bun:test` instead. If your test's import graph reaches `@actions/*` or `@octokit/*`, it belongs there — see +> [tests/tooling/README.md](tooling/README.md). + ## Asynchronous Testing - Much of the logic in the app is asynchronous in nature. [`react-native-onyx`](https://github.com/expensify/react-native-onyx) writes data async before updating subscribers. diff --git a/tests/unit/CIGitLogicTest.ts b/tests/tooling/CIGitLogic.test.ts similarity index 60% rename from tests/unit/CIGitLogicTest.ts rename to tests/tooling/CIGitLogic.test.ts index f11709d3e19b..ad7e93c08d0a 100644 --- a/tests/unit/CIGitLogicTest.ts +++ b/tests/tooling/CIGitLogic.test.ts @@ -1,3 +1,5 @@ +import {afterAll, beforeAll, describe, expect, jest, setDefaultTimeout, test} from 'bun:test'; + import getPreviousVersion from '@github/actions/javascript/getPreviousVersion/getPreviousVersion'; import CONST from '@github/libs/CONST'; import GithubUtils from '@github/libs/GithubUtils'; @@ -5,12 +7,8 @@ import GitUtils from '@github/libs/GitUtils'; import * as VersionUpdater from '@github/libs/versionUpdater'; import type {SemverLevel} from '@github/libs/versionUpdater'; -/** - * @jest-environment node - * @jest-config bail=true - */ import * as core from '@actions/core'; -import {execSync} from 'child_process'; +import {$ as bun$} from 'bun'; import fs from 'fs'; import os from 'os'; import path from 'path'; @@ -18,40 +16,42 @@ import path from 'path'; import * as Log from '../../scripts/utils/Logger'; import createMock from '../utils/createMock'; -const DUMMY_DIR = path.resolve(os.homedir(), 'DumDumRepo'); -const GIT_REMOTE = path.resolve(os.homedir(), 'dummyGitRemotes/DumDumRepo'); +// Every run gets its own throw-away sandbox, so nothing on the machine is shared: this suite can run +// alongside other test files in sibling worker processes, a second copy of itself, or a developer's own +// checkout, without any of them fighting over the same directory. +// os.tmpdir() is resolved because on macOS it is a symlink (/var -> /private/var) and git reports the +// real path, which would make path comparisons against process.cwd() disagree. +const SANDBOX_DIR = fs.mkdtempSync(path.join(fs.realpathSync(os.tmpdir()), 'ci-git-logic-')); +const GIT_REMOTE = path.join(SANDBOX_DIR, 'remote'); +const DUMMY_DIR = path.join(SANDBOX_DIR, 'checkout'); // Used to mock the Octokit GithubAPI -const mockGetInput = jest.fn(); +const mockGetInput = jest.fn<(name: string) => string | undefined>(); type CompareCommitsCommit = NonNullable>['data']['commits']>[number]; -const isVerbose = process.env.JEST_VERBOSE === 'true'; +// Bun's shell, wrapped to log each command and to keep the subprocess output quiet. +const $ = new Proxy(bun$, { + apply(target, thisArg, args: [TemplateStringsArray, ...string[]]) { + Log.info(String.raw({raw: args[0]}, ...args.slice(1))); + return Reflect.apply(target, thisArg, args).quiet(); + }, +}); -function exec(command: string) { - try { - Log.info(command); - execSync(command, {stdio: isVerbose ? 'inherit' : 'pipe'}); - } catch (error) { - const stderr = typeof error === 'object' && error !== null && 'stderr' in error ? error.stderr : undefined; - if ((typeof stderr === 'string' || Buffer.isBuffer(stderr)) && stderr) { - Log.error(stderr.toString()); - } else { - Log.error('Error:', error); - } - throw new Error(String(error)); - } +/** Whether a ref resolves in the repo at `process.cwd()`. `nothrow` because a missing ref is an expected answer here, not a failure. */ +async function refExists(ref: string) { + return (await $`git rev-parse --verify ${ref}`.nothrow()).exitCode === 0; } -function setupGitAsHuman() { +async function setupGitAsHuman() { Log.info('Switching to human git user'); - exec('git config --local user.name test'); - exec('git config --local user.email test@test.com'); + await $`git config --local user.name test`; + await $`git config --local user.email test@test.com`; } -function setupGitAsOSBotify() { +async function setupGitAsOSBotify() { Log.info('Switching to OSBotify git user'); - exec(`git config --local user.name ${CONST.OS_BOTIFY}`); - exec('git config --local user.email infra+osbotify@expensify.com'); + await $`git config --local user.name ${CONST.OS_BOTIFY}`; + await $`git config --local user.email infra+osbotify@expensify.com`; } function getVersion(): string { @@ -72,8 +72,11 @@ function initGithubAPIMocking() { return mockGetInput(name) ?? ''; }); - // Mock various compareCommits responses with single mocked function - jest.spyOn(GithubUtils.octokit.repos, 'compareCommits').mockImplementation((params) => { + // Mock various compareCommits responses with a single mocked function. Assigned directly rather than via + // jest.spyOn/spyOn: Octokit's REST endpoint methods are lazily memoized (each is replaced with a plain value + // the first time it's accessed), and under bun:test, spyOn silently fails to override that already-memoized + // property on this particular object shape, so the mock is never installed. A direct assignment works fine. + const mockCompareCommits = jest.fn().mockImplementation((params: Parameters[0]) => { const base = params?.base; const head = params?.head; const tagPairKey = `${base}...${head}`; @@ -183,330 +186,333 @@ function initGithubAPIMocking() { }), ); }); + // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion -- the stub omits octokit's `defaults`/`endpoint` statics, which this action never touches + GithubUtils.octokit.repos.compareCommits = mockCompareCommits as unknown as typeof GithubUtils.octokit.repos.compareCommits; } -function initGitServer() { +async function initGitServer() { Log.info('Initializing git server...'); - if (fs.existsSync(GIT_REMOTE)) { - Log.info(`${GIT_REMOTE} exists, removing it now...`); - fs.rmSync(GIT_REMOTE, {recursive: true}); - } fs.mkdirSync(GIT_REMOTE, {recursive: true}); process.chdir(GIT_REMOTE); - exec('git init -b main'); - setupGitAsHuman(); - exec('npm init -y'); - exec('npm version --no-git-tag-version 1.0.0-0'); + await $`git init -b main`; + await setupGitAsHuman(); + await $`npm init -y`; + await $`npm version --no-git-tag-version 1.0.0-0`; fs.appendFileSync('.gitignore', 'node_modules/\n'); - exec('git add -A'); - exec('git commit -m "Initial commit"'); - exec('git switch -c staging'); - exec('git switch -c production'); + await $`git add -A`; + await $`git commit -m "Initial commit"`; + await $`git switch -c staging`; + await $`git switch -c production`; // Tag the production branch with 1.0.0.0 - exec(`git tag ${getVersion()}`); + await $`git tag ${getVersion()}`; // Bump version to 2.0.0.0 - bumpVersion(VersionUpdater.SEMANTIC_VERSION_LEVELS.MAJOR, true); - exec('git branch -D staging production'); - exec('git switch -c staging'); - exec('git switch -c production'); - exec(`git tag ${getVersion()}`); - exec(`git switch staging`); - exec('git config --local receive.denyCurrentBranch ignore'); + await bumpVersion(VersionUpdater.SEMANTIC_VERSION_LEVELS.MAJOR, true); + await $`git branch -D staging production`; + await $`git switch -c staging`; + await $`git switch -c production`; + await $`git tag ${getVersion()}`; + await $`git switch staging`; + await $`git config --local receive.denyCurrentBranch ignore`; Log.success(`Initialized git server in ${GIT_REMOTE}`); } -function checkoutRepo() { +async function checkoutRepo() { if (fs.existsSync(DUMMY_DIR)) { Log.warn(`Found existing directory at ${DUMMY_DIR}, deleting it to simulate a fresh checkout...`); fs.rmSync(DUMMY_DIR, {recursive: true}); } fs.mkdirSync(DUMMY_DIR); process.chdir(DUMMY_DIR); - exec('git init'); - exec(`git remote add origin ${GIT_REMOTE}`); - exec('git fetch --no-tags --prune --progress --no-recurse-submodules --depth=1 origin +refs/heads/main:refs/remotes/origin/main'); - exec('git checkout --progress --force -B main refs/remotes/origin/main'); + await $`git init`; + await $`git remote add origin ${GIT_REMOTE}`; + await $`git fetch --no-tags --prune --progress --no-recurse-submodules --depth=1 origin +refs/heads/main:refs/remotes/origin/main`; + await $`git checkout --progress --force -B main refs/remotes/origin/main`; Log.success('Checked out repo at $DUMMY_DIR!'); } -function bumpVersion(level: SemverLevel, isRemote = false) { +async function bumpVersion(level: SemverLevel, isRemote = false) { Log.info('Bumping version...'); - setupGitAsOSBotify(); - exec('git switch main'); + await setupGitAsOSBotify(); + await $`git switch main`; const nextVersion = VersionUpdater.incrementVersion(getVersion(), level); - exec(`npm --no-git-tag-version version ${nextVersion}`); - exec('git add package.json'); - exec(`git commit -m "Update version to ${nextVersion}"`); + await $`npm --no-git-tag-version version ${nextVersion}`; + await $`git add package.json`; + await $`git commit -m "Update version to ${nextVersion}"`; if (!isRemote) { - exec('git push origin main'); + await $`git push origin main`; } Log.success(`Version bumped to ${nextVersion} on main`); } -function updateStagingFromMain() { +async function updateStagingFromMain() { Log.info('Recreating staging from main...'); - exec('git switch main'); - try { - execSync('git rev-parse --verify staging', {stdio: 'ignore'}); - exec('git branch -D staging'); - } catch (e) {} - exec('git switch -c staging'); - exec('git push --force origin staging'); + await $`git switch main`; + if (await refExists('staging')) { + await $`git branch -D staging`; + } + await $`git switch -c staging`; + await $`git push --force origin staging`; Log.success('Recreated staging from main!'); } -function updateProductionFromStaging() { +async function updateProductionFromStaging() { Log.info('Recreating production from staging...'); - try { - execSync('git rev-parse --verify staging', {stdio: 'ignore'}); - } catch (e) { - exec('git fetch origin staging --depth=1'); + if (!(await refExists('staging'))) { + await $`git fetch origin staging --depth=1`; } - exec('git switch staging'); + await $`git switch staging`; - try { - execSync('git rev-parse --verify production', {stdio: 'ignore'}); - exec('git branch -D production'); - } catch (e) {} + if (await refExists('production')) { + await $`git branch -D production`; + } - exec('git switch -c production'); - exec(`git tag ${getVersion()}`); - exec('git push --force --tags origin production'); + await $`git switch -c production`; + await $`git tag ${getVersion()}`; + await $`git push --force --tags origin production`; Log.success('Recreated production from staging!'); } -function createBasicPR(num: number) { +async function createBasicPR(num: number) { const branchName = `pr-${num}`; const content = `Changes from PR #${num}`; const filePath = path.resolve(process.cwd(), `PR${num}.txt`); Log.info(`Creating PR #${num}`); - checkoutRepo(); - setupGitAsHuman(); - exec('git pull'); - exec(`git switch -c ${branchName}`); + await checkoutRepo(); + await setupGitAsHuman(); + await $`git pull`; + await $`git switch -c ${branchName}`; fs.appendFileSync(filePath, content); - exec(`git add ${filePath}`); - exec(`git commit -m "${content}"`); + await $`git add ${filePath}`; + await $`git commit -m ${content}`; Log.success(`Created PR #${num} in branch ${branchName}`); } -function mergePR(num: number) { +async function mergePR(num: number) { const branchName = `pr-${num}`; Log.info(`Merging PR #${num} to main`); - exec('git switch main'); - exec(`git merge ${branchName} --no-ff -m "Merge pull request #${num} from Expensify/${branchName}"`); - exec('git push origin main'); - exec(`git branch -d ${branchName}`); + await $`git switch main`; + const mergeMessage = `Merge pull request #${num} from Expensify/${branchName}`; + await $`git merge ${branchName} --no-ff -m ${mergeMessage}`; + await $`git push origin main`; + await $`git branch -d ${branchName}`; Log.success(`Merged PR #${num} to main`); } -function cherryPickPRToStaging(num: number, resolveVersionBumpConflicts: () => void = () => {}, resolveMergeCommitConflicts: () => void = () => {}) { +async function cherryPickPRToStaging(num: number, resolveVersionBumpConflicts: () => Promise = async () => {}, resolveMergeCommitConflicts: () => Promise = async () => {}) { Log.info(`Cherry-picking PR ${num} to staging...`); - const prMergeCommit = execSync('git rev-parse HEAD', {encoding: 'utf-8'}).trim(); - bumpVersion(VersionUpdater.SEMANTIC_VERSION_LEVELS.BUILD); - const versionBumpCommit = execSync('git rev-parse HEAD', {encoding: 'utf-8'}).trim(); - checkoutRepo(); - setupGitAsOSBotify(); + const prMergeCommit = (await $`git rev-parse HEAD`.text()).trim(); + await bumpVersion(VersionUpdater.SEMANTIC_VERSION_LEVELS.BUILD); + const versionBumpCommit = (await $`git rev-parse HEAD`.text()).trim(); + await checkoutRepo(); + await setupGitAsOSBotify(); mockGetInput.mockReturnValue(VersionUpdater.SEMANTIC_VERSION_LEVELS.PATCH); const previousPatchVersion = getPreviousVersion(); // --shallow-exclude is used to speed up the fetch - exec(`git fetch origin main staging --no-tags --shallow-exclude="${previousPatchVersion}"`); + await $`git fetch origin main staging --no-tags --shallow-exclude=${previousPatchVersion}`; - exec('git switch staging'); - exec('git switch -c cherry-pick-staging'); + await $`git switch staging`; + await $`git switch -c cherry-pick-staging`; try { - exec(`git cherry-pick -x --mainline 1 ${versionBumpCommit}`); + await $`git cherry-pick -x --mainline 1 ${versionBumpCommit}`; } catch (e) { - resolveVersionBumpConflicts(); + await resolveVersionBumpConflicts(); } - setupGitAsHuman(); + await setupGitAsHuman(); try { - exec(`git cherry-pick -x --mainline 1 --strategy=recursive -Xtheirs ${prMergeCommit}`); + await $`git cherry-pick -x --mainline 1 --strategy=recursive -Xtheirs ${prMergeCommit}`; } catch (e) { - resolveMergeCommitConflicts(); + await resolveMergeCommitConflicts(); } - setupGitAsOSBotify(); - exec('git switch staging'); - exec(`git merge cherry-pick-staging --no-ff -m "Merge pull request #${num + 1} from Expensify/cherry-pick-staging"`); - exec('git branch -d cherry-pick-staging'); - exec('git push origin staging'); + await setupGitAsOSBotify(); + await $`git switch staging`; + const mergeMessage = `Merge pull request #${num + 1} from Expensify/cherry-pick-staging`; + await $`git merge cherry-pick-staging --no-ff -m ${mergeMessage}`; + await $`git branch -d cherry-pick-staging`; + await $`git push origin staging`; Log.info(`Merged PR #${num + 1} into staging`); - tagStaging(); + await tagStaging(); Log.success(`Successfully cherry-picked PR #${num} to staging!`); } -function cherryPickPRToProduction(num: number, resolveVersionBumpConflicts: () => void = () => {}, resolveMergeCommitConflicts: () => void = () => {}) { +async function cherryPickPRToProduction(num: number, resolveVersionBumpConflicts: () => Promise = async () => {}, resolveMergeCommitConflicts: () => Promise = async () => {}) { Log.info(`Cherry-picking PR ${num} to production...`); - const prMergeCommit = execSync('git rev-parse HEAD', {encoding: 'utf-8'}).trim(); - bumpVersion(VersionUpdater.SEMANTIC_VERSION_LEVELS.PATCH); - let versionBumpCommit = execSync('git rev-parse HEAD', {encoding: 'utf-8'}).trim(); - checkoutRepo(); - setupGitAsOSBotify(); + const prMergeCommit = (await $`git rev-parse HEAD`.text()).trim(); + await bumpVersion(VersionUpdater.SEMANTIC_VERSION_LEVELS.PATCH); + let versionBumpCommit = (await $`git rev-parse HEAD`.text()).trim(); + await checkoutRepo(); + await setupGitAsOSBotify(); mockGetInput.mockReturnValue(VersionUpdater.SEMANTIC_VERSION_LEVELS.MINOR); const previousPatchVersion = getPreviousVersion(); - exec(`git fetch origin main production --no-tags --shallow-exclude="${previousPatchVersion}"`); + await $`git fetch origin main production --no-tags --shallow-exclude=${previousPatchVersion}`; - exec('git switch production'); - exec('git switch -c cherry-pick-production'); + await $`git switch production`; + await $`git switch -c cherry-pick-production`; try { - exec(`git cherry-pick -x --mainline 1 -Xtheirs ${versionBumpCommit}`); + await $`git cherry-pick -x --mainline 1 -Xtheirs ${versionBumpCommit}`; } catch (e) { - resolveVersionBumpConflicts(); + await resolveVersionBumpConflicts(); } - setupGitAsHuman(); + await setupGitAsHuman(); try { - exec(`git cherry-pick -x --mainline 1 --strategy=recursive -Xtheirs ${prMergeCommit}`); + await $`git cherry-pick -x --mainline 1 --strategy=recursive -Xtheirs ${prMergeCommit}`; } catch (e) { - resolveMergeCommitConflicts(); + await resolveMergeCommitConflicts(); } - setupGitAsOSBotify(); - exec('git switch production'); - exec(`git merge cherry-pick-production --no-ff -m "Merge pull request #${num + 1} from Expensify/cherry-pick-production"`); - exec('git branch -d cherry-pick-production'); - exec('git push origin production'); + await setupGitAsOSBotify(); + await $`git switch production`; + const mergeMessage = `Merge pull request #${num + 1} from Expensify/cherry-pick-production`; + await $`git merge cherry-pick-production --no-ff -m ${mergeMessage}`; + await $`git branch -d cherry-pick-production`; + await $`git push origin production`; Log.info(`Merged PR #${num + 1} into production`); - tagProduction(); - - checkoutRepo(); - bumpVersion(VersionUpdater.SEMANTIC_VERSION_LEVELS.BUILD); - versionBumpCommit = execSync('git rev-parse HEAD', {encoding: 'utf-8'}).trim(); - exec(`git fetch origin staging --depth=1`); - exec(`git switch staging`); - exec(`git cherry-pick -x --mainline 1 -Xtheirs ${versionBumpCommit}`); - exec('git push origin staging'); - tagStaging(); + await tagProduction(); + + await checkoutRepo(); + await bumpVersion(VersionUpdater.SEMANTIC_VERSION_LEVELS.BUILD); + versionBumpCommit = (await $`git rev-parse HEAD`.text()).trim(); + await $`git fetch origin staging --depth=1`; + await $`git switch staging`; + await $`git cherry-pick -x --mainline 1 -Xtheirs ${versionBumpCommit}`; + await $`git push origin staging`; + await tagStaging(); Log.success(`Pushed to staging after CP to production`); Log.success(`Successfully cherry-picked PR #${num} to production!`); } -function tagStaging() { +async function tagStaging() { Log.info('Tagging new version from the staging branch...'); - checkoutRepo(); - setupGitAsOSBotify(); - try { - execSync('git rev-parse --verify staging', {stdio: 'ignore'}); - } catch (e) { - exec('git fetch origin staging --depth=1'); + await checkoutRepo(); + await setupGitAsOSBotify(); + if (!(await refExists('staging'))) { + await $`git fetch origin staging --depth=1`; } - exec('git switch staging'); - exec(`git tag ${getVersion()}-staging`); - exec('git push --tags'); + await $`git switch staging`; + await $`git tag ${getVersion()}-staging`; + await $`git push --tags`; Log.success(`Created new tag ${getVersion()}`); } -function tagProduction() { +async function tagProduction() { Log.info('Tagging new version from the production branch...'); Log.info(`Version is: ${getVersion()}`); - checkoutRepo(); - setupGitAsOSBotify(); - try { - execSync('git rev-parse --verify production', {stdio: 'ignore'}); - } catch (e) { - exec('git fetch origin production --depth=1'); + await checkoutRepo(); + await setupGitAsOSBotify(); + if (!(await refExists('production'))) { + await $`git fetch origin production --depth=1`; } - exec('git switch production'); - exec(`git tag ${getVersion()}`); - exec('git push --tags'); + await $`git switch production`; + await $`git tag ${getVersion()}`; + await $`git push --tags`; Log.success(`Created new tag ${getVersion()}`); } -function deployStaging() { +async function deployStaging() { Log.info('Deploying staging...'); - checkoutRepo(); - bumpVersion(VersionUpdater.SEMANTIC_VERSION_LEVELS.BUILD); - updateStagingFromMain(); - tagStaging(); + await checkoutRepo(); + await bumpVersion(VersionUpdater.SEMANTIC_VERSION_LEVELS.BUILD); + await updateStagingFromMain(); + await tagStaging(); Log.success(`Deployed ${getVersion()} to staging!`); } -function deployProduction() { +async function deployProduction() { Log.info('Checklist closed, deploying production and staging...'); Log.info('Deploying production...'); - updateProductionFromStaging(); + await updateProductionFromStaging(); Log.success(`Deployed v${getVersion()} to production!`); Log.info('Deploying staging...'); - bumpVersion(VersionUpdater.SEMANTIC_VERSION_LEVELS.PATCH); - updateStagingFromMain(); - tagStaging(); + await bumpVersion(VersionUpdater.SEMANTIC_VERSION_LEVELS.PATCH); + await updateStagingFromMain(); + await tagStaging(); Log.success(`Deployed v${getVersion()} to staging!`); } async function assertPRsMergedBetween(from: string, to: string, expected: number[]) { - checkoutRepo(); + await checkoutRepo(); const PRs = await GitUtils.getPullRequestsDeployedBetween(from, to, CONST.APP_REPO); expect(PRs).toStrictEqual(expected); Log.success(`Verified PRs merged between ${from} and ${to} are [${expected.join(',')}]`); } /* - * These tests are different from most jest tests. They create a dummy git repo and simulate the GitHub Actions CI environment + * These tests are different from most of the suite. They create a dummy git repo and simulate the GitHub Actions CI environment * and ensure that deploy checklists, comments, and releases are created correctly and completely, * including a number of real-world edge cases we have encountered and fixed. * * However, because they are different, there are a few additional "rules" with these tests: - * - They should not be run in parallel with other tests on the same machine. They will not play nicely with other tests. * - The whole suite should be run. Running individual tests from the suite may not work as expected. + * - Each test builds on the repo state the previous one left behind, so the first failure cascades into the rest. + * That chain is why the suite is `describe.serial`, which pins the order no matter what flags Bun is given. + * Re-run with `--bail` to see only the first failure; Bun has no per-file equivalent. + * - The suite changes the process-wide cwd, because the git helpers it exercises resolve `git` against + * `process.cwd()` exactly as they do in a real GitHub Actions checkout. It is restored in `afterAll`, + * but it does mean this file needs its own process to run truly in parallel with other files (`--parallel`, + * which Bun implements with worker processes, gives it one). */ +// These tests shell out to real `git`/`npm` subprocesses many times per test and can exceed the default 5000ms +// per-test timeout, especially on a cold cache. +setDefaultTimeout(30000); + let startingDir: string; -describe('CIGitLogic', () => { - beforeAll(() => { +describe.serial('CIGitLogic', () => { + beforeAll(async () => { Log.info('Starting setup'); startingDir = process.cwd(); - initGitServer(); + await initGitServer(); initGithubAPIMocking(); - checkoutRepo(); + await checkoutRepo(); Log.success('Setup complete!'); }); afterAll(() => { jest.restoreAllMocks(); - fs.rmSync(DUMMY_DIR, {recursive: true, force: true}); - fs.rmSync(path.resolve(GIT_REMOTE, '..'), {recursive: true, force: true}); + // Restore the cwd before removing the sandbox, so the process is never left sitting in a deleted directory. process.chdir(startingDir); + fs.rmSync(SANDBOX_DIR, {recursive: true, force: true}); }); test('Merge a pull request while the checklist is unlocked', async () => { - createBasicPR(1); - mergePR(1); - deployStaging(); + await createBasicPR(1); + await mergePR(1); + await deployStaging(); // Verify output for checklist and deploy comment await assertPRsMergedBetween('2.0.0-0', '2.0.0-1-staging', [1]); }); test("Merge a pull request with the checklist locked, but don't CP it", async () => { - createBasicPR(2); - mergePR(2); + await createBasicPR(2); + await mergePR(2); // Verify output for checklist and deploy comment, and make sure PR #2 is not on staging await assertPRsMergedBetween('2.0.0-0', '2.0.0-1-staging', [1]); }); test('Merge a pull request with the checklist locked and CP it to staging', async () => { - createBasicPR(3); - mergePR(3); - cherryPickPRToStaging(3); + await createBasicPR(3); + await mergePR(3); + await cherryPickPRToStaging(3); // Verify output for checklist await assertPRsMergedBetween('2.0.0-0', '2.0.0-2-staging', [1, 3]); @@ -516,9 +522,9 @@ describe('CIGitLogic', () => { }); test('Merge a pull request with the checklist locked and CP it to production', async () => { - createBasicPR(5); - mergePR(5); - cherryPickPRToProduction(5); + await createBasicPR(5); + await mergePR(5); + await cherryPickPRToProduction(5); // Verify output for checklist await assertPRsMergedBetween('2.0.0-0', '2.0.1-1-staging', [1, 3]); @@ -528,7 +534,7 @@ describe('CIGitLogic', () => { }); test('Close the checklist, deploy production and staging', async () => { - deployProduction(); + await deployProduction(); // Verify output for release body and production deploy comments await assertPRsMergedBetween('2.0.0-0', '2.0.1-1', [1, 3]); @@ -538,9 +544,9 @@ describe('CIGitLogic', () => { }); test('Merging another pull request when the checklist is unlocked', async () => { - createBasicPR(6); - mergePR(6); - deployStaging(); + await createBasicPR(6); + await mergePR(6); + await deployStaging(); // Verify output for checklist await assertPRsMergedBetween('2.0.0-2-staging', '2.0.2-1-staging', [2, 5, 6]); @@ -551,16 +557,17 @@ describe('CIGitLogic', () => { test('Deploying a PR, then CPing a revert, then adding the same code back again before the next production deploy results in the correct code on staging and production', async () => { Log.info('Creating myFile.txt in PR #7'); - setupGitAsHuman(); - exec('git switch main'); - exec('git switch -c pr-7'); + await setupGitAsHuman(); + await $`git switch main`; + await $`git switch -c pr-7`; const initialFileContent = 'Changes from PR #7'; fs.appendFileSync('myFile.txt', 'Changes from PR #7'); - exec('git add myFile.txt'); - exec('git commit -m "Add myFile.txt in PR #7"'); + await $`git add myFile.txt`; + const commitMessage = 'Add myFile.txt in PR #7'; + await $`git commit -m ${commitMessage}`; - mergePR(7); - deployStaging(); + await mergePR(7); + await deployStaging(); // Verify output for checklist await assertPRsMergedBetween('2.0.0-2-staging', '2.0.2-2-staging', [2, 5, 6, 7]); @@ -569,19 +576,19 @@ describe('CIGitLogic', () => { await assertPRsMergedBetween('2.0.2-1-staging', '2.0.2-2-staging', [7]); Log.info('Appending and prepending content to myFile.txt in PR #8'); - setupGitAsHuman(); - exec('git switch main'); - exec('git switch -c pr-8'); + await setupGitAsHuman(); + await $`git switch main`; + await $`git switch -c pr-8`; const newFileContent = ` Prepended content ${initialFileContent} Appended content `; fs.writeFileSync('myFile.txt', newFileContent, {encoding: 'utf-8'}); - exec('git add myFile.txt'); - exec('git commit -m "Append and prepend content in myFile.txt"'); - mergePR(8); - deployStaging(); + await $`git add myFile.txt`; + await $`git commit -m "Append and prepend content in myFile.txt"`; + await mergePR(8); + await deployStaging(); // Verify output for checklist await assertPRsMergedBetween('2.0.0-2-staging', '2.0.2-3-staging', [2, 5, 6, 7, 8]); @@ -590,38 +597,38 @@ Appended content await assertPRsMergedBetween('2.0.2-2-staging', '2.0.2-3-staging', [8]); Log.info('Making an unrelated change in PR #9'); - setupGitAsHuman(); - exec('git switch main'); - exec('git switch -c pr-9'); + await setupGitAsHuman(); + await $`git switch main`; + await $`git switch -c pr-9`; fs.appendFileSync('anotherFile.txt', 'some content'); - exec('git add anotherFile.txt'); - exec('git commit -m "Create another file"'); - mergePR(9); + await $`git add anotherFile.txt`; + await $`git commit -m "Create another file"`; + await mergePR(9); Log.info('Reverting the append + prepend on main in PR #10'); - setupGitAsHuman(); - exec('git switch main'); - exec('git switch -c pr-10'); + await setupGitAsHuman(); + await $`git switch main`; + await $`git switch -c pr-10`; fs.writeFileSync('myFile.txt', initialFileContent); - exec('git add myFile.txt'); - exec('git commit -m "Revert append and prepend"'); - mergePR(10); - cherryPickPRToStaging(10); + await $`git add myFile.txt`; + await $`git commit -m "Revert append and prepend"`; + await mergePR(10); + await cherryPickPRToStaging(10); Log.info('Verifying that the revert is present on staging, but the unrelated change is not'); expect(fs.readFileSync('myFile.txt', {encoding: 'utf8'})).toBe(initialFileContent); expect(fs.existsSync('anotherFile.txt')).toBe(false); Log.info('Repeating previously reverted append + prepend on main in PR #10'); - setupGitAsHuman(); - exec('git switch main'); - exec('git switch -c pr-11'); + await setupGitAsHuman(); + await $`git switch main`; + await $`git switch -c pr-11`; fs.writeFileSync('myFile.txt', newFileContent, {encoding: 'utf-8'}); - exec('git add myFile.txt'); - exec('git commit -m "Append and prepend content in myFile.txt"'); + await $`git add myFile.txt`; + await $`git commit -m "Append and prepend content in myFile.txt"`; - mergePR(11); - deployProduction(); + await mergePR(11); + await deployProduction(); // Verify production release list await assertPRsMergedBetween('2.0.1-1', '2.0.2-4', [2, 5, 6, 7, 8, 10]); @@ -631,11 +638,11 @@ Appended content }); test('Force-pushing to a branch after rebasing older commits', async () => { - createBasicPR(12); - exec('git push origin pr-12'); - createBasicPR(13); - mergePR(13); - deployStaging(); + await createBasicPR(12); + await $`git push origin pr-12`; + await createBasicPR(13); + await mergePR(13); + await deployStaging(); // Verify PRs for checklist await assertPRsMergedBetween('2.0.2-4-staging', '2.0.3-1-staging', [9, 11, 13]); @@ -643,15 +650,15 @@ Appended content // Verify PRs for deploy comments await assertPRsMergedBetween('2.0.3-0-staging', '2.0.3-1-staging', [13]); - checkoutRepo(); - setupGitAsHuman(); - exec('git fetch origin pr-12'); - exec('git switch pr-12'); - exec('git rebase main -Xours'); - exec('git push --force origin pr-12'); - mergePR(12); + await checkoutRepo(); + await setupGitAsHuman(); + await $`git fetch origin pr-12`; + await $`git switch pr-12`; + await $`git rebase main -Xours`; + await $`git push --force origin pr-12`; + await mergePR(12); - deployProduction(); + await deployProduction(); // Verify PRs for deploy comments / release await assertPRsMergedBetween('2.0.2-4-staging', '2.0.3-1-staging', [9, 11, 13]); @@ -662,50 +669,52 @@ Appended content test('Manual version bump', async () => { Log.info('Creating manual version bump in PR #14'); - checkoutRepo(); - setupGitAsHuman(); - exec('git pull'); - exec('git switch -c "pr-14"'); + await checkoutRepo(); + await setupGitAsHuman(); + await $`git pull`; + await $`git switch -c pr-14`; for (let i = 0; i < 3; i++) { - exec(`npm --no-git-tag-version version ${VersionUpdater.incrementVersion(getVersion(), VersionUpdater.SEMANTIC_VERSION_LEVELS.MAJOR)}`); + await $`npm --no-git-tag-version version ${VersionUpdater.incrementVersion(getVersion(), VersionUpdater.SEMANTIC_VERSION_LEVELS.MAJOR)}`; } - exec('git add package.json'); - exec(`git commit -m "Manually bump version to ${getVersion()} in PR #14"`); + await $`git add package.json`; + const bumpMessage14 = `Manually bump version to ${getVersion()} in PR #14`; + await $`git commit -m ${bumpMessage14}`; Log.success('Created manual version bump in PR #13 in branch pr-14'); - mergePR(14); + await mergePR(14); Log.info('Deploying staging...'); - checkoutRepo(); - updateStagingFromMain(); - tagStaging(); + await checkoutRepo(); + await updateStagingFromMain(); + await tagStaging(); Log.success(`Deployed v${getVersion()} to staging!`); // Verify PRs for deploy comments / release and new checklist await assertPRsMergedBetween('2.0.4-0-staging', '5.0.0-0-staging', [14]); Log.info('Creating manual version bump in PR #15'); - checkoutRepo(); - setupGitAsHuman(); - exec('git pull'); - exec('git switch -c "pr-15"'); + await checkoutRepo(); + await setupGitAsHuman(); + await $`git pull`; + await $`git switch -c pr-15`; for (let i = 0; i < 3; i++) { - exec(`npm --no-git-tag-version version ${VersionUpdater.incrementVersion(getVersion(), VersionUpdater.SEMANTIC_VERSION_LEVELS.MAJOR)}`); + await $`npm --no-git-tag-version version ${VersionUpdater.incrementVersion(getVersion(), VersionUpdater.SEMANTIC_VERSION_LEVELS.MAJOR)}`; } - exec('git add package.json'); - exec(`git commit -m "Manually bump version to ${getVersion()} in PR #15"`); + await $`git add package.json`; + const bumpMessage15 = `Manually bump version to ${getVersion()} in PR #15`; + await $`git commit -m ${bumpMessage15}`; Log.success('Created manual version bump in PR #15 in branch pr-15'); const packageJSONBefore = fs.readFileSync('package.json', {encoding: 'utf-8'}); - mergePR(15); - cherryPickPRToStaging( + await mergePR(15); + await cherryPickPRToStaging( 15, - () => { + async () => { fs.writeFileSync('package.json', packageJSONBefore); - exec('git add package.json'); - exec('git cherry-pick --no-edit --continue'); + await $`git add package.json`; + await $`git cherry-pick --no-edit --continue`; }, - () => { - exec('git commit --no-edit --allow-empty'); + async () => { + await $`git commit --no-edit --allow-empty`; }, ); diff --git a/tests/unit/ChatGPTTranslatorTest.ts b/tests/tooling/ChatGPTTranslator.test.ts similarity index 91% rename from tests/unit/ChatGPTTranslatorTest.ts rename to tests/tooling/ChatGPTTranslator.test.ts index 8d78d03867be..55dd0cbd63a2 100644 --- a/tests/unit/ChatGPTTranslatorTest.ts +++ b/tests/tooling/ChatGPTTranslator.test.ts @@ -1,13 +1,12 @@ -/** - * @jest-environment node - */ +import {beforeEach, describe, expect, it, jest} from 'bun:test'; + import OpenAIUtils from '@scripts/utils/OpenAIUtils'; import ChatGPTTranslator from '@scripts/utils/Translator/ChatGPTTranslator'; -import type Locale from '@src/types/onyx/Locale'; - -jest.mock('@scripts/utils/OpenAIUtils'); +import type {TranslationTargetLocale} from '@src/CONST/LOCALES'; +// Only `promptResponses` needs stubbing: the OpenAIUtils constructor just stores the key and builds a client, so +// there is nothing to gain from replacing the whole module (which Bun has no automock for anyway). const mockedPromptResponses = jest.spyOn(OpenAIUtils.prototype, 'promptResponses'); /** @@ -19,7 +18,7 @@ function mockResponse(text: string, responseID = 'resp_test_123') { describe('ChatGPTTranslator.performTranslation', () => { const apiKey = 'test-api-key'; - const targetLang: Locale = 'it'; + const targetLang: TranslationTargetLocale = 'it'; const maxRetries = 8; // eslint-disable-next-line no-template-curly-in-string const original = 'Hello ${name}!'; diff --git a/tests/unit/DeployChecklistUtilsTest.ts b/tests/tooling/DeployChecklistUtils.test.ts similarity index 86% rename from tests/unit/DeployChecklistUtilsTest.ts rename to tests/tooling/DeployChecklistUtils.test.ts index b645d39a3fa8..115e2f9dd9eb 100644 --- a/tests/unit/DeployChecklistUtilsTest.ts +++ b/tests/tooling/DeployChecklistUtils.test.ts @@ -1,11 +1,11 @@ +import type {Mock} from 'bun:test'; +import {afterAll, afterEach, beforeAll, beforeEach, describe, expect, jest, test} from 'bun:test'; + import CONST from '@github/libs/CONST'; import {generateDeployChecklistBodyAndAssignees, getDeployChecklist, NoOpenDeployChecklistError} from '@github/libs/DeployChecklistUtils'; import type {InternalOctokit, ListForRepoMethod, OctokitIssueItem} from '@github/libs/GithubUtils'; import GithubUtils from '@github/libs/GithubUtils'; -/** - * @jest-environment node - */ /* eslint-disable @typescript-eslint/naming-convention */ import {RequestError} from '@octokit/request-error'; @@ -19,10 +19,49 @@ type GetPullRequestResponse = Awaited>; const createListForRepoResponse = (data: OctokitIssueItem[]): ListForRepoResponse => createMock({data}); -const mockListIssues = jest.fn, Parameters>(); -let listForRepoSpy: jest.SpiedFunction; +let listForRepoSpy: Mock; let internalOctokit: InternalOctokit; +/** + * Runs `operation` with fake timers so its retry backoff resolves instantly, advancing the clock by exactly + * `expectedDelaysMs` in order. Advancing by the exact delays rather than some arbitrarily large amount keeps + * these tests pinned to LIST_RETRY_DELAYS_MS: lengthen a delay there and the operation never settles. + * + * Bun only exposes a synchronous `jest.advanceTimersByTime`, and the code under test schedules each backoff timer + * from a `catch` block - i.e. several microtasks after the call starts - so yield until that timer exists before + * firing it. + */ +async function runWithFakeTimers(operation: () => Promise, expectedDelaysMs: number[]): Promise { + try { + jest.useFakeTimers(); + let isSettled = false; + const pending = operation().finally(() => { + isSettled = true; + }); + + // The caller decides whether a rejection is expected; swallow it here only so driving the clock below + // doesn't trip Bun's unhandled-rejection reporting in the meantime. + pending.catch(() => {}); + + for (const delayMs of expectedDelaysMs) { + for (let i = 0; jest.getTimerCount() === 0 && !isSettled && i < 100; i++) { + await Promise.resolve(); + } + jest.advanceTimersByTime(delayMs); + } + + for (let i = 0; !isSettled && i < 100; i++) { + await Promise.resolve(); + } + if (!isSettled) { + throw new Error(`Operation did not settle after advancing the clock by ${expectedDelaysMs.join(' + ')}ms; did its retry schedule change?`); + } + return await pending; + } finally { + jest.useRealTimers(); + } +} + beforeAll(() => { GithubUtils.initOctokitWithToken('fake_token'); const initializedOctokit = GithubUtils.internalOctokit; @@ -31,12 +70,11 @@ beforeAll(() => { } internalOctokit = initializedOctokit; - listForRepoSpy = jest.spyOn(internalOctokit.rest.issues, 'listForRepo').mockImplementation(mockListIssues); + listForRepoSpy = jest.spyOn(internalOctokit.rest.issues, 'listForRepo'); }); afterEach(() => { - listForRepoSpy.mockClear(); - mockListIssues.mockReset(); + listForRepoSpy.mockReset(); }); describe('DeployChecklistUtils', () => { @@ -63,7 +101,7 @@ describe('DeployChecklistUtils', () => { issueWithDeployBlockers.body += `\r\n**Deploy Blockers:**\r\n- [ ] https://github.com/${process.env.GITHUB_REPOSITORY}/issues/1\r\n- [x] https://github.com/${process.env.GITHUB_REPOSITORY}/issues/2\r\n- [ ] https://github.com/${process.env.GITHUB_REPOSITORY}/pull/1234\r\n`; - const baseExpectedResponse: Partial>> = { + const baseExpectedResponse: Awaited> = { PRList: [ { url: `https://github.com/${process.env.GITHUB_REPOSITORY}/pull/21`, @@ -130,23 +168,23 @@ describe('DeployChecklistUtils', () => { body: `**Release Version:** \`1.0.1-47\`\r\n**Compare Changes:** https://github.com/${process.env.GITHUB_REPOSITORY}/compare/production...staging\r\n\r\ncc @Expensify/applauseleads\n`, }); - const bareExpectedResponse: Partial>> = { + const bareExpectedResponse: Awaited> = { ...baseExpectedResponse, PRList: [], PRListMobileExpensify: [], }; - mockListIssues.mockResolvedValue(createListForRepoResponse([bareIssue])); + listForRepoSpy.mockResolvedValue(createListForRepoResponse([bareIssue])); return getDeployChecklist().then((data) => expect(data).toStrictEqual(bareExpectedResponse)); }); test('Test finding an open issue successfully', () => { - mockListIssues.mockResolvedValue(createListForRepoResponse([baseIssue])); + listForRepoSpy.mockResolvedValue(createListForRepoResponse([baseIssue])); return getDeployChecklist().then((data) => expect(data).toStrictEqual(baseExpectedResponse)); }); test('Test finding an open issue successfully and parsing with deploy blockers', () => { - mockListIssues.mockResolvedValue(createListForRepoResponse([issueWithDeployBlockers])); + listForRepoSpy.mockResolvedValue(createListForRepoResponse([issueWithDeployBlockers])); return getDeployChecklist().then((data) => expect(data).toStrictEqual(expectedResponseWithDeployBlockers)); }); @@ -154,14 +192,14 @@ describe('DeployChecklistUtils', () => { const modifiedIssueWithDeployBlockers = {...issueWithDeployBlockers}; modifiedIssueWithDeployBlockers.body = (modifiedIssueWithDeployBlockers.body ?? '').replaceAll('\r', ''); - mockListIssues.mockResolvedValue(createListForRepoResponse([modifiedIssueWithDeployBlockers])); + listForRepoSpy.mockResolvedValue(createListForRepoResponse([modifiedIssueWithDeployBlockers])); return getDeployChecklist().then((data) => expect(data).toStrictEqual(expectedResponseWithDeployBlockers)); }); test('Test finding an open issue without a body', () => { const noBodyIssue = {...baseIssue, body: ''}; - mockListIssues.mockResolvedValue(createListForRepoResponse([noBodyIssue])); + listForRepoSpy.mockResolvedValue(createListForRepoResponse([noBodyIssue])); return getDeployChecklist().then((data) => expect(data).toMatchObject({ PRList: [], @@ -179,12 +217,12 @@ describe('DeployChecklistUtils', () => { test('Test finding an open issue with malformed URL', async () => { const malformedURLIssue = {...baseIssue, url: 'invalid-url'}; - mockListIssues.mockResolvedValue(createListForRepoResponse([malformedURLIssue])); + listForRepoSpy.mockResolvedValue(createListForRepoResponse([malformedURLIssue])); await expect(getDeployChecklist()).rejects.toThrow(`Unable to find ${CONST.LABELS.STAGING_DEPLOY} issue with correct data.`); }); test('Test finding more than one issue', async () => { - mockListIssues.mockResolvedValue(createListForRepoResponse([createMock({number: 1}), createMock({number: 2})])); + listForRepoSpy.mockResolvedValue(createListForRepoResponse([createMock({number: 1}), createMock({number: 2})])); try { await getDeployChecklist(); throw new Error('Expected getDeployChecklist to reject'); @@ -194,7 +232,7 @@ describe('DeployChecklistUtils', () => { }); test('state:open empty + state:all returns closed issue → NoOpenDeployChecklistError', async () => { - mockListIssues + listForRepoSpy .mockResolvedValueOnce(createListForRepoResponse([])) .mockResolvedValueOnce(createListForRepoResponse([createMock({number: 100, state: 'closed'})])); try { @@ -205,12 +243,12 @@ describe('DeployChecklistUtils', () => { if (!(e instanceof Error)) { throw e; } - expect(e.message).toEqual(expect.stringContaining('#100')); + expect(e.message).toContain('#100'); } }); test('state:open empty + state:all returns open issue → fails closed (inconsistency)', async () => { - mockListIssues + listForRepoSpy .mockResolvedValueOnce(createListForRepoResponse([])) .mockResolvedValueOnce(createListForRepoResponse([createMock({number: 500, state: 'open'})])); try { @@ -221,13 +259,13 @@ describe('DeployChecklistUtils', () => { if (!(e instanceof Error)) { throw e; } - expect(e.message).toEqual(expect.stringContaining('Inconsistent GitHub response')); - expect(e.message).toEqual(expect.stringContaining('#500')); + expect(e.message).toContain('Inconsistent GitHub response'); + expect(e.message).toContain('#500'); } }); test('state:open empty + state:all empty → fails closed (pathological)', async () => { - mockListIssues.mockResolvedValue(createListForRepoResponse([])); + listForRepoSpy.mockResolvedValue(createListForRepoResponse([])); try { await getDeployChecklist(); throw new Error('Expected getDeployChecklist to reject'); @@ -236,7 +274,7 @@ describe('DeployChecklistUtils', () => { if (!(e instanceof Error)) { throw e; } - expect(e.message).toEqual(expect.stringContaining(`No StagingDeployCash issues found at all`)); + expect(e.message).toContain(`No StagingDeployCash issues found at all`); } }); }); @@ -246,47 +284,31 @@ describe('DeployChecklistUtils', () => { const err503 = new RequestError('Service Unavailable', 503, { request: {method: 'GET', url: 'https://api.github.com/repos/o/i/issues', headers: {}}, }); - mockListIssues + listForRepoSpy .mockRejectedValueOnce(err503) .mockResolvedValueOnce( createListForRepoResponse([createMock({number: 88, url: 'https://api.github.com/repos/o/i/issues/88', title: 't', labels: [], body: ''})]), ); - jest.useFakeTimers(); - try { - const pending = getDeployChecklist(); - await jest.advanceTimersByTimeAsync(2000); - const data = await pending; - - expect(data.number).toBe(88); - expect(GithubUtils.octokit.issues.listForRepo).toHaveBeenCalledTimes(2); - } finally { - jest.useRealTimers(); - } + const data = await runWithFakeTimers(() => getDeployChecklist(), [2000]); + + expect(data.number).toBe(88); + expect(GithubUtils.octokit.issues.listForRepo).toHaveBeenCalledTimes(2); }); test('re-throws after all retry attempts fail', async () => { const err503 = new RequestError('Service Unavailable', 503, { request: {method: 'GET', url: 'https://api.github.com/repos/o/i/issues', headers: {}}, }); - mockListIssues.mockRejectedValue(err503); + listForRepoSpy.mockRejectedValue(err503); - jest.useFakeTimers(); - try { - const pending = getDeployChecklist(); - const assertion = expect(pending).rejects.toThrow(RequestError); - await jest.advanceTimersByTimeAsync(2000); - await jest.advanceTimersByTimeAsync(5000); - await assertion; - - expect(GithubUtils.octokit.issues.listForRepo).toHaveBeenCalledTimes(3); - } finally { - jest.useRealTimers(); - } + await expect(runWithFakeTimers(() => getDeployChecklist(), [2000, 5000])).rejects.toThrow(RequestError); + + expect(GithubUtils.octokit.issues.listForRepo).toHaveBeenCalledTimes(3); }); test('does not retry on empty result; falls through to state:all cross-check', async () => { - mockListIssues + listForRepoSpy .mockResolvedValueOnce(createListForRepoResponse([])) .mockResolvedValueOnce(createListForRepoResponse([createMock({number: 200, state: 'closed'})])); await expect(getDeployChecklist()).rejects.toBeInstanceOf(NoOpenDeployChecklistError); @@ -297,7 +319,7 @@ describe('DeployChecklistUtils', () => { const err404 = new RequestError('Not Found', 404, { request: {method: 'GET', url: 'https://api.github.com/repos/o/i/issues', headers: {}}, }); - mockListIssues.mockRejectedValue(err404); + listForRepoSpy.mockRejectedValue(err404); await expect(getDeployChecklist()).rejects.toBeInstanceOf(RequestError); expect(GithubUtils.octokit.issues.listForRepo).toHaveBeenCalledTimes(1); @@ -307,27 +329,20 @@ describe('DeployChecklistUtils', () => { const err403 = new RequestError('Secondary rate limit', 403, { request: {method: 'GET', url: 'https://api.github.com/repos/o/i/issues', headers: {}}, }); - mockListIssues + listForRepoSpy .mockRejectedValueOnce(err403) .mockResolvedValueOnce( createListForRepoResponse([createMock({number: 77, url: 'https://api.github.com/repos/o/i/issues/77', title: 't', labels: [], body: ''})]), ); - jest.useFakeTimers(); - try { - const pending = getDeployChecklist(); - await jest.advanceTimersByTimeAsync(2000); - const data = await pending; - - expect(data.number).toBe(77); - expect(GithubUtils.octokit.issues.listForRepo).toHaveBeenCalledTimes(2); - } finally { - jest.useRealTimers(); - } + const data = await runWithFakeTimers(() => getDeployChecklist(), [2000]); + + expect(data.number).toBe(77); + expect(GithubUtils.octokit.issues.listForRepo).toHaveBeenCalledTimes(2); }); test('state:all reports a non-first open issue → fails closed with that number', async () => { - mockListIssues + listForRepoSpy .mockResolvedValueOnce(createListForRepoResponse([])) .mockResolvedValueOnce( createListForRepoResponse([ @@ -344,8 +359,8 @@ describe('DeployChecklistUtils', () => { if (!(e instanceof Error)) { throw e; } - expect(e.message).toEqual(expect.stringContaining('Inconsistent GitHub response')); - expect(e.message).toEqual(expect.stringContaining('#800')); + expect(e.message).toContain('Inconsistent GitHub response'); + expect(e.message).toContain('#800'); } }); }); @@ -360,8 +375,8 @@ describe('DeployChecklistUtils', () => { createMock({number: 6, title: '[Internal QA] Another Test Internal QA PR', labels: [{name: 'InternalQA'}]}), createMock({number: 7, title: '[Internal QA] Another Test Internal QA PR', labels: [{name: 'InternalQA'}]}), ]; - let paginateSpy: jest.SpiedFunction; - let getPullRequestSpy: jest.SpiedFunction; + let paginateSpy: Mock; + let getPullRequestSpy: Mock; beforeAll(() => { paginateSpy = jest.spyOn(internalOctokit, 'paginate'); @@ -369,8 +384,10 @@ describe('DeployChecklistUtils', () => { }); beforeEach(() => { - paginateSpy.mockImplementation(async () => mockPRs); - getPullRequestSpy.mockImplementation(async (parameters) => { + paginateSpy.mockResolvedValue(mockPRs); + // Octokit endpoint methods carry `defaults`/`endpoint` statics that mockImplementation insists on but + // the action never touches, so the stub only implements the call signature. + const getPullRequest = async (parameters?: Parameters[0]) => { if (!parameters) { throw new Error('Expected pull request parameters.'); } @@ -381,7 +398,9 @@ describe('DeployChecklistUtils', () => { merged_by: pullRequest ? {login: 'octocat'} : null, }, }); - }); + }; + // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion -- the stub deliberately omits the statics described above + getPullRequestSpy.mockImplementation(getPullRequest as unknown as OctokitGetPullRequest); }); afterEach(() => { diff --git a/tests/unit/GitTest.ts b/tests/tooling/Git.test.ts similarity index 97% rename from tests/unit/GitTest.ts rename to tests/tooling/Git.test.ts index 0f8167be4a7a..e2810ea12253 100644 --- a/tests/unit/GitTest.ts +++ b/tests/tooling/Git.test.ts @@ -1,17 +1,23 @@ -import Git from '@scripts/utils/Git'; +import type {Mock} from 'bun:test'; +import {afterEach, beforeEach, describe, expect, it, jest, mock} from 'bun:test'; -/** - * @jest-environment node - */ -import {execSync} from 'child_process'; +import * as childProcess from 'child_process'; import {Str} from 'expensify-common'; import fs from 'fs'; import createMock from '../utils/createMock'; -// Mock execSync to control git diff output -jest.mock('child_process'); -const mockExecSync = jest.mocked(execSync); +// Mock execSync to control git diff output. Bun has no equivalent of `jest.mock(path)`'s automock, and +// `child_process`'s named exports are read-only live bindings, so replace the whole module. This must run before +// `Git` (which imports execSync internally) is imported below, and `bun test --isolate` keeps the replacement from +// leaking into the other files in tests/tooling. +// Typed to the single overload Git.ts actually uses (it always passes `encoding: 'utf8'`), so the test's +// string-returning stubs type-check against it. +const mockExecSync = jest.fn<(command: string, options?: childProcess.ExecSyncOptions) => string>(); +await mock.module('child_process', () => ({...childProcess, execSync: mockExecSync})); + +// Must be imported after the mock.module() call above so it picks up the mock. +const {default: Git} = await import('@scripts/utils/Git'); // Test constants for untracked files tests const MOCK_COMPONENT_CONTENT = 'const Component = () => null;\n'; @@ -1223,8 +1229,8 @@ describe('Git', () => { }); describe('diff with shouldIncludeUntrackedFiles', () => { - let mockExistsSync: jest.SpyInstance; - let mockReadFileSync: jest.SpyInstance; + let mockExistsSync: Mock; + let mockReadFileSync: Mock; beforeEach(() => { jest.clearAllMocks(); @@ -1252,7 +1258,7 @@ describe('Git', () => { // Verify git ls-files was not called const calls = mockExecSync.mock.calls.map((call) => call[0]); - expect(calls).not.toContain(expect.stringContaining('git ls-files')); + expect(calls.some((command) => command.includes('git ls-files'))).toBe(false); }); it('does not include untracked files when toRef is provided even if shouldIncludeUntrackedFiles is true', () => { @@ -1262,7 +1268,7 @@ describe('Git', () => { expect(mockExecSync).toHaveBeenCalledTimes(1); const calls = mockExecSync.mock.calls.map((call) => call[0]); - expect(calls).not.toContain(expect.stringContaining('git ls-files')); + expect(calls.some((command) => command.includes('git ls-files'))).toBe(false); }); it('includes untracked files when shouldIncludeUntrackedFiles is true and toRef is undefined', () => { diff --git a/tests/unit/GitUtilsTest.ts b/tests/tooling/GitUtils.test.ts similarity index 99% rename from tests/unit/GitUtilsTest.ts rename to tests/tooling/GitUtils.test.ts index 9de2b3182ca1..a87fe42bafcb 100644 --- a/tests/unit/GitUtilsTest.ts +++ b/tests/tooling/GitUtils.test.ts @@ -1,3 +1,5 @@ +import {describe, expect, test} from 'bun:test'; + import type {CommitType, MergedPR, SubmoduleUpdate} from '../../.github/libs/GitUtils'; import GitUtils from '../../.github/libs/GitUtils'; diff --git a/tests/unit/GithubUtilsTest.ts b/tests/tooling/GithubUtils.test.ts similarity index 98% rename from tests/unit/GithubUtilsTest.ts rename to tests/tooling/GithubUtils.test.ts index 48e88c732dff..203192242f24 100644 --- a/tests/unit/GithubUtilsTest.ts +++ b/tests/tooling/GithubUtils.test.ts @@ -1,10 +1,10 @@ +import type {Mock} from 'bun:test'; +import {afterEach, beforeAll, beforeEach, describe, expect, jest, test} from 'bun:test'; + import CONST from '@github/libs/CONST'; import type {InternalOctokit} from '@github/libs/GithubUtils'; import GithubUtils from '@github/libs/GithubUtils'; -/** - * @jest-environment node - */ /* eslint-disable @typescript-eslint/naming-convention */ import * as core from '@actions/core'; import {RequestError} from '@octokit/request-error'; @@ -174,7 +174,7 @@ describe('GithubUtils', () => { }; describe('getCommitHistoryBetweenTags', () => { - let mockCompareCommits: jest.SpiedFunction; + let mockCompareCommits: Mock; beforeEach(() => { jest.spyOn(core, 'getInput').mockImplementation((name) => { @@ -246,7 +246,7 @@ describe('GithubUtils', () => { }); test('should handle 404 RequestError with specific error message', async () => { - const coreErrorSpy = jest.spyOn(core, 'error').mockImplementation(); + const coreErrorSpy = jest.spyOn(core, 'error').mockImplementation(() => {}); const requestError = new RequestError('Not Found', 404, { request: { method: 'GET', diff --git a/tests/tooling/README.md b/tests/tooling/README.md new file mode 100644 index 000000000000..9a6e63c9164e --- /dev/null +++ b/tests/tooling/README.md @@ -0,0 +1,89 @@ +# Tooling tests + +Tests for the code that builds, deploys and lints this repo — `.github/actions/`, `.github/libs/`, +`.github/scripts/` and `scripts/` — as opposed to the app itself. They run under +[`bun:test`](https://bun.com/docs/cli/test), not Jest. + +## Running them + +These run as part of `test:bun`, alongside the `server/` suite — one Bun invocation covering both roots. + +```sh +# Everything Bun runs (this is what CI runs) +npm run test:bun + +# Just this directory +TZ=utc bun test --parallel --preload ./scripts/stubReactNative.js --preload ./tests/tooling/setup.ts ./tests/tooling + +# One file +TZ=utc bun test --parallel --preload ./scripts/stubReactNative.js --preload ./tests/tooling/setup.ts ./tests/tooling/GithubUtils.test.ts + +# One test, by name +npm run test:bun -- -t 'getPullRequestNumberFromURL' +``` + +The leading `./` on a file path is required: without it Bun treats the argument as a name filter and finds +nothing, because `bunfig.toml` points bare `bun test` at `server/`. + +Neither flag is optional: + +- `--parallel` — runs each file in a worker process, and implies `--isolate`. Isolation is the load-bearing part: + several files replace `fs`, `child_process` or a shared lib with `mock.module()`, and Bun shares one module + registry (and `process.env`) across files unless each gets its own. Separate processes additionally give + `CIGitLogic` its own working directory, which it changes. +- `--preload ./scripts/stubReactNative.js` — `generateTranslations.test.ts` reaches `src/languages/en`, which pulls + in `react-native`, whose Flow syntax Bun can't parse. `bunfig.toml`'s top-level `preload` does not apply to + `bun test`, so it has to be passed here. It's the same stub `bun scripts/generateTranslations.ts` runs with. + +`--concurrent` is deliberately not used. It makes the tests *within* each file run at once, which does not help: +under `--parallel` the wall clock is set by the single longest file (`CIGitLogic`, ~52s of the ~53s total), and +that file has to stay ordered. It also breaks tests — 16 of the files here reset module-level spies in +`beforeEach`, so a concurrent sibling clears the mocks a running test is about to assert on. Measured on the full +suite: `--parallel` 52.9s and passing, `--parallel --concurrent --max-concurrency 7` 57.2s with 9 failures. + +## Why bun:test and not Jest + +`@actions/core` and `@actions/github` are ESM-only from their next majors. Jest resolves them through Babel's +CommonJS interop, so keeping these tests on Jest would mean maintaining hand-built CJS shims for every +`@actions/*` and `@octokit/*` package. Bun imports them natively. + +That is also the rule for where a new test belongs: **if its import graph reaches `@actions/*` or `@octokit/*`, +it goes here.** Everything else — including scripts that only use `scripts/utils/*` — can stay in `tests/unit/` +under Jest. + +Prefer importing the narrowest module that has what you need — `@src/CONST/LOCALES` is self-contained, whereas +`@src/types/onyx/Locale` re-exports the whole `@src/CONST` barrel and drags a large part of the app in with it. + +## Differences from the Jest tests + +Bun's `jest` object is close to Jest's but not identical. The gaps that come up here: + +| Jest | Bun equivalent | +| --- | --- | +| `jest.mock('foo')` (automock) | `mock.module('foo', factory)`, before the module under test is imported. There is no automock, so stub each export you need. | +| `jest.requireActual('foo')` | `await import('foo')` before the `mock.module()` call. | +| `jest.advanceTimersByTimeAsync(ms)` | No equivalent; alternate `await Promise.resolve()` with `jest.advanceTimersByTime(ms)`. | +| `jest.mocked(fn)` / `jest.SpiedFunction` | `Mock` from `bun:test`. | +| `asMutable(core).getInput = mock` | `jest.spyOn(core, 'getInput')` — real ESM namespace exports are read-only. | + +Because `mock.module()` is hoisting-sensitive, files that use it import the module under test with a top-level +`await import(...)` placed after the mock. + +## Type-checking + +These files are type-checked by the root `tsconfig.json` along with everything else, so they see the app's real +types. `bun:test` resolves because that config pulls in `node_modules/bun-types/test.d.ts` — the one file in +bun-types that declares the module — through `files` rather than `include`, since `exclude` covers node_modules. + +The rest of bun-types is deliberately left out: its global JSX declarations are incompatible with the app's React +types, and `generateTranslations.test.ts` reaches `src/` through the script it covers. One consequence is that +`@types/jest`'s globals are visible here too, so a missing `bun:test` import can type-check but still fail at +runtime — import every helper you use. + +`CIGitLogic.test.ts` is the exception. It uses Bun's `$` shell, which is a runtime API rather than a module +declaration, so it needs the full `@types/bun`. Those types redeclare globals the app already owns — a `jest` +namespace that shadows `@types/jest`'s generic signatures, and a `fetch` carrying `preconnect` — and adding them +to the root project produces ~1,100 errors across `tests/unit`. So that one file is excluded from the root +project and type-checked by `tests/tooling/tsconfig.json`, which mirrors what `server/tsconfig.json` does. A new +test needing Bun runtime APIs should be added to that project's `files`; one that only needs `bun:test` should +not, so it keeps seeing the app's types. diff --git a/tests/unit/ReactCompilerComplianceCheckTest.ts b/tests/tooling/ReactCompilerComplianceCheck.test.ts similarity index 98% rename from tests/unit/ReactCompilerComplianceCheckTest.ts rename to tests/tooling/ReactCompilerComplianceCheck.test.ts index 23704bda275a..f00e99968e50 100644 --- a/tests/unit/ReactCompilerComplianceCheckTest.ts +++ b/tests/tooling/ReactCompilerComplianceCheck.test.ts @@ -1,3 +1,5 @@ +import {describe, expect, it} from 'bun:test'; + import {checkReactCompilerCompliance} from '../../scripts/react-compiler-compliance-check'; describe('checkReactCompilerCompliance', () => { diff --git a/tests/unit/ArtifactsResolverTest.ts b/tests/tooling/artifactsResolver.test.ts similarity index 75% rename from tests/unit/ArtifactsResolverTest.ts rename to tests/tooling/artifactsResolver.test.ts index 02bee6f5133f..d798ce3290cb 100644 --- a/tests/unit/ArtifactsResolverTest.ts +++ b/tests/tooling/artifactsResolver.test.ts @@ -1,30 +1,34 @@ -import GithubUtils from '@github/libs/GithubUtils'; +import {afterEach, beforeEach, describe, expect, it, jest, mock} from 'bun:test'; -import resolveArtifacts, {ARTIFACT_IDS} from '@scripts/artifacts-utils/lib/artifactsResolver'; -import {getCredentials} from '@scripts/artifacts-utils/lib/githubCLI'; +import type GithubUtils from '@github/libs/GithubUtils'; -/** - * @jest-environment node - */ -import {execFileSync} from 'child_process'; +import type {getCredentials} from '@scripts/artifacts-utils/lib/githubCLI'; + +import * as childProcess from 'child_process'; import fs from 'fs'; -jest.mock('child_process'); -jest.mock('@scripts/artifacts-utils/lib/githubCLI'); -jest.mock('@github/libs/GithubUtils', () => ({ - __esModule: true, +const mockExecFileSync = jest.fn<(command: string) => string>(); +const mockGetCredentials = jest.fn(); +const mockPaginate = jest.fn<() => Promise>>(); +const mockInitGithubClient = jest.fn(); + +// Bun has no equivalent of `jest.mock(path)`'s automock, so each of these replaces the module explicitly. They must +// run before `artifactsResolver` is imported below: mock.module patches the shared module registry entry, and +// existing import bindings are live, but only if the patch happens before those bindings are first read. +// `bun test --isolate` keeps the replacements from reaching the other files in tests/tooling. +await mock.module('child_process', () => ({...childProcess, execFileSync: mockExecFileSync})); +const realGithubCLI = await import('@scripts/artifacts-utils/lib/githubCLI'); +await mock.module('@scripts/artifacts-utils/lib/githubCLI', () => ({...realGithubCLI, getCredentials: mockGetCredentials})); +await mock.module('@github/libs/GithubUtils', () => ({ default: { - initOctokitWithToken: jest.fn(), - paginate: jest.fn(), + initOctokitWithToken: mockInitGithubClient, + paginate: mockPaginate, octokit: {packages: {getAllPackageVersionsForPackageOwnedByOrg: jest.fn()}}, }, })); -const mockExecFileSync = jest.mocked(execFileSync); -const mockGetCredentials = jest.mocked(getCredentials); -const mockPaginate = jest.mocked(GithubUtils.paginate); -// eslint-disable-next-line @typescript-eslint/unbound-method -- jest.fn() mocks don't rely on `this` binding -const mockInitGithubClient = jest.mocked(GithubUtils.initOctokitWithToken); +// Must be imported after the mock.module() calls above so it picks up the mocks. +const {default: resolveArtifacts, ARTIFACT_IDS} = await import('@scripts/artifacts-utils/lib/artifactsResolver'); const NEW_DOT_ROOT = '/repo'; const LOCAL_HASH = 'abc123hash'; @@ -39,19 +43,21 @@ function fakeFetchResponse(body: string) { /** Replaces global fetch with a queue of POM responses (one per candidate lookup). */ function mockFetchBodies(bodies: string[]) { let call = 0; - global.fetch = jest.fn().mockImplementation(() => Promise.resolve(fakeFetchResponse(bodies.at(call++) ?? ''))); + // `preconnect` is part of the fetch type but nothing here calls it. + global.fetch = Object.assign( + jest.fn().mockImplementation(() => Promise.resolve(fakeFetchResponse(bodies.at(call++) ?? ''))), + {preconnect: () => {}}, + ); } /** Makes the package-versions API return the given version names. */ function mockVersions(names: string[]) { - // Faking the paginate() surface in a unit test. - // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion - (mockPaginate as unknown as jest.Mock).mockResolvedValue(names.map((name) => ({name}))); + mockPaginate.mockResolvedValue(names.map((name) => ({name}))); } /** Mocks the local patches hash and the react-native version read from package.json. */ function mockLocalRepo() { - mockExecFileSync.mockImplementation((cmd: string) => (cmd === 'bash' ? LOCAL_HASH : '')); + mockExecFileSync.mockImplementation((command: string) => (command === 'bash' ? LOCAL_HASH : '')); jest.spyOn(fs, 'readFileSync').mockReturnValue('{"dependencies":{"react-native":"0.85.3"}}'); } @@ -141,8 +147,7 @@ describe('artifactsResolver', () => { it('falls back to source build when the packages API fails', async () => { mockLocalRepo(); - // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion - (mockPaginate as unknown as jest.Mock).mockRejectedValue(new Error('403 Forbidden')); + mockPaginate.mockRejectedValue(new Error('403 Forbidden')); const result = await resolveArtifacts({platform: 'ios', packageName: 'react-hybrid', newDotRoot: NEW_DOT_ROOT, isHybrid: true}); diff --git a/tests/unit/awaitStagingDeploysTest.ts b/tests/tooling/awaitStagingDeploys.test.ts similarity index 81% rename from tests/unit/awaitStagingDeploysTest.ts rename to tests/tooling/awaitStagingDeploys.test.ts index 6363525b44a0..0f6e2b3c0aee 100644 --- a/tests/unit/awaitStagingDeploysTest.ts +++ b/tests/tooling/awaitStagingDeploys.test.ts @@ -1,14 +1,11 @@ +import type {Mock} from 'bun:test'; +import {beforeAll, beforeEach, describe, expect, jest, test} from 'bun:test'; + import run from '@github/actions/javascript/awaitStagingDeploys/awaitStagingDeploys'; -import type CONST from '@github/libs/CONST'; -import type {InternalOctokit} from '@github/libs/GithubUtils'; +import CONST from '@github/libs/CONST'; import GithubUtils from '@github/libs/GithubUtils'; -import asMutable from '@src/types/utils/asMutable'; - /* eslint-disable @typescript-eslint/naming-convention */ -/** - * @jest-environment node - */ import * as core from '@actions/core'; type Workflow = { @@ -30,7 +27,7 @@ type MockListResponse = { }; }; -type MockedFunctionListResponse = jest.MockedFunction<() => Promise>; +type MockedFunctionListResponse = Mock<() => Promise>; const consoleSpy = jest.spyOn(console, 'log'); const mockGetInput = jest.fn(); @@ -59,26 +56,21 @@ const mockListWorkflowRuns = jest.fn().mockImplementation((args: Workflow) => { return defaultReturn; }); -jest.mock('@github/libs/CONST', () => ({ - ...jest.requireActual('@github/libs/CONST'), - POLL_RATE: TEST_POLL_RATE, -})); +// CONST's default export is a plain mutable object shared by reference with every other importer (unlike named +// exports on a module namespace, which are read-only live bindings), so it can be overridden in place rather than +// needing mock.module. Lower the poll rate to speed up the test. +(CONST as {POLL_RATE: number}).POLL_RATE = TEST_POLL_RATE; beforeAll(() => { - // Mock core module - asMutable(core).getInput = mockGetInput; - - // Mock octokit module - const mockOctokit = { - rest: { - actions: { - ...(GithubUtils.internalOctokit as unknown as typeof GithubUtils.octokit.actions), - listWorkflowRuns: mockListWorkflowRuns as unknown as typeof GithubUtils.octokit.actions.listWorkflowRuns, - }, - }, - }; - - GithubUtils.internalOctokit = mockOctokit as InternalOctokit; + // Mock core module. Real ESM module namespace exports are read-only live bindings, so `core.getInput` can't be + // reassigned directly (unlike Jest's Babel-transpiled CJS interop); spy on it instead. + jest.spyOn(core, 'getInput').mockImplementation(mockGetInput); + + // Octokit endpoint methods carry `defaults`/`endpoint` statics that a bare mock doesn't, so the real ones are + // copied onto the stub rather than asserted away. + GithubUtils.initOctokitWithToken('fake_token'); + const {endpoint, defaults} = GithubUtils.octokit.actions.listWorkflowRuns; + jest.spyOn(GithubUtils.octokit.actions, 'listWorkflowRuns').mockImplementation(Object.assign(mockListWorkflowRuns, {endpoint, defaults})); }); beforeEach(() => { diff --git a/tests/unit/BumpVersionTest.ts b/tests/tooling/bumpVersion.test.ts similarity index 75% rename from tests/unit/BumpVersionTest.ts rename to tests/tooling/bumpVersion.test.ts index b218135a6502..13326d21dc1b 100644 --- a/tests/unit/BumpVersionTest.ts +++ b/tests/tooling/bumpVersion.test.ts @@ -1,8 +1,17 @@ -import fs from 'fs'; -import {vol} from 'memfs'; +import {beforeEach, describe, expect, mock, test} from 'bun:test'; + +import {fs as memfsFs, vol} from 'memfs'; import path from 'path'; -import {generateAndroidVersionCode, updateAndroid} from '../../scripts/bumpVersion'; +// Must run before `bumpVersion` (which imports `fs` and `fs/promises` internally) is imported below: mock.module +// patches the shared module registry entry, and existing import bindings are live, but only if the patch happens +// before those bindings are first read. `bun test --isolate` gives each test file its own registry, so this does +// not leak into the other files in tests/tooling. +await mock.module('fs', () => ({...memfsFs, default: memfsFs})); +await mock.module('fs/promises', () => ({...memfsFs.promises, default: memfsFs.promises})); + +// Must be imported after the mock.module() calls above so it picks up the mocked filesystem. +const {generateAndroidVersionCode, updateAndroid} = await import('../../scripts/bumpVersion'); const BUILD_GRADLE_PATH = path.resolve(__dirname, '../../android/app/build.gradle'); const ANDROID_MANIFEST_PATH = path.resolve(__dirname, '../../Mobile-Expensify/Android/AndroidManifest.xml'); @@ -23,9 +32,6 @@ const mockAndroidManifest = ` xmlns:tools="http://schemas.android.com/tools"> `; -jest.mock('fs'); -jest.mock('fs/promises'); - beforeEach(() => { // Clear the mocked filesystem vol.reset(); @@ -108,9 +114,9 @@ describe('BumpVersion', () => { ], ])('updateAndroid("%s")', async (versionName, expectedBuildGradle, expectedAndroidManifest) => { await updateAndroid(versionName); - const buildGradle = fs.readFileSync(BUILD_GRADLE_PATH, {encoding: 'utf8'}); + const buildGradle = memfsFs.readFileSync(BUILD_GRADLE_PATH, {encoding: 'utf8'}); expect(buildGradle).toBe(expectedBuildGradle); - const androidManifest = fs.readFileSync(ANDROID_MANIFEST_PATH, {encoding: 'utf8'}); + const androidManifest = memfsFs.readFileSync(ANDROID_MANIFEST_PATH, {encoding: 'utf8'}); expect(androidManifest).toBe(expectedAndroidManifest); }); }); diff --git a/tests/unit/checkDeployBlockersTest.ts b/tests/tooling/checkDeployBlockers.test.ts similarity index 90% rename from tests/unit/checkDeployBlockersTest.ts rename to tests/tooling/checkDeployBlockers.test.ts index 168b97e51295..6517d482b388 100644 --- a/tests/unit/checkDeployBlockersTest.ts +++ b/tests/tooling/checkDeployBlockers.test.ts @@ -1,12 +1,10 @@ +import type {Mock} from 'bun:test'; +import {afterAll, afterEach, beforeAll, beforeEach, describe, expect, jest, test} from 'bun:test'; + import run from '@github/actions/javascript/checkDeployBlockers/checkDeployBlockers'; import type {InternalOctokit} from '@github/libs/GithubUtils'; import GithubUtils from '@github/libs/GithubUtils'; -import asMutable from '@src/types/utils/asMutable'; - -/** - * @jest-environment node - */ import * as core from '@actions/core'; import createMock from '../utils/createMock'; @@ -30,13 +28,14 @@ const mockGetInput = jest.fn().mockImplementation((arg: string): string | number }); const mockSetOutput = jest.fn(); -let mockGetIssue: jest.SpiedFunction; -let mockListComments: jest.SpiedFunction; +let mockGetIssue: Mock; +let mockListComments: Mock; beforeAll(() => { - // Mock core module - asMutable(core).getInput = mockGetInput; - asMutable(core).setOutput = mockSetOutput; + // Mock core module. Real ESM module namespace exports are read-only live bindings, so `core.getInput` can't be + // reassigned directly (unlike Jest's Babel-transpiled CJS interop); spy on it instead. + jest.spyOn(core, 'getInput').mockImplementation(mockGetInput); + jest.spyOn(core, 'setOutput').mockImplementation(mockSetOutput); GithubUtils.initOctokitWithToken('fake_token'); if (!GithubUtils.internalOctokit) { diff --git a/tests/unit/createOrUpdateDeployChecklistTest.ts b/tests/tooling/createOrUpdateDeployChecklist.test.ts similarity index 89% rename from tests/unit/createOrUpdateDeployChecklistTest.ts rename to tests/tooling/createOrUpdateDeployChecklist.test.ts index 988ca408a5dd..cd9793dd7896 100644 --- a/tests/unit/createOrUpdateDeployChecklistTest.ts +++ b/tests/tooling/createOrUpdateDeployChecklist.test.ts @@ -1,37 +1,32 @@ +import type {Mock} from 'bun:test'; +import {afterAll, afterEach, beforeAll, describe, expect, jest, mock, test} from 'bun:test'; + import CONST from '@github/libs/CONST'; import * as DeployChecklistUtils from '@github/libs/DeployChecklistUtils'; import type {InternalOctokit, ListForRepoMethod} from '@github/libs/GithubUtils'; import GithubUtils from '@github/libs/GithubUtils'; import GitUtils from '@github/libs/GitUtils'; -import run from '@scripts/createOrUpdateDeployChecklist'; - import * as core from '@actions/core'; import * as fns from 'date-fns'; -import {vol} from 'memfs'; +import {fs as memfsFs, vol} from 'memfs'; import path from 'path'; import createMock from '../utils/createMock'; -/** - * @jest-environment node - */ - /* eslint-disable @typescript-eslint/naming-convention */ -// Mock fs -jest.mock('fs'); +// Must run before `createOrUpdateDeployChecklist` (which imports `fs` internally) is imported below: mock.module +// patches the shared module registry entry, and existing import bindings are live, but only if the patch happens +// before those bindings are first read. +await mock.module('fs', () => ({...memfsFs, default: memfsFs})); + +// Must be imported after the mock.module() call above so it picks up the mock. +const {default: run} = await import('@scripts/createOrUpdateDeployChecklist'); -// Mock @actions/core for input handling and logging in tests -jest.mock('@actions/core', () => ({ - getInput: jest.fn(), - info: jest.fn(), - startGroup: jest.fn(), - endGroup: jest.fn(), - setFailed: jest.fn(), -})); +type IssuesCreateResponse = Awaited>['data']; -const mockGetInput = jest.mocked(core.getInput); +const mockGetInput = jest.fn(); type ListForRepoParameters = Parameters; type ListForRepoResponse = Awaited>; @@ -44,20 +39,27 @@ type PullsListResponse = Awaited; -let mockUpdateIssue: jest.SpiedFunction; -let mockListIssues: jest.MockedFunction; -const mockGetMergedPRsDeployedBetween = jest.fn, Parameters>(); +let mockCreateIssue: Mock; +let mockUpdateIssue: Mock; +let mockListIssues: Mock; +let listForRepoStatics: Pick; +const mockGetMergedPRsDeployedBetween = jest.fn(); const mockGetWorkflowRunURLForCommit = jest.fn().mockResolvedValue(undefined); beforeAll(() => { + // The action stamps the checklist title with today's date and the assertions below re-derive it, so pin the + // clock: otherwise the two reads can straddle local midnight. Jest froze Date globally via fakeTimers. + jest.setSystemTime(new Date('2026-02-03T12:00:00Z')); + GithubUtils.initOctokitWithToken('fake_token'); const mockOctokit = GithubUtils.internalOctokit; if (!mockOctokit) { throw new Error('GithubUtils failed to initialize Octokit.'); } - mockCreateIssue = jest.spyOn(mockOctokit.rest.issues, 'create').mockImplementation((...args: CreateIssueParameters): Promise => { + // Octokit endpoint methods carry `defaults`/`endpoint` statics. A Bun mock doesn't, and `paginate` reads them + // off the method, so the real ones are copied back onto each mock and stub below. + const createIssue = (...args: CreateIssueParameters): Promise => { const [arg] = args; if (!arg) { throw new Error('GithubUtils issues.create mock requires request parameters.'); @@ -69,8 +71,8 @@ beforeAll(() => { }, }), ); - }); - mockUpdateIssue = jest.spyOn(mockOctokit.rest.issues, 'update').mockImplementation((...args: UpdateIssueParameters): Promise => { + }; + const updateIssue = (...args: UpdateIssueParameters): Promise => { const [arg] = args; if (!arg) { throw new Error('GithubUtils issues.update mock requires request parameters.'); @@ -82,22 +84,28 @@ beforeAll(() => { }, }), ); - }); - const listForRepoEndpoint = mockOctokit.rest.issues.listForRepo.endpoint; - const listForRepoDefaults = mockOctokit.rest.issues.listForRepo.defaults; - const pullsListEndpoint = mockOctokit.rest.pulls.list.endpoint; - const pullsListDefaults = mockOctokit.rest.pulls.list.defaults; - jest.spyOn(mockOctokit.rest.issues, 'listForRepo'); - mockListIssues = jest.mocked(mockOctokit.rest.issues.listForRepo); - mockListIssues.endpoint = listForRepoEndpoint; - mockListIssues.defaults = listForRepoDefaults; - jest.spyOn(mockOctokit.rest.pulls, 'list'); - const mockListPullRequests = jest.mocked(mockOctokit.rest.pulls.list, {shallow: true}); + }; + const {endpoint: createEndpoint, defaults: createDefaults} = mockOctokit.rest.issues.create; + const {endpoint: updateEndpoint, defaults: updateDefaults} = mockOctokit.rest.issues.update; + mockCreateIssue = jest.spyOn(mockOctokit.rest.issues, 'create').mockImplementation(Object.assign(createIssue, {endpoint: createEndpoint, defaults: createDefaults})); + mockUpdateIssue = jest.spyOn(mockOctokit.rest.issues, 'update').mockImplementation(Object.assign(updateIssue, {endpoint: updateEndpoint, defaults: updateDefaults})); + const {endpoint: listForRepoEndpoint, defaults: listForRepoDefaults} = mockOctokit.rest.issues.listForRepo; + listForRepoStatics = {endpoint: listForRepoEndpoint, defaults: listForRepoDefaults}; + const {endpoint: pullsListEndpoint, defaults: pullsListDefaults} = mockOctokit.rest.pulls.list; + mockListIssues = Object.assign(jest.spyOn(mockOctokit.rest.issues, 'listForRepo'), {endpoint: listForRepoEndpoint, defaults: listForRepoDefaults}); + const mockListPullRequests = Object.assign(jest.spyOn(mockOctokit.rest.pulls, 'list'), {endpoint: pullsListEndpoint, defaults: pullsListDefaults}); mockListPullRequests.mockResolvedValue(createMock({data: [], headers: {}})); - mockListPullRequests.endpoint = pullsListEndpoint; - mockListPullRequests.defaults = pullsListDefaults; GithubUtils.internalOctokit = mockOctokit; + // Mock @actions/core for input handling and logging in tests. Real ESM module namespace exports are read-only + // live bindings, so these can't be reassigned directly (unlike Jest's Babel-transpiled CJS interop); spy on + // them instead. + jest.spyOn(core, 'getInput').mockImplementation(mockGetInput); + jest.spyOn(core, 'info').mockImplementation(() => {}); + jest.spyOn(core, 'startGroup').mockImplementation(() => {}); + jest.spyOn(core, 'endGroup').mockImplementation(() => {}); + jest.spyOn(core, 'setFailed').mockImplementation(() => {}); + // Mock GitUtils GitUtils.getMergedPRsDeployedBetween = mockGetMergedPRsDeployedBetween; GithubUtils.getWorkflowRunURLForCommit = mockGetWorkflowRunURLForCommit; @@ -120,10 +128,11 @@ afterEach(() => { afterAll(() => { jest.clearAllMocks(); + jest.useRealTimers(); }); function mockDeployChecklistIssuesByLabel(responseByLabel: Partial>[0]['data']>>) { - mockListIssues.mockImplementation((...parameters: ListForRepoParameters): Promise => { + const listForRepo = (...parameters: ListForRepoParameters): Promise => { const receivedParameters = parameters[0]; if (!receivedParameters) { throw new Error('GithubUtils issues.listForRepo mock requires request parameters.'); @@ -140,7 +149,8 @@ function mockDeployChecklistIssuesByLabel(responseByLabel: Partial({data, headers: {}})); - }); + }; + mockListIssues.mockImplementation(Object.assign(listForRepo, listForRepoStatics)); } const LABELS = { @@ -330,9 +340,11 @@ describe('createOrUpdateDeployChecklist', () => { `${lineBreak}${openCheckbox}${ghVerification}` + `${lineBreak}${ccApplauseLeads}`, }); - expect(result).toStrictEqual({ - html_url: `https://github.com/${process.env.GITHUB_REPOSITORY}/issues/29`, - }); + expect(result).toStrictEqual( + createMock({ + html_url: `https://github.com/${process.env.GITHUB_REPOSITORY}/issues/29`, + }), + ); }); test('creates new issue when there are no Mobile-Expensify PRs', async () => { @@ -376,9 +388,11 @@ describe('createOrUpdateDeployChecklist', () => { `${lineBreak}${openCheckbox}${ghVerification}` + `${lineBreak}${ccApplauseLeads}`, }); - expect(result).toStrictEqual({ - html_url: `https://github.com/${process.env.GITHUB_REPOSITORY}/issues/29`, - }); + expect(result).toStrictEqual( + createMock({ + html_url: `https://github.com/${process.env.GITHUB_REPOSITORY}/issues/29`, + }), + ); }); describe('updates existing issue when there is one open', () => { @@ -495,9 +509,11 @@ describe('createOrUpdateDeployChecklist', () => { `${lineBreak}${openCheckbox}${ghVerification}` + `${lineBreak}${ccApplauseLeads}`, }); - expect(result).toStrictEqual({ - html_url: `https://github.com/${process.env.GITHUB_REPOSITORY}/issues/${openDeployChecklistBefore.number}`, - }); + expect(result).toStrictEqual( + createMock({ + html_url: `https://github.com/${process.env.GITHUB_REPOSITORY}/issues/${openDeployChecklistBefore.number}`, + }), + ); }); test('without NPM_VERSION input, just a new deploy blocker', async () => { @@ -563,9 +579,11 @@ describe('createOrUpdateDeployChecklist', () => { `${lineBreak}${closedCheckbox}${ghVerification}` + `${lineBreak}${ccApplauseLeads}`, }); - expect(result).toStrictEqual({ - html_url: `https://github.com/${process.env.GITHUB_REPOSITORY}/issues/${openDeployChecklistBefore.number}`, - }); + expect(result).toStrictEqual( + createMock({ + html_url: `https://github.com/${process.env.GITHUB_REPOSITORY}/issues/${openDeployChecklistBefore.number}`, + }), + ); }); test('without Mobile-Expensify PRs, just app PRs and deploy blockers', async () => { @@ -612,9 +630,11 @@ describe('createOrUpdateDeployChecklist', () => { `${lineBreak}${closedCheckbox}${ghVerification}` + `${lineBreak}${ccApplauseLeads}`, }); - expect(result).toStrictEqual({ - html_url: `https://github.com/${process.env.GITHUB_REPOSITORY}/issues/${openDeployChecklistBefore.number}`, - }); + expect(result).toStrictEqual( + createMock({ + html_url: `https://github.com/${process.env.GITHUB_REPOSITORY}/issues/${openDeployChecklistBefore.number}`, + }), + ); }); }); @@ -673,9 +693,11 @@ describe('createOrUpdateDeployChecklist', () => { }); const result = await run(); - expect(result).toStrictEqual({ - html_url: `https://github.com/${process.env.GITHUB_REPOSITORY}/issues/29`, - }); + expect(result).toStrictEqual( + createMock({ + html_url: `https://github.com/${process.env.GITHUB_REPOSITORY}/issues/29`, + }), + ); const createCall = mockCreateIssue.mock.lastCall; if (!createCall || !createCall[0]) { throw new Error('Expected issues.create to receive a request payload.'); @@ -741,9 +763,11 @@ describe('createOrUpdateDeployChecklist', () => { }); const result = await run(); - expect(result).toStrictEqual({ - html_url: `https://github.com/${process.env.GITHUB_REPOSITORY}/issues/29`, - }); + expect(result).toStrictEqual( + createMock({ + html_url: `https://github.com/${process.env.GITHUB_REPOSITORY}/issues/29`, + }), + ); const createCall = mockCreateIssue.mock.lastCall; if (!createCall || !createCall[0]) { throw new Error('Expected issues.create to receive a request payload.'); @@ -812,9 +836,11 @@ describe('createOrUpdateDeployChecklist', () => { }); const result = await run(); - expect(result).toStrictEqual({ - html_url: `https://github.com/${process.env.GITHUB_REPOSITORY}/issues/29`, - }); + expect(result).toStrictEqual( + createMock({ + html_url: `https://github.com/${process.env.GITHUB_REPOSITORY}/issues/29`, + }), + ); const createCall = mockCreateIssue.mock.lastCall; if (!createCall || !createCall[0]) { throw new Error('Expected issues.create to receive a request payload.'); @@ -862,9 +888,11 @@ describe('createOrUpdateDeployChecklist', () => { }); const result = await run(); - expect(result).toStrictEqual({ - html_url: `https://github.com/${process.env.GITHUB_REPOSITORY}/issues/29`, - }); + expect(result).toStrictEqual( + createMock({ + html_url: `https://github.com/${process.env.GITHUB_REPOSITORY}/issues/29`, + }), + ); const createCall = mockCreateIssue.mock.lastCall; if (!createCall || !createCall[0]) { throw new Error('Expected issues.create to receive a request payload.'); @@ -938,9 +966,11 @@ describe('createOrUpdateDeployChecklist', () => { }); const result = await run(); - expect(result).toStrictEqual({ - html_url: `https://github.com/${process.env.GITHUB_REPOSITORY}/issues/29`, - }); + expect(result).toStrictEqual( + createMock({ + html_url: `https://github.com/${process.env.GITHUB_REPOSITORY}/issues/29`, + }), + ); const updateCall = mockUpdateIssue.mock.lastCall; if (!updateCall || !updateCall[0]) { throw new Error('Expected issues.update to receive a request payload.'); @@ -1016,9 +1046,11 @@ describe('createOrUpdateDeployChecklist', () => { }); const result = await run(); - expect(result).toStrictEqual({ - html_url: `https://github.com/${process.env.GITHUB_REPOSITORY}/issues/29`, - }); + expect(result).toStrictEqual( + createMock({ + html_url: `https://github.com/${process.env.GITHUB_REPOSITORY}/issues/29`, + }), + ); const createCall = mockCreateIssue.mock.lastCall; if (!createCall || !createCall[0]) { throw new Error('Expected issues.create to receive a request payload.'); @@ -1078,9 +1110,11 @@ describe('createOrUpdateDeployChecklist', () => { }); const result = await run(); - expect(result).toStrictEqual({ - html_url: `https://github.com/${process.env.GITHUB_REPOSITORY}/issues/29`, - }); + expect(result).toStrictEqual( + createMock({ + html_url: `https://github.com/${process.env.GITHUB_REPOSITORY}/issues/29`, + }), + ); const createCall = mockCreateIssue.mock.lastCall; if (!createCall || !createCall[0]) { throw new Error('Expected issues.create to receive a request payload.'); diff --git a/tests/unit/createRetestRequestForCPTest.ts b/tests/tooling/createRetestRequestForCP.test.ts similarity index 98% rename from tests/unit/createRetestRequestForCPTest.ts rename to tests/tooling/createRetestRequestForCP.test.ts index dac34082b397..77f9247e1370 100644 --- a/tests/unit/createRetestRequestForCPTest.ts +++ b/tests/tooling/createRetestRequestForCP.test.ts @@ -1,12 +1,10 @@ +import {describe, expect, it} from 'bun:test'; + import CONST from '@github/libs/CONST'; import {buildRetestPayload, getCherryPickSourceSHAs, getLinkedIssueNumbers, getRetestMarker} from '@scripts/createRetestRequestForCP'; import type {RetestHit} from '@scripts/createRetestRequestForCP'; -/** - * @jest-environment node - */ - describe('createRetestRequestForCP', () => { describe('getCherryPickSourceSHAs', () => { it('pulls the source SHA out of a cherry-pick trailer', () => { diff --git a/tests/actions/detectReactComponent.test.ts b/tests/tooling/detectReactComponent.test.ts similarity index 98% rename from tests/actions/detectReactComponent.test.ts rename to tests/tooling/detectReactComponent.test.ts index 355aab843482..5df93baad9b9 100644 --- a/tests/actions/detectReactComponent.test.ts +++ b/tests/tooling/detectReactComponent.test.ts @@ -1,3 +1,5 @@ +import {describe, expect, it} from 'bun:test'; + import {detectReactComponent} from '../../.github/actions/javascript/authorChecklist/categories/newComponentCategory'; describe('detectReactComponent test', () => { diff --git a/tests/unit/FailureNotifierTest.ts b/tests/tooling/failureNotifier.test.ts similarity index 98% rename from tests/unit/FailureNotifierTest.ts rename to tests/tooling/failureNotifier.test.ts index 0d252d9b7f74..b43800d92bf7 100644 --- a/tests/unit/FailureNotifierTest.ts +++ b/tests/tooling/failureNotifier.test.ts @@ -1,6 +1,5 @@ -/** - * @jest-environment node - */ +import {describe, expect, it} from 'bun:test'; + /* eslint-disable @typescript-eslint/naming-convention -- matching GitHub API response field names */ import {getMergedPR} from '@github/actions/javascript/failureNotifier/failureNotifier'; import type {PullRequest} from '@github/actions/javascript/failureNotifier/failureNotifier'; diff --git a/tests/unit/generateTranslationsTest.ts b/tests/tooling/generateTranslations.test.ts similarity index 97% rename from tests/unit/generateTranslationsTest.ts rename to tests/tooling/generateTranslations.test.ts index 2a95c69b4d8c..900cc03668f4 100644 --- a/tests/unit/generateTranslationsTest.ts +++ b/tests/tooling/generateTranslations.test.ts @@ -1,41 +1,43 @@ -import generateTranslations, {GENERATED_FILE_PREFIX} from '@scripts/generateTranslations'; +import {afterAll, afterEach, beforeEach, describe, expect, it, jest, mock} from 'bun:test'; +import type {Mock} from 'bun:test'; + import Git from '@scripts/utils/Git'; import DummyTranslator from '@scripts/utils/Translator/DummyTranslator'; import Translator from '@scripts/utils/Translator/Translator'; -/** - * @jest-environment node - */ import {Str} from 'expensify-common'; import fs from 'fs'; import os from 'os'; import path from 'path'; -let processExitSpy: jest.SpyInstance; -let consoleErrorSpy: jest.SpyInstance; - -// eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-assignment -let mockEn: any = jest.requireActual('@src/languages/en'); -jest.mock('@src/languages/en', () => ({ - __esModule: true, - // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment - get default() { - // eslint-disable-next-line @typescript-eslint/no-unsafe-return - return mockEn; - }, -})); -jest.mock('openai'); -jest.mock('@scripts/utils/Git'); - -// Mock Git methods -const mockIsValidRef = jest.fn, Parameters>(); -const mockDiff = jest.fn, Parameters>(); -const mockShow = jest.fn, Parameters>(); - -// Apply mocks to Git using jest.spyOn (ignore type errors for now) -jest.spyOn(Git, 'isValidRef').mockImplementation(mockIsValidRef); -jest.spyOn(Git, 'diff').mockImplementation(mockDiff); -jest.spyOn(Git, 'show').mockImplementation(mockShow); +let processExitSpy: Mock; +let consoleErrorSpy: Mock; + +/** + * Swaps the `en` strings the script reads as its source of truth. Re-mocking per call rather than reading a mutable + * variable through a getter: `generateTranslations` imports `en` as a default binding, which Bun resolves once at + * link time, so a getter would only ever be read for the first test. + * + * The first call has to happen before `generateTranslations` is imported below, because mock.module patches the + * shared module registry entry and existing bindings only pick that up if the patch lands first. `bun test + * --isolate` keeps the replacement from reaching the other files in tests/tooling. + */ +async function setMockEn(strings: unknown) { + await mock.module('@src/languages/en', () => ({default: strings})); +} + +await setMockEn((await import('@src/languages/en')).default); + +// Must be imported after setMockEn above so it reads the mocked `en`. +const {default: generateTranslations, GENERATED_FILE_PREFIX} = await import('@scripts/generateTranslations'); + +// `Git` is a class of static methods, so the three the script calls can be spied on directly. Its remaining +// methods are left real: the script never reaches them under --dry-run, and stubbing them would only hide it if +// that changed. `openai` needs no stub either - the script builds a DummyTranslator under --dry-run and never +// constructs the OpenAI client. +const mockIsValidRef = jest.spyOn(Git, 'isValidRef'); +const mockDiff = jest.spyOn(Git, 'diff'); +const mockShow = jest.spyOn(Git, 'show'); let tempDir: string; let LANGUAGES_DIR: string; @@ -640,7 +642,7 @@ describe('generateTranslations', () => { network: 'Network error', }, }; - mockEn = strings; + await setMockEn(strings); fs.writeFileSync( EN_PATH, @@ -716,7 +718,7 @@ describe('generateTranslations', () => { }, }; - mockEn = strings; + await setMockEn(strings); fs.writeFileSync( EN_PATH, @@ -786,7 +788,7 @@ describe('generateTranslations', () => { save: 'Save', }, }; - mockEn = strings; + await setMockEn(strings); fs.writeFileSync( EN_PATH, @@ -842,7 +844,7 @@ describe('generateTranslations', () => { save: 'Save', }, }; - mockEn = strings; + await setMockEn(strings); fs.writeFileSync( EN_PATH, @@ -894,7 +896,7 @@ describe('generateTranslations', () => { generic: 'An error occurred', }, }; - mockEn = strings; + await setMockEn(strings); fs.writeFileSync( EN_PATH, @@ -960,7 +962,7 @@ describe('generateTranslations', () => { }, simpleTemplate: (name: string) => `Welcome ${name} to our app`, }; - mockEn = strings; + await setMockEn(strings); // Create English source file fs.writeFileSync( @@ -1577,7 +1579,7 @@ describe('generateTranslations', () => { }, }, }; - mockEn = strings; + await setMockEn(strings); fs.writeFileSync( EN_PATH, @@ -1645,7 +1647,7 @@ describe('generateTranslations', () => { }, }, }; - mockEn = strings; + await setMockEn(strings); fs.writeFileSync( EN_PATH, @@ -1860,7 +1862,7 @@ describe('generateTranslations', () => { pin: 'Pin', alsoUnchanged: 'Also unchanged', }; - mockEn = strings; + await setMockEn(strings); // Create English source without context annotation fs.writeFileSync( @@ -2270,7 +2272,7 @@ describe('generateTranslations', () => { }); describe('error summary', () => { - let consoleLogSpy: jest.SpyInstance; + let consoleLogSpy: Mock; beforeEach(() => { consoleLogSpy = jest.spyOn(console, 'log'); diff --git a/tests/unit/getPullRequestIncrementalChangesTest.ts b/tests/tooling/getPullRequestIncrementalChanges.test.ts similarity index 88% rename from tests/unit/getPullRequestIncrementalChangesTest.ts rename to tests/tooling/getPullRequestIncrementalChanges.test.ts index 0d06edf57151..d8acf5a6c04d 100644 --- a/tests/unit/getPullRequestIncrementalChangesTest.ts +++ b/tests/tooling/getPullRequestIncrementalChanges.test.ts @@ -1,14 +1,12 @@ +import type {Mock} from 'bun:test'; +import {beforeAll, beforeEach, describe, expect, it, jest} from 'bun:test'; + import run from '@github/actions/javascript/getPullRequestIncrementalChanges/getPullRequestIncrementalChanges'; import GitHubUtils from '@github/libs/GithubUtils'; import type {InternalOctokit} from '@github/libs/GithubUtils'; import Git from '@scripts/utils/Git'; -import type {Writable} from 'type-fest'; - -/** - * @jest-environment node - */ import * as core from '@actions/core'; import {context} from '@actions/github'; @@ -19,20 +17,22 @@ type ListFilesResponse = Awaited>; type PaginateMethod = InternalOctokit['paginate']; let internalOctokit: InternalOctokit; -let paginateSpy: jest.SpiedFunction; - -// Mock all dependencies -jest.mock('@actions/core'); -jest.mock('@actions/github'); -jest.mock('@scripts/utils/Git'); - -const mockSetOutput = jest.mocked(core.setOutput); -const mockGetInput = jest.fn(); - -// Mock @actions/core getInput -(core as Writable).getInput = mockGetInput; - -// Mock Git methods +let paginateSpy: Mock; + +const mockGetInput = jest.fn(); + +// Bun has no equivalent of `jest.mock(path)`'s automock, so stub the @actions/core functions this action calls +// explicitly. `@actions/github`'s `context` needs no stub: it is a plain mutable object, and beforeEach below +// overwrites every field this action reads, so whatever the real constructor loaded from the environment is +// irrelevant. +jest.spyOn(core, 'getInput').mockImplementation(mockGetInput); +const mockSetOutput = jest.spyOn(core, 'setOutput').mockImplementation(() => {}); +jest.spyOn(core, 'warning').mockImplementation(() => {}); +jest.spyOn(core, 'startGroup').mockImplementation(() => {}); +jest.spyOn(core, 'endGroup').mockImplementation(() => {}); +jest.spyOn(core, 'setFailed').mockImplementation(() => {}); + +// Mock Git methods. `Git`'s default export is a plain mutable object, so its methods can be overridden in place. const mockGitEnsureRef = jest.fn(); const mockGitDiff = jest.fn(); const mockGitParseDiff = jest.fn(); @@ -42,7 +42,7 @@ Git.diff = mockGitDiff; Git.parseDiff = mockGitParseDiff; // Mock GitHubUtils methods -const mockGetPullRequestDiff = jest.fn, Parameters>(); +const mockGetPullRequestDiff = jest.fn(); beforeAll(() => { GitHubUtils.initOctokitWithToken('fake_token'); @@ -71,7 +71,7 @@ describe('getPullRequestIncrementalChanges', () => { }; // Default mocks - mockGetInput.mockReturnValue(null); + mockGetInput.mockReturnValue(''); mockGitEnsureRef.mockResolvedValue(undefined); mockGetPullRequestDiff.mockReset(); paginateSpy.mockReset(); @@ -251,7 +251,7 @@ describe('getPullRequestIncrementalChanges', () => { if (inputName === 'PULL_REQUEST_NUMBER') { return '456'; } - return null; + return ''; }); // Mock paginate to return PR files diff --git a/tests/unit/isAuthorizedContributorTest.ts b/tests/tooling/isAuthorizedContributor.test.ts similarity index 88% rename from tests/unit/isAuthorizedContributorTest.ts rename to tests/tooling/isAuthorizedContributor.test.ts index 8e5444f078cf..0f996ef2aaa1 100644 --- a/tests/unit/isAuthorizedContributorTest.ts +++ b/tests/tooling/isAuthorizedContributor.test.ts @@ -1,6 +1,6 @@ -/** - * @jest-environment node - */ +import type {Mock} from 'bun:test'; +import {afterEach, beforeEach, describe, expect, jest, test} from 'bun:test'; + import {RequestError} from '@octokit/request-error'; import {isAuthorizedContributor, isContributorPlusMember, isInternalExpensifyEngineer} from '../../.github/actions/javascript/isAuthorizedContributor/isAuthorizedContributor'; @@ -25,9 +25,11 @@ type MembershipResponse = Awaited>; type PullResponse = Awaited>; type IssueResponse = Awaited>; -let mockGetMembershipForUserInOrg: jest.SpiedFunction; -let mockPullsGet: jest.SpiedFunction; -let mockIssuesGet: jest.SpiedFunction; +// Narrowed to the call signature: octokit's methods also carry `defaults`/`endpoint` statics, which a mock +// implementation has no way to supply, and this test only ever calls the endpoint. +let mockGetMembershipForUserInOrg: Mock<(...args: Parameters) => ReturnType>; +let mockPullsGet: Mock; +let mockIssuesGet: Mock; beforeEach(() => { jest.clearAllMocks(); @@ -38,8 +40,9 @@ beforeEach(() => { mockPullsGet = jest.spyOn(mockOctokit.pulls, 'get'); mockIssuesGet = jest.spyOn(mockOctokit.issues, 'get'); + // `octokit` is a getter over `internalOctokit.rest`, already populated by initOctokitWithToken above, so it + // resolves to mockOctokit without being stubbed. Bun's spyOn cannot wrap accessor properties in any case. jest.spyOn(GithubUtils, 'initOctokitWithToken').mockImplementation(() => {}); - jest.spyOn(GithubUtils, 'octokit', 'get').mockReturnValue(mockOctokit); }); afterEach(() => { diff --git a/tests/unit/isDeployChecklistLockedTest.ts b/tests/tooling/isDeployChecklistLocked.test.ts similarity index 82% rename from tests/unit/isDeployChecklistLockedTest.ts rename to tests/tooling/isDeployChecklistLocked.test.ts index afed2a34e4fe..781e545116f1 100644 --- a/tests/unit/isDeployChecklistLockedTest.ts +++ b/tests/tooling/isDeployChecklistLocked.test.ts @@ -1,22 +1,23 @@ -/** - * @jest-environment node - */ +import {afterAll, afterEach, beforeAll, beforeEach, describe, expect, jest, mock, test} from 'bun:test'; + import * as core from '@actions/core'; -import run from '../../.github/actions/javascript/isDeployChecklistLocked/isDeployChecklistLocked'; import CONST from '../../.github/libs/CONST'; import * as DeployChecklistUtils from '../../.github/libs/DeployChecklistUtils'; import createMock from '../utils/createMock'; -jest.mock('../../.github/libs/DeployChecklistUtils', () => { - const actual = jest.requireActual('../../.github/libs/DeployChecklistUtils'); - return { - ...actual, - getDeployChecklist: jest.fn(), - }; -}); +const mockGetDeployChecklist = jest.fn(); + +// Must run before `isDeployChecklistLocked` (which imports DeployChecklistUtils internally) is imported below: +// mock.module patches the shared module registry entry, and existing named-import bindings to it are live, but +// only if the patch happens before those bindings are first read. +await mock.module('../../.github/libs/DeployChecklistUtils', () => ({ + ...DeployChecklistUtils, + getDeployChecklist: mockGetDeployChecklist, +})); -const mockGetDeployChecklist = jest.mocked(DeployChecklistUtils.getDeployChecklist); +// Must be imported after the mock.module() call above so it picks up the mock. +const {default: run} = await import('../../.github/actions/javascript/isDeployChecklistLocked/isDeployChecklistLocked'); beforeAll(() => { process.env.INPUT_GITHUB_TOKEN = 'fake_token'; @@ -85,7 +86,7 @@ describe('isDeployChecklistLockedTest', () => { const setFailedMock = jest.spyOn(core, 'setFailed').mockImplementation(() => {}); return run().then(() => { expect(setFailedMock).toHaveBeenCalledTimes(1); - expect(setFailedMock.mock.calls.at(0)?.at(0)).toEqual(expect.stringContaining('Could not resolve deploy checklist')); + expect(setFailedMock.mock.calls.at(0)?.at(0)).toContain('Could not resolve deploy checklist'); expect(setOutputMock).not.toHaveBeenCalledWith('IS_LOCKED', expect.anything()); expect(setOutputMock).not.toHaveBeenCalledWith('NUMBER', expect.anything()); }); diff --git a/tests/tooling/markPullRequestsAsDeployed.test.ts b/tests/tooling/markPullRequestsAsDeployed.test.ts new file mode 100644 index 000000000000..21eac000e4a0 --- /dev/null +++ b/tests/tooling/markPullRequestsAsDeployed.test.ts @@ -0,0 +1,306 @@ +import {afterAll, afterEach, beforeAll, beforeEach, describe, expect, it, jest} from 'bun:test'; + +import * as core from '@actions/core'; + +import type {InternalOctokit} from '../../.github/libs/GithubUtils'; + +/* eslint-disable @typescript-eslint/naming-convention */ +import CONST from '../../.github/libs/CONST'; +import GithubUtils from '../../.github/libs/GithubUtils'; +import GitUtils from '../../.github/libs/GitUtils'; + +type ObjectMethodData = { + data: T; +}; + +type PullRequest = { + issue_number: number; + title: string; + merged_by: {login: string}; + labels: Array<{name: string}>; +}; + +type PullRequestParams = { + pull_number: number; +}; + +type PullRequestData = { + data?: PullRequest; +}; + +type Commit = { + commit_sha: string; +}; + +type CommitData = { + data: { + message: string; + }; +}; + +const mockGetInput = jest.fn(); +const mockGetPullRequest = jest.fn(); +const mockCreateComment = jest.fn(); +const mockListTags = jest.fn(); +const mockGetCommit = jest.fn(); + +// Must be set before `markPullRequestsAsDeployed` is imported below: it computes `workflowURL` from these env +// vars in a top-level (module-load-time) constant, not at runtime. +process.env.GITHUB_SERVER_URL = 'https://github.com'; +process.env.GITHUB_RUN_ID = '1234'; +const workflowRunURL = `${process.env.GITHUB_SERVER_URL}/${process.env.GITHUB_REPOSITORY}/actions/runs/${process.env.GITHUB_RUN_ID}`; + +// Mock core module. Real ESM module namespace exports are read-only live bindings, so `core.getInput` can't be +// reassigned directly (unlike Jest's Babel-transpiled CJS interop); spy on it instead. +jest.spyOn(core, 'getInput').mockImplementation(mockGetInput); + +// Must also be set before `markPullRequestsAsDeployed` is imported below: it does `memoize(GithubUtils.octokit.git. +// getCommit)` in a top-level (module-load-time) constant, capturing whatever `internalOctokit` points to at that +// moment rather than reading it fresh on each call. +const mockOctokit = { + rest: { + issues: { + listForRepo: jest.fn().mockImplementation(async () => ({ + data: [ + { + number: 5, + }, + ], + })), + + listEvents: jest.fn().mockImplementation(async () => ({ + data: [{event: 'closed', actor: {login: 'thor'}}], + })), + createComment: mockCreateComment, + }, + pulls: { + get: mockGetPullRequest, + }, + repos: { + listTags: mockListTags, + }, + git: { + getCommit: mockGetCommit, + }, + }, + paginate: jest.fn().mockImplementation((objectMethod: () => Promise>) => objectMethod().then(({data}) => data)), +}; +// eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion -- the stub implements only the endpoints this action touches, and the load-order constraint above rules out initialising a real octokit and spying on it +GithubUtils.internalOctokit = mockOctokit as unknown as InternalOctokit; + +// Must be imported after the GithubUtils.internalOctokit setup above so it picks up the mocks. +const {default: run} = await import('../../.github/actions/javascript/markPullRequestsAsDeployed/markPullRequestsAsDeployed'); + +const PRList: Record = { + 1: { + issue_number: 1, + title: 'Test PR 1', + merged_by: { + login: 'odin', + }, + labels: [], + }, + 2: { + issue_number: 2, + title: 'Test PR 2', + merged_by: { + login: 'loki', + }, + labels: [], + }, +}; +const version = '42.42.42-42'; +const defaultTags = [ + {name: '42.42.42-42', commit: {sha: 'abcd'}}, + {name: '42.42.42-41', commit: {sha: 'hash'}}, +]; + +// `core.getInput` always returns a string, so this returns strings too: the action's inputs go through the real +// ActionUtils.getJSONInput, which JSON.parses whatever it gets back. +function mockGetInputDefaultImplementation(key: string): string { + switch (key) { + case 'PR_LIST': + return JSON.stringify(Object.keys(PRList)); + case 'IS_PRODUCTION_DEPLOY': + return 'false'; + case 'DEPLOY_VERSION': + return version; + case 'IOS': + case 'ANDROID': + case 'WEB': + return 'success'; + case 'DATE': + case 'MOBILE_EXPENSIFY_PR_LIST': + case 'NOTE': + case 'ANDROID_SENTRY_URL': + case 'IOS_SENTRY_URL': + return ''; + default: + throw new Error(`Trying to access invalid input: ${key}`); + } +} + +function mockGetCommitDefaultImplementation({commit_sha}: Commit): CommitData { + if (commit_sha === 'abcd') { + return {data: {message: 'Test commit 1'}}; + } + return {data: {message: 'Test commit 2'}}; +} + +beforeAll(() => { + mockGetInput.mockImplementation(mockGetInputDefaultImplementation); + + // Mock GitUtils + GitUtils.getPullRequestsDeployedBetween = jest.fn(); +}); + +beforeEach(() => { + mockGetPullRequest.mockImplementation(({pull_number}: PullRequestParams): PullRequestData => (pull_number in PRList ? {data: PRList[pull_number]} : {})); + mockListTags.mockResolvedValue({ + data: defaultTags, + }); + mockGetCommit.mockImplementation(mockGetCommitDefaultImplementation); +}); + +afterEach(() => { + mockGetInput.mockClear(); + mockCreateComment.mockClear(); + mockGetPullRequest.mockClear(); +}); + +afterAll(() => { + jest.clearAllMocks(); +}); + +describe('markPullRequestsAsDeployed', () => { + it('comments on pull requests correctly for a standard staging deploy', async () => { + await run(); + expect(mockCreateComment).toHaveBeenCalledTimes(Object.keys(PRList).length); + for (let i = 0; i < Object.keys(PRList).length; i++) { + const PR = PRList[i + 1]; + expect(mockCreateComment).toHaveBeenNthCalledWith(i + 1, { + body: `🚀 [Deployed](${workflowRunURL}) to staging by https://github.com/${PR.merged_by.login} in version: ${version} 🚀 + +platform | result +---|--- +🕸 web 🕸|success ✅ +🤖 android 🤖|success ✅ +🍎 iOS 🍎|success ✅`, + issue_number: PR.issue_number, + owner: CONST.GITHUB_OWNER, + repo: CONST.APP_REPO, + }); + } + }); + + it('comments on pull requests correctly for a standard production deploy', async () => { + mockGetInput.mockImplementation((key: string) => { + if (key === 'IS_PRODUCTION_DEPLOY') { + return 'true'; + } + return mockGetInputDefaultImplementation(key); + }); + + await run(); + expect(mockCreateComment).toHaveBeenCalledTimes(Object.keys(PRList).length); + for (let i = 0; i < Object.keys(PRList).length; i++) { + expect(mockCreateComment).toHaveBeenNthCalledWith(i + 1, { + body: `🚀 [Deployed](${workflowRunURL}) to production by https://github.com/thor in version: ${version} 🚀 + +platform | result +---|--- +🕸 web 🕸|success ✅ +🤖 android 🤖|success ✅ +🍎 iOS 🍎|success ✅`, + issue_number: PRList[i + 1].issue_number, + owner: CONST.GITHUB_OWNER, + repo: CONST.APP_REPO, + }); + } + }); + + it('comments on pull requests correctly for a cherry pick', async () => { + mockGetInput.mockImplementation((key: string) => { + if (key === 'PR_LIST') { + return JSON.stringify([3]); + } + if (key === 'DEPLOY_VERSION') { + return '42.42.42-43'; + } + return mockGetInputDefaultImplementation(key); + }); + mockGetPullRequest.mockImplementation(({pull_number}: PullRequestParams) => { + if (pull_number === 3) { + return { + data: { + issue_number: 3, + title: 'Test PR 3', + merged_by: { + login: 'thor', + }, + labels: [{name: CONST.LABELS.CP_STAGING}], + }, + }; + } + return {}; + }); + mockListTags.mockResolvedValue({ + data: [{name: '42.42.42-43', commit: {sha: 'xyz'}}, ...defaultTags], + }); + mockGetCommit.mockImplementation(({commit_sha}: Commit) => { + if (commit_sha === 'xyz') { + return { + data: { + message: `Merge pull request #3 blahblahblah\\n(cherry picked from commit dag_dag)\\n(cherry-picked to staging by freyja)`, + }, + }; + } + return mockGetCommitDefaultImplementation({commit_sha}); + }); + + await run(); + expect(mockCreateComment).toHaveBeenCalledTimes(1); + expect(mockCreateComment).toHaveBeenCalledWith({ + body: `🚀 [Cherry-picked](${workflowRunURL}) to staging by https://github.com/freyja in version: 42.42.42-43 🚀 + +platform | result +---|--- +🕸 web 🕸|success ✅ +🤖 android 🤖|success ✅ +🍎 iOS 🍎|success ✅`, + issue_number: 3, + owner: CONST.GITHUB_OWNER, + repo: CONST.APP_REPO, + }); + }); + + it('comments on pull requests correctly when one platform fails', async () => { + mockGetInput.mockImplementation((key: string) => { + if (key === 'ANDROID') { + return 'skipped'; + } + if (key === 'IOS') { + return 'failed'; + } + return mockGetInputDefaultImplementation(key); + }); + + await run(); + expect(mockCreateComment).toHaveBeenCalledTimes(Object.keys(PRList).length); + for (let i = 0; i < Object.keys(PRList).length; i++) { + const PR = PRList[i + 1]; + expect(mockCreateComment).toHaveBeenNthCalledWith(i + 1, { + body: `🚀 [Deployed](${workflowRunURL}) to staging by https://github.com/${PR.merged_by.login} in version: ${version} 🚀 + +platform | result +---|--- +🕸 web 🕸|success ✅ +🤖 android 🤖|skipped 🚫 +🍎 iOS 🍎|failed ❌`, + issue_number: PR.issue_number, + owner: CONST.GITHUB_OWNER, + repo: CONST.APP_REPO, + }); + } + }); +}); diff --git a/tests/unit/postOrReplaceComment.ts b/tests/tooling/postOrReplaceComment.test.ts similarity index 64% rename from tests/unit/postOrReplaceComment.ts rename to tests/tooling/postOrReplaceComment.test.ts index 8f37589de9a1..60501edf82ef 100644 --- a/tests/unit/postOrReplaceComment.ts +++ b/tests/tooling/postOrReplaceComment.test.ts @@ -1,20 +1,17 @@ +import type {Mock} from 'bun:test'; +import {beforeAll, beforeEach, describe, expect, jest, test} from 'bun:test'; + import ghAction from '@github/actions/javascript/postOrReplaceComment/postOrReplaceComment'; import CONST from '@github/libs/CONST'; import GithubUtils from '@github/libs/GithubUtils'; -import asMutable from '@src/types/utils/asMutable'; - -/** - * @jest-environment node - */ import * as core from '@actions/core'; import {context} from '@actions/github'; import * as GitHubEnvironment from '@actions/github/lib/utils'; -import {when} from 'jest-when'; import createMock from '../utils/createMock'; -const mockGetInput = jest.fn(); +const mockGetInput = jest.fn(); const createCommentMock = jest.spyOn(GithubUtils, 'createComment'); type InternalOctokit = NonNullable; type CreateCommentResponse = Awaited>; @@ -24,27 +21,26 @@ type ListCommentsEndpoint = ListCommentsMethod['endpoint']; type GraphqlMethod = InternalOctokit['graphql']; let internalOctokit: InternalOctokit; -let listCommentsSpy: jest.SpiedFunction; -let graphqlSpy: jest.SpiedFunction; - -jest.mock('@actions/github', () => { - const repository = process.env.GITHUB_REPOSITORY; - if (!repository) { - throw new Error('GITHUB_REPOSITORY must be set in owner/repository format.'); - } +let listCommentsSpy: Mock; +let graphqlSpy: Mock; - const [owner, repo, ...extraParts] = repository.split('/'); - if (!owner || !repo || extraParts.length > 0 || /\s/.test(owner) || /\s/.test(repo)) { - throw new Error(`GITHUB_REPOSITORY must be set in owner/repository format, received: ${repository}`); - } +// `context` is a plain object instance, so the fields this action reads can be assigned directly rather than +// mocking the module. `context.repo` derives from GITHUB_REPOSITORY, which tests/tooling/setup.ts defaults to +// Expensify/App, and `runId` is fixed here so the expected messages below don't depend on the environment. +context.runId = 1234; - return { - context: { - repo: {owner, repo}, - runId: 1234, - }, - }; -}); +/** + * Stubs `core.getInput` for one test. Reading an input the test didn't declare throws rather than silently + * returning the empty string, so a new `getInput` call in the action can't quietly change what these tests assert. + */ +function mockInputs(inputs: Record) { + mockGetInput.mockImplementation((name: string) => { + if (!(name in inputs)) { + throw new Error(`Unexpected core.getInput('${name}'): add it to this test's inputs.`); + } + return inputs[name]; + }); +} const previousCommentsResponse = createMock({ data: [ @@ -66,7 +62,7 @@ const commentsFetchResponse = createMock({ headers: commentsResponseHeaders, json: () => Promise.resolve(previousCommentsResponse.data), }); -const fetchComments: typeof globalThis.fetch = () => Promise.resolve(commentsFetchResponse); +const fetchComments: typeof globalThis.fetch = Object.assign(() => Promise.resolve(commentsFetchResponse), {preconnect: () => {}}); beforeAll(() => { const getOctokitOptions = GitHubEnvironment.getOctokitOptions; @@ -96,7 +92,7 @@ beforeAll(() => { listCommentsSpy = jest.spyOn(internalOctokit.rest.issues.listComments, 'endpoint'); jest.spyOn(internalOctokit, 'paginate'); graphqlSpy = jest.spyOn(internalOctokit, 'graphql'); - graphqlSpy.mockImplementation(() => Promise.resolve({})); + graphqlSpy.mockResolvedValue({}); }); const androidLink = 'https://expensify.app/ANDROID_LINK'; @@ -163,8 +159,9 @@ Built from Mobile-Expensify PR Expensify/Mobile-Expensify#13. describe('postOrReplaceComment action tests', () => { beforeAll(() => { - // Mock core module - asMutable(core).getInput = mockGetInput; + // Real ESM module namespace exports are read-only live bindings, so `core.getInput` can't be reassigned + // directly the way Jest's Babel-transpiled CJS interop allowed; spy on it instead. + jest.spyOn(core, 'getInput').mockImplementation(mockGetInput); }); beforeEach(() => { @@ -191,17 +188,19 @@ describe('postOrReplaceComment action tests', () => { } test('Test GH action', async () => { - when(core.getInput).calledWith('REPO', {required: true}).mockReturnValue(CONST.APP_REPO); - when(core.getInput).calledWith('APP_PR_NUMBER', {required: false}).mockReturnValue('12'); - when(core.getInput).calledWith('MOBILE_EXPENSIFY_PR_NUMBER', {required: false}).mockReturnValue('13'); - when(core.getInput).calledWith('COMMENT_PREFIX', {required: true}).mockReturnValue(testBuildCommentPrefix); - when(core.getInput).calledWith('COMMENT_BODY', {required: false}).mockReturnValue(''); - when(core.getInput).calledWith('ANDROID', {required: false}).mockReturnValue('success'); - when(core.getInput).calledWith('IOS', {required: false}).mockReturnValue('success'); - when(core.getInput).calledWith('WEB', {required: false}).mockReturnValue('success'); - when(core.getInput).calledWith('ANDROID_LINK').mockReturnValue(androidLink); - when(core.getInput).calledWith('IOS_LINK').mockReturnValue(iOSLink); - when(core.getInput).calledWith('WEB_LINK').mockReturnValue('https://expensify.app/WEB_LINK'); + mockInputs({ + REPO: CONST.APP_REPO, + APP_PR_NUMBER: '12', + MOBILE_EXPENSIFY_PR_NUMBER: '13', + COMMENT_PREFIX: testBuildCommentPrefix, + COMMENT_BODY: '', + ANDROID: 'success', + IOS: 'success', + WEB: 'success', + ANDROID_LINK: androidLink, + IOS_LINK: iOSLink, + WEB_LINK: webLink, + }); createCommentMock.mockResolvedValue(createMock({})); await ghAction(); expectPreviousCommentToBeHidden(); @@ -210,15 +209,17 @@ describe('postOrReplaceComment action tests', () => { }); test('Test GH action when only App PR number is provided', async () => { - when(core.getInput).calledWith('REPO', {required: true}).mockReturnValue(CONST.APP_REPO); - when(core.getInput).calledWith('APP_PR_NUMBER', {required: false}).mockReturnValue('12'); - when(core.getInput).calledWith('MOBILE_EXPENSIFY_PR_NUMBER', {required: false}).mockReturnValue(''); - when(core.getInput).calledWith('COMMENT_PREFIX', {required: true}).mockReturnValue(testBuildCommentPrefix); - when(core.getInput).calledWith('COMMENT_BODY', {required: false}).mockReturnValue(''); - when(core.getInput).calledWith('ANDROID', {required: false}).mockReturnValue('success'); - when(core.getInput).calledWith('IOS', {required: false}).mockReturnValue('skipped'); - when(core.getInput).calledWith('WEB', {required: false}).mockReturnValue('skipped'); - when(core.getInput).calledWith('ANDROID_LINK').mockReturnValue('https://expensify.app/ANDROID_LINK'); + mockInputs({ + REPO: CONST.APP_REPO, + APP_PR_NUMBER: '12', + MOBILE_EXPENSIFY_PR_NUMBER: '', + COMMENT_PREFIX: testBuildCommentPrefix, + COMMENT_BODY: '', + ANDROID: 'success', + IOS: 'skipped', + WEB: 'skipped', + ANDROID_LINK: androidLink, + }); createCommentMock.mockResolvedValue(createMock({})); await ghAction(); expectPreviousCommentToBeHidden(); @@ -227,16 +228,18 @@ describe('postOrReplaceComment action tests', () => { }); test('Test GH action when only Mobile-Expensify PR number is provided', async () => { - when(core.getInput).calledWith('REPO', {required: true}).mockReturnValue(CONST.MOBILE_EXPENSIFY_REPO); - when(core.getInput).calledWith('APP_PR_NUMBER', {required: false}).mockReturnValue(''); - when(core.getInput).calledWith('MOBILE_EXPENSIFY_PR_NUMBER', {required: false}).mockReturnValue('13'); - when(core.getInput).calledWith('COMMENT_PREFIX', {required: true}).mockReturnValue(testBuildCommentPrefix); - when(core.getInput).calledWith('COMMENT_BODY', {required: false}).mockReturnValue(''); - when(core.getInput).calledWith('ANDROID', {required: false}).mockReturnValue('success'); - when(core.getInput).calledWith('IOS', {required: false}).mockReturnValue('success'); - when(core.getInput).calledWith('ANDROID_LINK').mockReturnValue(androidLink); - when(core.getInput).calledWith('IOS_LINK').mockReturnValue(iOSLink); - when(core.getInput).calledWith('WEB', {required: false}).mockReturnValue('skipped'); + mockInputs({ + REPO: CONST.MOBILE_EXPENSIFY_REPO, + APP_PR_NUMBER: '', + MOBILE_EXPENSIFY_PR_NUMBER: '13', + COMMENT_PREFIX: testBuildCommentPrefix, + COMMENT_BODY: '', + ANDROID: 'success', + IOS: 'success', + WEB: 'skipped', + ANDROID_LINK: androidLink, + IOS_LINK: iOSLink, + }); createCommentMock.mockResolvedValue(createMock({})); await ghAction(); expectPreviousCommentToBeHidden(); diff --git a/tests/tooling/setup.ts b/tests/tooling/setup.ts new file mode 100644 index 000000000000..f4ca502ad5d1 --- /dev/null +++ b/tests/tooling/setup.ts @@ -0,0 +1,23 @@ +// Preloaded via `bun test --preload` (see the `test:bun` npm script), once per test file because --isolate +// gives each file its own globals. Always run this directory through that script: several files replace `fs` or +// `child_process` with mock.module(), which without --isolate would reach every file that runs after them. +import {jest} from 'bun:test'; + +// GitHub Actions always sets GITHUB_REPOSITORY in CI, but local runs need a default, mirroring jest/setup.ts's +// equivalent fallback for the test files Jest still owns. +if (!('GITHUB_REPOSITORY' in process.env)) { + (process.env as NodeJS.ProcessEnv).GITHUB_REPOSITORY_OWNER = 'Expensify'; + (process.env as NodeJS.ProcessEnv).GITHUB_REPOSITORY = 'Expensify/App'; +} + +// The code under test logs heavily, which drowns out the actual results. Jest's CI runs pass --silent for the same +// reason; `bun test` has no equivalent flag, so swap in stubs. Workflow commands that @actions/core writes straight +// to process.stdout still come through, as does anything bun:test reports about a failure. +globalThis.console = { + ...console, + log: jest.fn(), + info: jest.fn(), + debug: jest.fn(), + warn: jest.fn(), + error: jest.fn(), +}; diff --git a/tests/tooling/tsconfig.json b/tests/tooling/tsconfig.json new file mode 100644 index 000000000000..89e7d0f63b6d --- /dev/null +++ b/tests/tooling/tsconfig.json @@ -0,0 +1,17 @@ +{ + // CIGitLogic is the one test here that uses Bun's runtime API — the `$` shell — which needs `@types/bun`. + // Those types redeclare globals that the app's own types already own: a `jest` namespace that shadows + // @types/jest's generic signatures, and a `fetch` carrying `preconnect`. Loading them into the root project + // produces ~1,100 errors across tests/unit, so this project confines them to the single file that needs them. + // Everything else in tests/tooling stays in the root project, where it sees the app's real types. + // + // `include: []` is required: `extends` inherits the root project's `include`, which would otherwise pull the + // whole app in here and reintroduce the same conflict this file exists to avoid. + "extends": "../../tsconfig.json", + "compilerOptions": { + "types": ["@types/bun"], + "noEmit": true + }, + "include": [], + "files": ["CIGitLogic.test.ts"] +} diff --git a/tests/unit/versionUpdaterTest.ts b/tests/tooling/versionUpdater.test.ts similarity index 97% rename from tests/unit/versionUpdaterTest.ts rename to tests/tooling/versionUpdater.test.ts index e59288710b85..a63aca84b25b 100644 --- a/tests/unit/versionUpdaterTest.ts +++ b/tests/tooling/versionUpdater.test.ts @@ -1,3 +1,5 @@ +import {describe, expect, it, test} from 'bun:test'; + import * as versionUpdater from '../../.github/libs/versionUpdater'; const VERSION = '2.3.9-80'; @@ -6,7 +8,7 @@ const VERSION_NUMBER = [2, 3, 9, 80] as const; describe('versionUpdater', () => { describe('getVersionNumberFromString', () => { it('should return a list with version levels numbers', () => { - expect(versionUpdater.getVersionNumberFromString(VERSION)).toStrictEqual(VERSION_NUMBER); + expect(versionUpdater.getVersionNumberFromString(VERSION)).toStrictEqual([...VERSION_NUMBER]); }); it('should return build as zero if not present in string', () => { diff --git a/tests/unit/waitForPreviousRunsTest.ts b/tests/tooling/waitForPreviousRuns.test.ts similarity index 92% rename from tests/unit/waitForPreviousRunsTest.ts rename to tests/tooling/waitForPreviousRuns.test.ts index ece675aa4647..c224b6c936b2 100644 --- a/tests/unit/waitForPreviousRunsTest.ts +++ b/tests/tooling/waitForPreviousRuns.test.ts @@ -1,12 +1,9 @@ +import {beforeAll, beforeEach, describe, expect, jest, test} from 'bun:test'; + import run from '@github/actions/javascript/waitForPreviousRuns/waitForPreviousRuns'; import GithubUtils from '@github/libs/GithubUtils'; -import asMutable from '@src/types/utils/asMutable'; - /* eslint-disable @typescript-eslint/naming-convention */ -/** - * @jest-environment node - */ import * as core from '@actions/core'; import createMock from '../utils/createMock'; @@ -21,7 +18,7 @@ type ListWorkflowRunsResponse = Awaited>; type WorkflowRun = Pick; const mockGetInput = jest.fn(); -const mockListWorkflowRuns = jest.fn, Parameters>(); +const mockListWorkflowRuns = jest.fn<(...args: Parameters) => ReturnType>(); /** Mock a single poll response with the given runs. */ function mockPoll(runs: WorkflowRun[]) { @@ -54,16 +51,10 @@ function getErrorMessages(): string[] { return coreErrorSpy.mock.calls.map((call) => String(call[0])); } -jest.mock('@github/libs/CONST', () => ({ - __esModule: true, - default: { - GITHUB_OWNER: 'Expensify', - APP_REPO: 'App', - }, -})); - beforeAll(() => { - asMutable(core).getInput = mockGetInput; + // Real ESM module namespace exports are read-only live bindings, so `core.getInput` can't be reassigned + // directly (unlike Jest's Babel-transpiled CJS interop); spy on it instead. + jest.spyOn(core, 'getInput').mockImplementation(mockGetInput); mockGetInput.mockImplementation((name: string) => { if (name === 'WORKFLOW_ID') { @@ -82,7 +73,10 @@ beforeAll(() => { }); GithubUtils.initOctokitWithToken('fake_token'); - jest.spyOn(GithubUtils.octokit.actions, 'listWorkflowRuns').mockImplementation(mockListWorkflowRuns); + // Octokit endpoint methods carry `defaults`/`endpoint` statics that a bare mock doesn't, so the real ones are + // copied onto the stub rather than asserted away. + const {endpoint, defaults} = GithubUtils.octokit.actions.listWorkflowRuns; + jest.spyOn(GithubUtils.octokit.actions, 'listWorkflowRuns').mockImplementation(Object.assign(mockListWorkflowRuns, {endpoint, defaults})); }); beforeEach(() => { diff --git a/tests/unit/markPullRequestsAsDeployedTest.ts b/tests/unit/markPullRequestsAsDeployedTest.ts deleted file mode 100644 index 024ea45561dc..000000000000 --- a/tests/unit/markPullRequestsAsDeployedTest.ts +++ /dev/null @@ -1,335 +0,0 @@ -import type {InternalOctokit} from '../../.github/libs/GithubUtils'; - -/** - * @jest-environment node - */ -/* eslint-disable @typescript-eslint/naming-convention */ -import CONST from '../../.github/libs/CONST'; -import GithubUtils from '../../.github/libs/GithubUtils'; -import GitUtils from '../../.github/libs/GitUtils'; -import createMock from '../utils/createMock'; - -type GetPullRequest = InternalOctokit['rest']['pulls']['get']; -type GetPullRequestResponse = Awaited>; -type PullRequest = GetPullRequestResponse['data']; -type ListTags = InternalOctokit['rest']['repos']['listTags']; -type ListTagsResponse = Awaited>; -type Tag = ListTagsResponse['data'][number]; -type GetCommit = InternalOctokit['rest']['git']['getCommit']; -type GetCommitResponse = Awaited>; -type Commit = GetCommitResponse['data']; -type CreateComment = InternalOctokit['rest']['issues']['createComment']; -type CreateCommentResponse = Awaited>; - -let run: () => Promise; - -const mockGetInput = jest.fn(); -const mockGetPullRequest = jest.fn, Parameters>(); -const mockCreateComment = jest.fn, Parameters>(); -const mockListTags = jest.fn, Parameters>(); -const mockGetCommit = jest.fn, Parameters>(); - -let workflowRunURL: string | null; - -const PRList: Record = { - 1: createMock({ - number: 1, - title: 'Test PR 1', - merged_by: { - login: 'odin', - }, - labels: [], - }), - 2: createMock({ - number: 2, - title: 'Test PR 2', - merged_by: { - login: 'loki', - }, - labels: [], - }), -}; -const version = '42.42.42-42'; -const defaultTags: ListTagsResponse['data'] = [createMock({name: '42.42.42-42', commit: {sha: 'abcd'}}), createMock({name: '42.42.42-41', commit: {sha: 'hash'}})]; - -function mockGetInputDefaultImplementation(key: string): boolean | string { - switch (key) { - case 'PR_LIST': - return JSON.stringify(Object.keys(PRList)); - case 'IS_PRODUCTION_DEPLOY': - return false; - case 'DEPLOY_VERSION': - return version; - case 'IOS': - case 'ANDROID': - case 'WEB': - return 'success'; - case 'DATE': - case 'NOTE': - case 'ANDROID_SENTRY_URL': - case 'IOS_SENTRY_URL': - return ''; - default: - throw new Error(`Trying to access invalid input: ${key}`); - } -} - -async function mockGetCommitDefaultImplementation(...[params]: Parameters): ReturnType { - if (!params) { - throw new Error('Commit parameters are required.'); - } - const {commit_sha} = params; - if (commit_sha === 'abcd') { - return {data: createMock({message: 'Test commit 1'}), headers: {}, status: 200, url: ''}; - } - return {data: createMock({message: 'Test commit 2'}), headers: {}, status: 200, url: ''}; -} - -beforeAll(() => { - // Mock core module - jest.mock('@actions/core', () => ({ - getInput: mockGetInput, - })); - mockGetInput.mockImplementation(mockGetInputDefaultImplementation); - - // Mock octokit module - GithubUtils.initOctokitWithToken('fake_token'); - const initializedOctokit = GithubUtils.internalOctokit; - if (!initializedOctokit) { - throw new Error('GithubUtils failed to initialize Octokit.'); - } - jest.spyOn(initializedOctokit.rest.issues, 'listForRepo').mockResolvedValue( - createMock>>({ - data: [{number: 5}], - headers: {}, - }), - ); - const listEventsEndpoint = initializedOctokit.rest.issues.listEvents.endpoint; - const listEventsDefaults = initializedOctokit.rest.issues.listEvents.defaults; - jest.spyOn(initializedOctokit.rest.issues, 'listEvents').mockResolvedValue( - createMock>>({ - data: [{event: 'closed', actor: {login: 'thor'}}], - headers: {}, - }), - ); - const mockListEvents = jest.mocked(initializedOctokit.rest.issues.listEvents, {shallow: true}); - mockListEvents.endpoint = listEventsEndpoint; - mockListEvents.defaults = listEventsDefaults; - jest.spyOn(initializedOctokit.rest.issues, 'createComment').mockImplementation((...args) => mockCreateComment(...args)); - jest.spyOn(initializedOctokit.rest.pulls, 'get').mockImplementation((...args) => mockGetPullRequest(...args)); - jest.spyOn(initializedOctokit.rest.repos, 'listTags').mockImplementation((...args) => mockListTags(...args)); - jest.spyOn(initializedOctokit.rest.git, 'getCommit').mockImplementation((...args) => mockGetCommit(...args)); - - // Mock GitUtils - GitUtils.getPullRequestsDeployedBetween = jest.fn(); - - jest.mock('../../.github/libs/ActionUtils', () => ({ - getJSONInput: jest.fn().mockImplementation((name: string, defaultValue: string) => { - try { - const input = String(mockGetInput(name)); - return JSON.parse(input) as unknown; - } catch (err) { - return defaultValue; - } - }), - })); - - // Set GH runner environment variables - process.env.GITHUB_SERVER_URL = 'https://github.com'; - process.env.GITHUB_RUN_ID = '1234'; - workflowRunURL = `${process.env.GITHUB_SERVER_URL}/${process.env.GITHUB_REPOSITORY}/actions/runs/${process.env.GITHUB_RUN_ID}`; -}); - -beforeEach(() => { - mockGetPullRequest.mockImplementation(async (params) => { - if (!params) { - throw new Error('Pull request parameters are required.'); - } - const pullRequest = PRList[params.pull_number]; - if (!pullRequest) { - throw new Error(`Unexpected pull request: ${params.pull_number}`); - } - return {data: pullRequest, headers: {}, status: 200, url: ''}; - }); - mockCreateComment.mockResolvedValue({data: createMock({}), headers: {}, status: 201, url: ''}); - mockListTags.mockResolvedValue({data: defaultTags, headers: {}, status: 200, url: ''}); - mockGetCommit.mockImplementation(mockGetCommitDefaultImplementation); -}); - -afterEach(() => { - mockGetInput.mockClear(); - mockCreateComment.mockClear(); - mockGetPullRequest.mockClear(); -}); - -afterAll(() => { - jest.clearAllMocks(); -}); - -type MockedActionRun = () => Promise; - -describe('markPullRequestsAsDeployed', () => { - it('comments on pull requests correctly for a standard staging deploy', async () => { - // Note: we import this in here so that it executes after all the mocks are set up - run = require('../../.github/actions/javascript/markPullRequestsAsDeployed/markPullRequestsAsDeployed'); - await run(); - expect(mockCreateComment).toHaveBeenCalledTimes(Object.keys(PRList).length); - for (let i = 0; i < Object.keys(PRList).length; i++) { - const PR = PRList[i + 1]; - if (!PR.merged_by) { - throw new Error(`Pull request ${PR.number} has no merger.`); - } - expect(mockCreateComment).toHaveBeenNthCalledWith(i + 1, { - body: `🚀 [Deployed](${workflowRunURL}) to staging by https://github.com/${PR.merged_by.login} in version: ${version} 🚀 - -platform | result ----|--- -🕸 web 🕸|success ✅ -🤖 android 🤖|success ✅ -🍎 iOS 🍎|success ✅`, - issue_number: PR.number, - owner: CONST.GITHUB_OWNER, - repo: CONST.APP_REPO, - }); - } - }); - - it('comments on pull requests correctly for a standard production deploy', async () => { - mockGetInput.mockImplementation((key: string) => { - if (key === 'IS_PRODUCTION_DEPLOY') { - return true; - } - return mockGetInputDefaultImplementation(key); - }); - - // Note: we import this in here so that it executes after all the mocks are set up - run = require('../../.github/actions/javascript/markPullRequestsAsDeployed/markPullRequestsAsDeployed'); - - await run(); - expect(mockCreateComment).toHaveBeenCalledTimes(Object.keys(PRList).length); - for (let i = 0; i < Object.keys(PRList).length; i++) { - expect(mockCreateComment).toHaveBeenNthCalledWith(i + 1, { - body: `🚀 [Deployed](${workflowRunURL}) to production by https://github.com/thor in version: ${version} 🚀 - -platform | result ----|--- -🕸 web 🕸|success ✅ -🤖 android 🤖|success ✅ -🍎 iOS 🍎|success ✅`, - issue_number: PRList[i + 1].number, - owner: CONST.GITHUB_OWNER, - repo: CONST.APP_REPO, - }); - } - }); - - it('comments on pull requests correctly for a cherry pick', async () => { - mockGetInput.mockImplementation((key: string) => { - if (key === 'PR_LIST') { - return JSON.stringify([3]); - } - if (key === 'DEPLOY_VERSION') { - return '42.42.42-43'; - } - return mockGetInputDefaultImplementation(key); - }); - mockGetPullRequest.mockImplementation(async (params) => { - if (!params) { - throw new Error('Pull request parameters are required.'); - } - const {pull_number} = params; - if (pull_number === 3) { - return { - data: createMock({ - number: 3, - title: 'Test PR 3', - merged_by: { - login: 'thor', - }, - labels: [{name: CONST.LABELS.CP_STAGING}], - }), - headers: {}, - status: 200, - url: '', - }; - } - throw new Error(`Unexpected pull request: ${pull_number}`); - }); - mockListTags.mockResolvedValue({ - data: [createMock({name: '42.42.42-43', commit: {sha: 'xyz'}}), ...defaultTags], - headers: {}, - status: 200, - url: '', - }); - mockGetCommit.mockImplementation(async (...args) => { - const [params] = args; - if (!params) { - throw new Error('Commit parameters are required.'); - } - const {commit_sha} = params; - if (commit_sha === 'xyz') { - return { - data: createMock({ - message: `Merge pull request #3 blahblahblah\\n(cherry picked from commit dag_dag)\\n(cherry-picked to staging by freyja)`, - }), - headers: {}, - status: 200, - url: '', - }; - } - return mockGetCommitDefaultImplementation(...args); - }); - - // Note: we import this in here so that it executes after all the mocks are set up - run = require('../../.github/actions/javascript/markPullRequestsAsDeployed/markPullRequestsAsDeployed'); - await run(); - expect(mockCreateComment).toHaveBeenCalledTimes(1); - expect(mockCreateComment).toHaveBeenCalledWith({ - body: `🚀 [Cherry-picked](${workflowRunURL}) to staging by https://github.com/freyja in version: 42.42.42-43 🚀 - -platform | result ----|--- -🕸 web 🕸|success ✅ -🤖 android 🤖|success ✅ -🍎 iOS 🍎|success ✅`, - issue_number: 3, - owner: CONST.GITHUB_OWNER, - repo: CONST.APP_REPO, - }); - }); - - it('comments on pull requests correctly when one platform fails', async () => { - mockGetInput.mockImplementation((key: string) => { - if (key === 'ANDROID') { - return 'skipped'; - } - if (key === 'IOS') { - return 'failed'; - } - return mockGetInputDefaultImplementation(key); - }); - - // Note: we import this in here so that it executes after all the mocks are set up - run = require('../../.github/actions/javascript/markPullRequestsAsDeployed/markPullRequestsAsDeployed'); - await run(); - expect(mockCreateComment).toHaveBeenCalledTimes(Object.keys(PRList).length); - for (let i = 0; i < Object.keys(PRList).length; i++) { - const PR = PRList[i + 1]; - if (!PR.merged_by) { - throw new Error(`Pull request ${PR.number} has no merger.`); - } - expect(mockCreateComment).toHaveBeenNthCalledWith(i + 1, { - body: `🚀 [Deployed](${workflowRunURL}) to staging by https://github.com/${PR.merged_by.login} in version: ${version} 🚀 - -platform | result ----|--- -🕸 web 🕸|success ✅ -🤖 android 🤖|skipped 🚫 -🍎 iOS 🍎|failed ❌`, - issue_number: PR.number, - owner: CONST.GITHUB_OWNER, - repo: CONST.APP_REPO, - }); - } - }); -}); diff --git a/tsconfig.json b/tsconfig.json index e9feecbbc06b..59575a3578f3 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -24,6 +24,11 @@ "tests/*": ["./tests/*"] } }, + // bun:test's module declaration, for tests/tooling. Referenced through `files` rather than `include` + // because `exclude` below covers node_modules and would otherwise drop it. This is the one file in bun-types + // that declares the module; the package's other declarations (notably its global JSX namespace) are + // deliberately not pulled in, as they conflict with the app's React types. + "files": ["node_modules/bun-types/test.d.ts"], "include": [ "src", "web", @@ -43,5 +48,7 @@ "**/*.nitro/*.ts", "**/*.nitro/*.tsx" ], - "exclude": ["**/node_modules/*", "**/dist/*", ".github/actions/**/index.js", "**/docs/*", ".claude/worktrees/**"] + // CIGitLogic uses Bun's `$` shell, so it needs @types/bun, whose globals conflict with this project's. + // It is type-checked by tests/tooling/tsconfig.json instead — see the comment there. + "exclude": ["**/node_modules/*", "**/dist/*", ".github/actions/**/index.js", "**/docs/*", ".claude/worktrees/**", "tests/tooling/CIGitLogic.test.ts"] }