From 2328431b2a1540333bf8cddfc9034c0b6c34277e Mon Sep 17 00:00:00 2001 From: NullVoxPopuli <199018+NullVoxPopuli@users.noreply.github.com> Date: Wed, 25 Jun 2025 12:03:08 -0400 Subject: [PATCH 1/4] Add build mode tests, use setTetsing for tests --- .gitignore | 2 +- .prettierignore | 1 + files/gitignore | 4 ++ files/package.json | 2 +- files/tests/test-helper.__ext__ | 2 + pnpm-lock.yaml | 20 ++++----- .../build-mode-tests/debug-utils-test.js | 33 ++++++++++++++ tests/helpers/utils.ts | 20 +++++++++ tests/package.json | 2 +- tests/smoke-tests/--typescript.test.ts | 13 +++--- tests/smoke-tests/defaults.test.ts | 43 +++++++++++++++---- 11 files changed, 116 insertions(+), 26 deletions(-) create mode 100644 tests/fixtures/build-mode-tests/debug-utils-test.js diff --git a/.gitignore b/.gitignore index 93cd668..a036415 100644 --- a/.gitignore +++ b/.gitignore @@ -1,3 +1,3 @@ /node_modules/ .log/ -/my-addon/ +my-addon/ diff --git a/.prettierignore b/.prettierignore index 0d5f7e7..84071d5 100644 --- a/.prettierignore +++ b/.prettierignore @@ -3,6 +3,7 @@ *.md files/ tests/fixtures/ +my-addon/ node_modules/ diff --git a/files/gitignore b/files/gitignore index cb26594..56a3f29 100644 --- a/files/gitignore +++ b/files/gitignore @@ -15,3 +15,7 @@ ember-cli-build.cjs node_modules/ .eslintcache .prettiercache + +# potentially containing secrets +.env +.env.*.local diff --git a/files/package.json b/files/package.json index 2c2c76d..6cd454e 100644 --- a/files/package.json +++ b/files/package.json @@ -46,7 +46,7 @@ "@embroider/addon-dev": "^8.1.0", "@embroider/compat": "^4.1.0", "@embroider/core": "^4.1.0", - "@embroider/macros": "^1.18.0", + "@embroider/macros": "^1.20.1", "@embroider/vite": "^1.1.5", "@eslint/js": "^9.17.0", "@glimmer/component": "^2.0.0<% if (typescript) { %>", diff --git a/files/tests/test-helper.__ext__ b/files/tests/test-helper.__ext__ index ac6a374..c3d5aad 100644 --- a/files/tests/test-helper.__ext__ +++ b/files/tests/test-helper.__ext__ @@ -4,6 +4,7 @@ import * as QUnit from 'qunit'; import { setApplication } from '@ember/test-helpers'; import { setup } from 'qunit-dom'; import { start as qunitStart, setupEmberOnerrorValidation } from 'ember-qunit'; +import { setTesting } from '@embroider/macros'; class Router extends EmberRouter { location = 'none'; @@ -21,6 +22,7 @@ class TestApp extends EmberApp { Router.map(function () {}); export function start() { + setTesting(true); setApplication( TestApp.create({ autoboot: false, diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 68060fd..587a640 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -37,8 +37,8 @@ importers: specifier: github:ember-cli/ember-cli#master version: https://codeload.github.com/ember-cli/ember-cli/tar.gz/8ea8eb59e8f32682c538f48f40657d708ad325cc(handlebars@4.7.8)(underscore@1.13.7) execa: - specifier: ^9.5.2 - version: 9.5.2 + specifier: ^9.6.0 + version: 9.6.0 fixturify: specifier: ^3.0.0 version: 3.0.0 @@ -1927,8 +1927,8 @@ packages: resolution: {integrity: sha512-8uSpZZocAZRBAPIEINJj3Lo9HyGitllczc27Eh5YYojjMFMn8yHMDMaUHE2Jqfq05D/wucwI4JGURyXt1vchyg==} engines: {node: '>=10'} - execa@9.5.2: - resolution: {integrity: sha512-EHlpxMCpHWSAh1dgS6bVeoLAXGnJNdR93aabr4QCGbzOM73o5XmRfM/e5FUqsw3aagP8S8XEWUWFAxnRBnAF0Q==} + execa@9.6.0: + resolution: {integrity: sha512-jpWzZ1ZhwUmeWRhS7Qv3mhpOhLfwI+uAX4e5fOcXqwMR7EcJ0pj2kV1CVzHVMX/LphnKWD3LObjZCoJ71lKpHw==} engines: {node: ^18.19.0 || >=20.5.0} exit@0.1.2: @@ -2371,8 +2371,8 @@ packages: resolution: {integrity: sha512-B4FFZ6q/T2jhhksgkbEW3HBvWIfDW85snkQgawt07S7J5QXTk6BkNV+0yAeZrM5QpMAdYlocGoljn0sJ/WQkFw==} engines: {node: '>=10.17.0'} - human-signals@8.0.0: - resolution: {integrity: sha512-/1/GPCpDUCCYwlERiYjxoczfP0zfvZMU/OWgQPMya9AbAE24vseigFdhAMObpc8Q4lc/kjutPfUddDYyAmejnA==} + human-signals@8.0.1: + resolution: {integrity: sha512-eKCa6bwnJhvxj14kZk5NCPc6Hb6BdsU9DZcOnmQKSnO1VKrfV0zCvtttPZUsBvjmNDn8rpcJfpwSYnHBjc95MQ==} engines: {node: '>=18.18.0'} humanize-ms@1.2.1: @@ -6303,13 +6303,13 @@ snapshots: signal-exit: 3.0.7 strip-final-newline: 2.0.0 - execa@9.5.2: + execa@9.6.0: dependencies: '@sindresorhus/merge-streams': 4.0.0 cross-spawn: 7.0.6 figures: 6.1.0 get-stream: 9.0.1 - human-signals: 8.0.0 + human-signals: 8.0.1 is-plain-obj: 4.1.0 is-stream: 4.0.1 npm-run-path: 6.0.0 @@ -6940,7 +6940,7 @@ snapshots: human-signals@2.1.0: {} - human-signals@8.0.0: {} + human-signals@8.0.1: {} humanize-ms@1.2.1: dependencies: @@ -7915,7 +7915,7 @@ snapshots: assert-never: 1.4.0 chalk: 5.4.1 cli-highlight: 2.1.11 - execa: 9.5.2 + execa: 9.6.0 fs-extra: 11.3.0 github-changelog: 2.0.0 js-yaml: 4.1.0 diff --git a/tests/fixtures/build-mode-tests/debug-utils-test.js b/tests/fixtures/build-mode-tests/debug-utils-test.js new file mode 100644 index 0000000..a7d1dec --- /dev/null +++ b/tests/fixtures/build-mode-tests/debug-utils-test.js @@ -0,0 +1,33 @@ +import { test, module } from 'qunit'; +import { assert } from '@ember/debug'; +import { DEBUG } from '@glimmer/env'; +import { isDevelopingApp, isTesting } from '@embroider/macros'; + +module('debug utils remain in the build', function () { + test('assert', function(qAssert) { + // If we get the build mode wrong, e.g.: `NODE_ENV` != 'development' + // then the assert won't exist, causing qAssert to not detect a thrown Error + qAssert.throws(() => { + assert('should throw'); + }, /should throw/, `The error "should throw" is thrown`); + }); + + test('isTesting', function (assert) { + assert.strictEqual(isTesting(), true, `isTesting() === true`); + }); + + test('isDevelopingApp', function (assert) { + assert.strictEqual(isDevelopingApp(), true, `isDevelopingApp() === true`); + }); + + + module('not supported', function () { + test('DEBUG', function (assert) { + if (DEBUG) { + assert.step('DEBUG'); + } + + assert.verifySteps([]); + }); + }); +}); diff --git a/tests/helpers/utils.ts b/tests/helpers/utils.ts index 89fa619..6cf8403 100644 --- a/tests/helpers/utils.ts +++ b/tests/helpers/utils.ts @@ -48,6 +48,26 @@ export async function createTmp() { return tmpDirPath; } +/** + * Returns a copy of the current process env with EMBER_, VITE_, and NODE_ + * prefixed vars removed. + * + * Useful when spawning child processes with `extendEnv: false` so that + * vitest's NODE_ENV=test (and similar) doesn't leak into the child and + * interfere with the build-time / runtime macro mode detection. + */ +export function safeExecaEnv(): Record { + let env = { ...process.env }; + + for (let key of Object.keys(env)) { + if (key.startsWith('EMBER_') || key.startsWith('VITE_') || key.startsWith('NODE_')) { + delete env[key]; + } + } + + return env; +} + /** * Abstraction for install, as the blueprint supports multiple package managers */ diff --git a/tests/package.json b/tests/package.json index e42c56c..a9f28e2 100644 --- a/tests/package.json +++ b/tests/package.json @@ -10,7 +10,7 @@ "@vitest/ui": "^0.18.0", "c8": "^7.11.3", "ember-cli": "github:ember-cli/ember-cli#master", - "execa": "^9.5.2", + "execa": "^9.6.0", "fixturify": "^3.0.0", "fs-extra": "^10.0.0", "globby": "^14.1.0", diff --git a/tests/smoke-tests/--typescript.test.ts b/tests/smoke-tests/--typescript.test.ts index 40af50a..a787a55 100644 --- a/tests/smoke-tests/--typescript.test.ts +++ b/tests/smoke-tests/--typescript.test.ts @@ -7,9 +7,9 @@ import { beforeAll, beforeEach, describe, expect, it } from 'vitest'; import { assertGeneratedCorrectly, - dirContents, filesMatching, SUPPORTED_PACKAGE_MANAGERS, + safeExecaEnv, } from '../helpers.js'; const blueprintPath = path.join(__dirname, '../..'); let localEmberCli = require.resolve('ember-cli/bin/ember'); @@ -25,6 +25,8 @@ for (let packageManager of SUPPORTED_PACKAGE_MANAGERS) { cwd: addonDir, shell: true, preferLocal: true, + extendEnv: false, + env: safeExecaEnv(), // Allows us to not fail yet when the command fails // but we'd still fail appropriately with the exitCode check below. // When we fail, we want to check for git diffs for debugging purposes. @@ -50,13 +52,14 @@ for (let packageManager of SUPPORTED_PACKAGE_MANAGERS) { addonDir = join(tmpDir, addonName); await execa({ cwd: tmpDir, + extendEnv: false, })`${localEmberCli} addon ${addonName} -b ${blueprintPath} --skip-npm --prefer-local true --${packageManager} --typescript`; // Have to use --force because NPM is *stricter* when you use tags in package.json // than pnpm (in that tags don't match any specified stable version) if (packageManager === 'npm') { - await execa({ cwd: addonDir })`npm install --force`; + await execa({ cwd: addonDir, extendEnv: false })`npm install --force`; } else if (packageManager === 'pnpm') { - await execa({ cwd: addonDir })`pnpm install`; + await execa({ cwd: addonDir, extendEnv: false })`pnpm install`; } }); @@ -92,7 +95,7 @@ for (let packageManager of SUPPORTED_PACKAGE_MANAGERS) { await commandSucceeds(`${packageManager} run build`); expect( - await filesMatching('src/**', addonDir), + (await filesMatching('src/**', addonDir)).sort(), `ensure we don't pollute the src dir with declarations and emit the js and .d.ts to the correct folders -- this should be the same as the input files (no change from the fixture + default files)`, ).toMatchInlineSnapshot(` [ @@ -107,7 +110,7 @@ for (let packageManager of SUPPORTED_PACKAGE_MANAGERS) { `); expect( - await filesMatching('{dist,declarations}/**/*', addonDir), + (await filesMatching('{dist,declarations}/**/*', addonDir)).sort(), `ensure we emit the correct files out of the box to the correct folders`, ).toMatchInlineSnapshot(` [ diff --git a/tests/smoke-tests/defaults.test.ts b/tests/smoke-tests/defaults.test.ts index 80f6571..970191d 100644 --- a/tests/smoke-tests/defaults.test.ts +++ b/tests/smoke-tests/defaults.test.ts @@ -14,6 +14,7 @@ import { dirContents, matchesFixture, packageJsonAt, + safeExecaEnv, SUPPORTED_PACKAGE_MANAGERS, } from '../helpers.js'; import { existsSync } from 'node:fs'; @@ -38,8 +39,9 @@ for (let packageManager of SUPPORTED_PACKAGE_MANAGERS) { addonDir = join(tmpDir, addonName); await execa({ cwd: tmpDir, + extendEnv: false, })`${localEmberCli} addon ${addonName} -b ${blueprintPath} --skip-npm --skip-git --prefer-local true --${packageManager}`; - await execa({ cwd: addonDir })`${packageManager} install`; + await execa({ cwd: addonDir, extendEnv: false })`${packageManager} install`; }); it('is using the correct packager', async () => { @@ -99,13 +101,16 @@ for (let packageManager of SUPPORTED_PACKAGE_MANAGERS) { // Tests are additive, so when running them in order, we want to check linting // before we add files from fixtures it('lints with no fixtures all pass', async () => { - let { exitCode } = await execa({ cwd: addonDir })`pnpm lint`; + let { exitCode } = await execa({ cwd: addonDir, extendEnv: false })`pnpm lint`; expect(exitCode).toEqual(0); }); it('lint:fix with no fixtures', async () => { - let { exitCode } = await execa({ cwd: addonDir })`${packageManager} run lint:fix`; + let { exitCode } = await execa({ + cwd: addonDir, + extendEnv: false, + })`${packageManager} run lint:fix`; expect(exitCode).toEqual(0); }); @@ -124,16 +129,27 @@ for (let packageManager of SUPPORTED_PACKAGE_MANAGERS) { let testFixture = fixturify.readSync('./fixtures/rendering-tests'); fixturify.writeSync(join(addonDir, 'tests/rendering'), testFixture); + + fixturify.writeSync( + join(addonDir, 'tests/unit'), + fixturify.readSync('./fixtures/build-mode-tests'), + ); }); it('lint:fix', async () => { - let { exitCode } = await execa({ cwd: addonDir })`${packageManager} run lint:fix`; + let { exitCode } = await execa({ + cwd: addonDir, + extendEnv: false, + })`${packageManager} run lint:fix`; expect(exitCode).toEqual(0); }); it('build', async () => { - let buildResult = await execa({ cwd: addonDir })`${packageManager} run build`; + let buildResult = await execa({ + cwd: addonDir, + extendEnv: false, + })`${packageManager} run build`; expect(buildResult.exitCode).toEqual(0); @@ -146,13 +162,24 @@ for (let packageManager of SUPPORTED_PACKAGE_MANAGERS) { // It's important that we ensure that dist directory is empty for this test, because await fs.rm(join(addonDir, 'dist'), { recursive: true, force: true }); - let testResult = await execa({ cwd: addonDir })`${packageManager} run test`; + let testResult = await execa({ + cwd: addonDir, + extendEnv: false, + // a modified env required with extendEnv, else we still inherit/merge the env of vitest + // which overrides our NODE_ENV from our .env.development file (read by vite) + // (because in-shell ENV vars override .env files) + env: safeExecaEnv(), + })`${packageManager} run test`; expect(testResult.exitCode).toEqual(0); + expect(testResult.stdout).includes('debug utils remain in the build: assert'); + expect(testResult.stdout).includes( + 'debug utils remain in the build > not supported: DEBUG', + ); expect(testResult.stdout).includes( - `# tests 2 -# pass 2 + `# tests 6 +# pass 6 # skip 0 # todo 0 # fail 0 From 820056adbbd6739de419bcbf98557c39ca32543d Mon Sep 17 00:00:00 2001 From: NullVoxPopuli <199018+NullVoxPopuli@users.noreply.github.com> Date: Wed, 4 Mar 2026 14:20:34 -0500 Subject: [PATCH 2/4] Force the NODE_ENV for the try scenarios test --- tests/smoke-tests/try-scenarios.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/smoke-tests/try-scenarios.test.ts b/tests/smoke-tests/try-scenarios.test.ts index ab7461e..83b9d1c 100644 --- a/tests/smoke-tests/try-scenarios.test.ts +++ b/tests/smoke-tests/try-scenarios.test.ts @@ -23,7 +23,7 @@ describe('try-scenarios', () => { let addonDir = join(tmpDir, addonName); function runInAddon(command: string) { - const env = { ...scenario.env }; + const env = { ...scenario.env, NODE_ENV: 'development' }; console.log(`Running \`${command}\` with ${JSON.stringify(env)}`); return execa({ shell: true, preferLocal: true, cwd: addonDir, env })(command); } From b06c2c6042e59a7e8467861eee7ea72fdbcad814 Mon Sep 17 00:00:00 2001 From: NullVoxPopuli <199018+NullVoxPopuli@users.noreply.github.com> Date: Tue, 10 Mar 2026 10:26:15 -0400 Subject: [PATCH 3/4] Gitignore should say *.local --- files/gitignore | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/files/gitignore b/files/gitignore index 56a3f29..0049e30 100644 --- a/files/gitignore +++ b/files/gitignore @@ -17,5 +17,4 @@ node_modules/ .prettiercache # potentially containing secrets -.env -.env.*.local +*.local From b36a28e65f7ae229572831b154b6e8ecce62f42a Mon Sep 17 00:00:00 2001 From: NullVoxPopuli <199018+NullVoxPopuli@users.noreply.github.com> Date: Tue, 10 Mar 2026 11:32:27 -0400 Subject: [PATCH 4/4] Update test snapshots --- tests/smoke-tests/--typescript.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/smoke-tests/--typescript.test.ts b/tests/smoke-tests/--typescript.test.ts index a787a55..aa1d5a2 100644 --- a/tests/smoke-tests/--typescript.test.ts +++ b/tests/smoke-tests/--typescript.test.ts @@ -95,7 +95,7 @@ for (let packageManager of SUPPORTED_PACKAGE_MANAGERS) { await commandSucceeds(`${packageManager} run build`); expect( - (await filesMatching('src/**', addonDir)).sort(), + await filesMatching('src/**', addonDir), `ensure we don't pollute the src dir with declarations and emit the js and .d.ts to the correct folders -- this should be the same as the input files (no change from the fixture + default files)`, ).toMatchInlineSnapshot(` [ @@ -110,7 +110,7 @@ for (let packageManager of SUPPORTED_PACKAGE_MANAGERS) { `); expect( - (await filesMatching('{dist,declarations}/**/*', addonDir)).sort(), + await filesMatching('{dist,declarations}/**/*', addonDir), `ensure we emit the correct files out of the box to the correct folders`, ).toMatchInlineSnapshot(` [