From dd721148b51f97130b3fe11bcca1fc3aed663468 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Thu, 8 Dec 2022 22:33:20 +0100 Subject: [PATCH 1/4] cli: fix coverage config warnings Signed-off-by: Patrik Oldsberg --- .changeset/fast-walls-explode.md | 5 +++++ packages/cli/config/jest.js | 22 ++++++++++++++-------- 2 files changed, 19 insertions(+), 8 deletions(-) create mode 100644 .changeset/fast-walls-explode.md diff --git a/.changeset/fast-walls-explode.md b/.changeset/fast-walls-explode.md new file mode 100644 index 0000000000..ac4c4335bc --- /dev/null +++ b/.changeset/fast-walls-explode.md @@ -0,0 +1,5 @@ +--- +'@backstage/cli': patch +--- + +Updated Jest coverage configuration to only apply either in the root project or package configuration, depending on whether repo or package tests are run. diff --git a/packages/cli/config/jest.js b/packages/cli/config/jest.js index 148d574441..cbe0fda0de 100644 --- a/packages/cli/config/jest.js +++ b/packages/cli/config/jest.js @@ -65,7 +65,7 @@ function getRoleConfig(role) { } } -async function getProjectConfig(targetPath, displayName) { +async function getProjectConfig(targetPath, extraConfig) { const configJsPath = path.resolve(targetPath, 'jest.config.js'); const configTsPath = path.resolve(targetPath, 'jest.config.ts'); // If the package has it's own jest config, we use that instead. @@ -125,11 +125,8 @@ async function getProjectConfig(targetPath, displayName) { } const options = { - ...(displayName && { displayName }), + ...extraConfig, rootDir: path.resolve(targetPath, 'src'), - coverageDirectory: path.resolve(targetPath, 'coverage'), - coverageProvider: envOptions.nextTests ? 'babel' : 'v8', - collectCoverageFrom: ['**/*.{js,jsx,ts,tsx,mjs,cjs}', '!**/*.d.ts'], moduleNameMapper: { '\\.(css|less|scss|sss|styl)$': require.resolve('jest-css-modules'), }, @@ -237,15 +234,21 @@ async function getRootConfig() { const targetPackagePath = path.resolve(targetPath, 'package.json'); const exists = await fs.pathExists(targetPackagePath); + const coverageConfig = { + coverageDirectory: path.resolve(targetPath, 'coverage'), + coverageProvider: envOptions.nextTests ? 'babel' : 'v8', + collectCoverageFrom: ['**/*.{js,jsx,ts,tsx,mjs,cjs}', '!**/*.d.ts'], + }; + if (!exists) { - return getProjectConfig(targetPath); + return getProjectConfig(targetPath, coverageConfig); } // Check whether the current package is a workspace root or not const data = await fs.readJson(targetPackagePath); const workspacePatterns = data.workspaces && data.workspaces.packages; if (!workspacePatterns) { - return getProjectConfig(targetPath); + return getProjectConfig(targetPath, coverageConfig); } // If the target package is a workspace root, we find all packages in the @@ -269,7 +272,9 @@ async function getRootConfig() { testScript?.includes('backstage-cli test') || testScript?.includes('backstage-cli package test'); if (testScript && isSupportedTestScript) { - return await getProjectConfig(projectPath, packageData.name); + return await getProjectConfig(projectPath, { + displayName: packageData.name, + }); } return undefined; @@ -279,6 +284,7 @@ async function getRootConfig() { return { rootDir: targetPath, projects: configs, + ...coverageConfig, }; } From 07283a62c1a36d2658da584b090aee50fc5def54 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Thu, 8 Dec 2022 22:54:39 +0100 Subject: [PATCH 2/4] cli: make next jest config the default Signed-off-by: Patrik Oldsberg --- .github/workflows/ci.yml | 2 -- .github/workflows/deploy_packages.yml | 1 - .github/workflows/verify_windows.yml | 1 - packages/cli/config/jest.js | 20 ++++++++++---------- 4 files changed, 10 insertions(+), 14 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 7cc5874184..65c2c76242 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -202,7 +202,6 @@ jobs: if: ${{ steps.yarn-lock.outcome == 'success' }} run: yarn backstage-cli repo test --maxWorkers=2 --workerIdleMemoryLimit=1300M --since origin/master env: - BACKSTAGE_NEXT_TESTS: 1 BACKSTAGE_TEST_DISABLE_DOCKER: 1 BACKSTAGE_TEST_DATABASE_POSTGRES13_CONNECTION_STRING: postgresql://postgres:postgres@localhost:${{ job.services.postgres13.ports[5432] }} BACKSTAGE_TEST_DATABASE_POSTGRES9_CONNECTION_STRING: postgresql://postgres:postgres@localhost:${{ job.services.postgres9.ports[5432] }} @@ -214,7 +213,6 @@ jobs: yarn backstage-cli repo test --maxWorkers=2 --workerIdleMemoryLimit=800M --coverage bash <(curl -s https://codecov.io/bash) -N $(git rev-parse FETCH_HEAD) env: - BACKSTAGE_NEXT_TESTS: 1 BACKSTAGE_TEST_DISABLE_DOCKER: 1 BACKSTAGE_TEST_DATABASE_POSTGRES13_CONNECTION_STRING: postgresql://postgres:postgres@localhost:${{ job.services.postgres13.ports[5432] }} BACKSTAGE_TEST_DATABASE_POSTGRES9_CONNECTION_STRING: postgresql://postgres:postgres@localhost:${{ job.services.postgres9.ports[5432] }} diff --git a/.github/workflows/deploy_packages.yml b/.github/workflows/deploy_packages.yml index bebe24d0f6..63dc93ba2c 100644 --- a/.github/workflows/deploy_packages.yml +++ b/.github/workflows/deploy_packages.yml @@ -105,7 +105,6 @@ jobs: bash <(curl -s https://codecov.io/bash) -f packages/core-components/coverage/* -F core-components bash <(curl -s https://codecov.io/bash) -f packages/core-plugin-api/coverage/* -F core-plugin-api env: - BACKSTAGE_NEXT_TESTS: 1 BACKSTAGE_TEST_DISABLE_DOCKER: 1 BACKSTAGE_TEST_DATABASE_POSTGRES13_CONNECTION_STRING: postgresql://postgres:postgres@localhost:${{ job.services.postgres13.ports[5432] }} BACKSTAGE_TEST_DATABASE_POSTGRES9_CONNECTION_STRING: postgresql://postgres:postgres@localhost:${{ job.services.postgres9.ports[5432] }} diff --git a/.github/workflows/verify_windows.yml b/.github/workflows/verify_windows.yml index 75a0633cbe..916dd53366 100644 --- a/.github/workflows/verify_windows.yml +++ b/.github/workflows/verify_windows.yml @@ -48,7 +48,6 @@ jobs: - name: test run: yarn backstage-cli repo test --maxWorkers=2 --workerIdleMemoryLimit=1300M env: - BACKSTAGE_NEXT_TESTS: 1 BACKSTAGE_TEST_DISABLE_DOCKER: 1 # credit: https://github.com/appleboy/discord-action/issues/3#issuecomment-731426861 diff --git a/packages/cli/config/jest.js b/packages/cli/config/jest.js index cbe0fda0de..2c224f8161 100644 --- a/packages/cli/config/jest.js +++ b/packages/cli/config/jest.js @@ -21,11 +21,11 @@ const glob = require('util').promisify(require('glob')); const { version } = require('../package.json'); const envOptions = { - nextTests: Boolean(process.env.BACKSTAGE_NEXT_TESTS), + oldTests: Boolean(process.env.BACKSTAGE_OLD_TESTS), enableSourceMaps: Boolean(process.env.ENABLE_SOURCE_MAPS), }; -if (envOptions.nextTests) { +if (!envOptions.oldTests) { // Needed so that, at import-time, it can hook into Jest's internals. require('./jestCachingModuleLoader'); } @@ -135,7 +135,7 @@ async function getProjectConfig(targetPath, extraConfig) { '\\.(mjs|cjs|js)$': [ require.resolve('./jestSwcTransform'), { - sourceMaps: envOptions.enableSourceMaps || envOptions.nextTests, + sourceMaps: envOptions.enableSourceMaps || !envOptions.oldTests, jsc: { parser: { syntax: 'ecmascript', @@ -146,7 +146,7 @@ async function getProjectConfig(targetPath, extraConfig) { '\\.jsx$': [ require.resolve('./jestSwcTransform'), { - sourceMaps: envOptions.enableSourceMaps || envOptions.nextTests, + sourceMaps: envOptions.enableSourceMaps || !envOptions.oldTests, jsc: { parser: { syntax: 'ecmascript', @@ -163,7 +163,7 @@ async function getProjectConfig(targetPath, extraConfig) { '\\.ts$': [ require.resolve('./jestSwcTransform'), { - sourceMaps: envOptions.enableSourceMaps || envOptions.nextTests, + sourceMaps: envOptions.enableSourceMaps || !envOptions.oldTests, jsc: { parser: { syntax: 'typescript', @@ -174,7 +174,7 @@ async function getProjectConfig(targetPath, extraConfig) { '\\.tsx$': [ require.resolve('./jestSwcTransform'), { - sourceMaps: envOptions.enableSourceMaps || envOptions.nextTests, + sourceMaps: envOptions.enableSourceMaps || !envOptions.oldTests, jsc: { parser: { syntax: 'typescript', @@ -196,9 +196,9 @@ async function getProjectConfig(targetPath, extraConfig) { // A bit more opinionated testMatch: ['**/*.test.{js,jsx,ts,tsx,mjs,cjs}'], - runtime: envOptions.nextTests - ? require.resolve('./jestCachingModuleLoader') - : undefined, + runtime: envOptions.oldTests + ? undefined + : require.resolve('./jestCachingModuleLoader'), transformIgnorePatterns: [`/node_modules/(?:${transformIgnorePattern})/`], ...getRoleConfig(closestPkgJson?.backstage?.role), @@ -236,7 +236,7 @@ async function getRootConfig() { const coverageConfig = { coverageDirectory: path.resolve(targetPath, 'coverage'), - coverageProvider: envOptions.nextTests ? 'babel' : 'v8', + coverageProvider: envOptions.oldTests ? 'v8' : 'babel', collectCoverageFrom: ['**/*.{js,jsx,ts,tsx,mjs,cjs}', '!**/*.d.ts'], }; From 00196b700ca13c1a7528e9cef7826ffd1a9419cc Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Thu, 8 Dec 2022 23:11:02 +0100 Subject: [PATCH 3/4] cli: remove redundant caching from jest module loader Signed-off-by: Patrik Oldsberg --- packages/cli/config/jest.js | 5 -- .../cli/config/jestCachingModuleLoader.js | 58 ------------------- 2 files changed, 63 deletions(-) diff --git a/packages/cli/config/jest.js b/packages/cli/config/jest.js index 2c224f8161..24385a6b99 100644 --- a/packages/cli/config/jest.js +++ b/packages/cli/config/jest.js @@ -25,11 +25,6 @@ const envOptions = { enableSourceMaps: Boolean(process.env.ENABLE_SOURCE_MAPS), }; -if (!envOptions.oldTests) { - // Needed so that, at import-time, it can hook into Jest's internals. - require('./jestCachingModuleLoader'); -} - const transformIgnorePattern = [ '@material-ui', 'ajv', diff --git a/packages/cli/config/jestCachingModuleLoader.js b/packages/cli/config/jestCachingModuleLoader.js index 7858a7cfb7..95c6212123 100644 --- a/packages/cli/config/jestCachingModuleLoader.js +++ b/packages/cli/config/jestCachingModuleLoader.js @@ -14,61 +14,11 @@ * limitations under the License. */ -const fs = require('fs'); const { default: JestRuntime } = require('jest-runtime'); -const fileTransformCache = new Map(); const scriptTransformCache = new Map(); -let runtimeGeneration = 0; -let isWatchMode; - module.exports = class CachingJestRuntime extends JestRuntime { - // Each Jest run creates a new runtime, including when rerunning tests in - // watch mode. This keeps track of whether we've switched runtime instance. - __runtimeGeneration = runtimeGeneration++; - - transformFile(filename, options) { - if (!isWatchMode) { - return super.transformFile(filename, options); - } - - const entry = fileTransformCache.get(filename); - if (entry) { - // Only check modification time if it's from a different runtime generation - if (entry.generation === this.__runtimeGeneration) { - return entry.code; - } - - // Keep track of the modification time of files so that we can properly - // reprocess them in watch mode. - const { mtimeMs } = fs.statSync(filename); - if (mtimeMs > entry.mtimeMs) { - const code = super.transformFile(filename, options); - fileTransformCache.set(filename, { - code, - mtimeMs, - generation: this.__runtimeGeneration, - }); - return code; - } - - fileTransformCache.set(filename, { - ...entry, - generation: this.__runtimeGeneration, - }); - return entry.code; - } - - const code = super.transformFile(filename, options); - fileTransformCache.set(filename, { - code, - mtimeMs: fs.statSync(filename).mtimeMs, - generation: this.__runtimeGeneration, - }); - return code; - } - // This may or may not be a good idea. Theoretically I don't know why this would impact // test correctness and flakiness, but it seems like it may introduce flakiness and strange failures. // It does seem to speed up test execution by a fair amount though. @@ -84,11 +34,3 @@ module.exports = class CachingJestRuntime extends JestRuntime { return script; } }; - -// Inject hook into createHasteMap, as it's the only way that we can -// determine (from our scope here) if we're in "watch mode" or not. -const originalCreateHasteMap = JestRuntime.createHasteMap; -JestRuntime.createHasteMap = (config, options = undefined) => { - isWatchMode = options && options.watch; - return originalCreateHasteMap(config, options); -}; From 736f893f72bde44d53db1e503eba2587c9ad1880 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Thu, 8 Dec 2022 23:22:53 +0100 Subject: [PATCH 4/4] changesets: added changeset for moving to next tests by default Signed-off-by: Patrik Oldsberg --- .changeset/neat-insects-share.md | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 .changeset/neat-insects-share.md diff --git a/.changeset/neat-insects-share.md b/.changeset/neat-insects-share.md new file mode 100644 index 0000000000..1fadb9b9a1 --- /dev/null +++ b/.changeset/neat-insects-share.md @@ -0,0 +1,7 @@ +--- +'@backstage/cli': minor +--- + +The Jest configuration that was previously enabled with `BACKSTAGE_NEXT_TESTS` is now enabled by default. To revert to the old configuration you can now instead set `BACKSTAGE_OLD_TESTS`. + +This new configuration uses the `babel` coverage provider rather than `v8`. It used to be that `v8` worked better when using Sucrase for transpilation, but now that we have switched to SWC, `babel` seems to work better. In addition, the new configuration also enables source maps by default, as they no longer have a negative impact on code coverage accuracy, and it also enables a modified Jest runtime with additional caching of script objects.