From 051ade93c9ee5bd4e9849c2dd1639b309e187905 Mon Sep 17 00:00:00 2001 From: Peter Meehan Date: Mon, 8 May 2023 10:33:25 +0100 Subject: [PATCH 1/5] WriteV1Config: fix /tests support --- packages/compat/src/v1-app.ts | 7 ++++++- packages/compat/src/v1-config.ts | 15 ++++++++++++--- 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/packages/compat/src/v1-app.ts b/packages/compat/src/v1-app.ts index 107e52d37..a816475ef 100644 --- a/packages/compat/src/v1-app.ts +++ b/packages/compat/src/v1-app.ts @@ -149,6 +149,11 @@ export default class V1App { return new V1Config(this.configTree, this.app.env); } + @Memoize() + get testConfig(): V1Config { + return new V1Config(this.configTree, 'test'); + } + get autoRun(): boolean { return this.app.options.autoRun; } @@ -659,7 +664,7 @@ export default class V1App { let appTree = this.appTree; let testsTree = this.testsTree; let lintTree = this.lintTree; - let config = new WriteV1Config(this.config, this.storeConfigInMeta); + let config = new WriteV1Config(this.config, this.storeConfigInMeta, this.testConfig); let patterns = this.configReplacePatterns; let configReplaced = new this.configReplace(config, this.configTree, { configPath: join('environments', `${this.app.env}.json`), diff --git a/packages/compat/src/v1-config.ts b/packages/compat/src/v1-config.ts index fe20f27eb..5329704a0 100644 --- a/packages/compat/src/v1-config.ts +++ b/packages/compat/src/v1-config.ts @@ -30,8 +30,8 @@ export class V1Config extends Plugin { export class WriteV1Config extends Plugin { private lastContents: string | undefined; - constructor(private inputTree: V1Config, private storeConfigInMeta: boolean) { - super([inputTree], { + constructor(private inputTree: V1Config, private storeConfigInMeta: boolean, private testInputTree?: V1Config) { + super([inputTree, testInputTree as V1Config], { persistentOutput: true, needsCache: false, }); @@ -42,7 +42,16 @@ export class WriteV1Config extends Plugin { if (this.storeConfigInMeta) { contents = metaLoader(); } else { - contents = `export default ${JSON.stringify(this.inputTree.readConfig())};`; + contents = ` + import { isTesting } from '@embroider/macros'; + let env; + if (isTesting()) { + env = ${JSON.stringify(this.testInputTree?.readConfig())}; + } else { + env = ${JSON.stringify(this.inputTree.readConfig())}; + } + export default env; + `; } if (!this.lastContents || this.lastContents !== contents) { outputFileSync(filename, contents); From 132a1d1856aad1d569f081fc3b2c74003ea34442 Mon Sep 17 00:00:00 2001 From: Peter Meehan Date: Mon, 8 May 2023 11:01:11 +0100 Subject: [PATCH 2/5] only provide testConfigTree if shouldBuildTests --- packages/compat/src/v1-app.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/compat/src/v1-app.ts b/packages/compat/src/v1-app.ts index a816475ef..80d7643c7 100644 --- a/packages/compat/src/v1-app.ts +++ b/packages/compat/src/v1-app.ts @@ -150,8 +150,10 @@ export default class V1App { } @Memoize() - get testConfig(): V1Config { - return new V1Config(this.configTree, 'test'); + get testConfig(): V1Config | undefined { + if (this.shouldBuildTests) { + return new V1Config(this.configTree, 'test'); + } } get autoRun(): boolean { From a42e6ff95fe69d26f570fa2ccaec4f686c76cad7 Mon Sep 17 00:00:00 2001 From: Peter Meehan Date: Mon, 8 May 2023 14:51:05 +0100 Subject: [PATCH 3/5] don't pass undefined tree to broccoli plugin --- packages/compat/src/v1-config.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/compat/src/v1-config.ts b/packages/compat/src/v1-config.ts index 5329704a0..e9c52e4ed 100644 --- a/packages/compat/src/v1-config.ts +++ b/packages/compat/src/v1-config.ts @@ -31,7 +31,7 @@ export class V1Config extends Plugin { export class WriteV1Config extends Plugin { private lastContents: string | undefined; constructor(private inputTree: V1Config, private storeConfigInMeta: boolean, private testInputTree?: V1Config) { - super([inputTree, testInputTree as V1Config], { + super([inputTree, testInputTree as V1Config].filter(Boolean), { persistentOutput: true, needsCache: false, }); From 2949e9b14fa4c901c9300f807b4441b3a33ef2db Mon Sep 17 00:00:00 2001 From: Peter Meehan Date: Tue, 6 Jun 2023 16:43:26 +0100 Subject: [PATCH 4/5] Add test scenario for `tests: true` `storeConfigInMeta: false` config (#1) * add skeleton scenario with TODO comments * implementing Ed's higher-order test recommendation * fix locationType for tests --- .../scenarios/app-config-environment-test.ts | 81 +++++++++++++++++++ 1 file changed, 81 insertions(+) create mode 100644 tests/scenarios/app-config-environment-test.ts diff --git a/tests/scenarios/app-config-environment-test.ts b/tests/scenarios/app-config-environment-test.ts new file mode 100644 index 000000000..14e4f3fce --- /dev/null +++ b/tests/scenarios/app-config-environment-test.ts @@ -0,0 +1,81 @@ +import merge from 'lodash/merge'; +import { appScenarios } from './scenarios'; +import { PreparedApp } from 'scenario-tester'; +import QUnit from 'qunit'; + +const { module: Qmodule, test } = QUnit; + +appScenarios + .only('release') + .map('app-config-environment', project => { + merge(project.files, { + 'ember-cli-build.js': ` + 'use strict'; + + const EmberApp = require('ember-cli/lib/broccoli/ember-app'); + const { maybeEmbroider } = require('@embroider/test-setup'); + + module.exports = function (defaults) { + let app = new EmberApp(defaults, { + tests: true, + storeConfigInMeta: false, + }); + return maybeEmbroider(app, {}); + }; + `, + config: { + 'environment.js': `module.exports = function(environment) { + // DEFAULT config/environment.js + let ENV = { + modulePrefix: 'my-app', + environment, + rootURL: '/', + locationType: 'history', + EmberENV: { + EXTEND_PROTOTYPES: false, + FEATURES: {}, + }, + APP: {}, + }; + + if (environment === 'test') { + ENV.locationType = 'none'; + ENV.APP.LOG_ACTIVE_GENERATION = false; + ENV.APP.LOG_VIEW_LOOKUPS = false; + ENV.APP.rootElement = '#ember-testing'; + ENV.APP.autoboot = false; + }; + + // CUSTOM + ENV.someCustomField = true; + return ENV; + };`, + }, + tests: { + unit: { + 'store-config-in-meta-test.js': ` + import { module, test } from 'qunit'; + import ENV from 'app-template/config/environment'; + + module('Unit | storeConfigInMeta', function (hooks) { + test('it has loaded the correct config values', async function (assert) { + assert.equal(ENV.someCustomField, true); + }); + });`, + }, + }, + }); + }) + .forEachScenario(scenario => { + Qmodule(scenario.name, function (hooks) { + let app: PreparedApp; + hooks.before(async () => { + app = await scenario.prepare(); + }); + + test(`yarn test ran with custom unit test`, async function (assert) { + let result = await app.execute(`yarn test`); + assert.equal(result.exitCode, 0, result.output); + }); + }); + }); From d61234fd9fafa25231b8fc0481ba7f548a1e46ed Mon Sep 17 00:00:00 2001 From: Peter Meehan Date: Tue, 6 Jun 2023 17:01:55 +0100 Subject: [PATCH 5/5] don't include runtime conditional if we don't have a test tree --- packages/compat/src/v1-config.ts | 20 +++++++++++-------- .../scenarios/app-config-environment-test.ts | 20 +++++++++++++------ 2 files changed, 26 insertions(+), 14 deletions(-) diff --git a/packages/compat/src/v1-config.ts b/packages/compat/src/v1-config.ts index e9c52e4ed..62d5ae3ac 100644 --- a/packages/compat/src/v1-config.ts +++ b/packages/compat/src/v1-config.ts @@ -42,16 +42,20 @@ export class WriteV1Config extends Plugin { if (this.storeConfigInMeta) { contents = metaLoader(); } else { - contents = ` - import { isTesting } from '@embroider/macros'; - let env; - if (isTesting()) { - env = ${JSON.stringify(this.testInputTree?.readConfig())}; + if (this.testInputTree) { + contents = ` + import { isTesting } from '@embroider/macros'; + let env; + if (isTesting()) { + env = ${JSON.stringify(this.testInputTree.readConfig())}; + } else { + env = ${JSON.stringify(this.inputTree.readConfig())}; + } + export default env; + `; } else { - env = ${JSON.stringify(this.inputTree.readConfig())}; + contents = `export default ${JSON.stringify(this.inputTree.readConfig())};`; } - export default env; - `; } if (!this.lastContents || this.lastContents !== contents) { outputFileSync(filename, contents); diff --git a/tests/scenarios/app-config-environment-test.ts b/tests/scenarios/app-config-environment-test.ts index 14e4f3fce..bbf76ca7e 100644 --- a/tests/scenarios/app-config-environment-test.ts +++ b/tests/scenarios/app-config-environment-test.ts @@ -44,10 +44,11 @@ appScenarios ENV.APP.LOG_VIEW_LOOKUPS = false; ENV.APP.rootElement = '#ember-testing'; ENV.APP.autoboot = false; + + // CUSTOM + ENV.someCustomField = true; }; - // CUSTOM - ENV.someCustomField = true; return ENV; };`, }, @@ -57,7 +58,7 @@ appScenarios import { module, test } from 'qunit'; import ENV from 'app-template/config/environment'; - module('Unit | storeConfigInMeta', function (hooks) { + module('Unit | storeConfigInMeta set to false', function (hooks) { test('it has loaded the correct config values', async function (assert) { assert.equal(ENV.someCustomField, true); }); @@ -73,9 +74,16 @@ appScenarios app = await scenario.prepare(); }); - test(`yarn test ran with custom unit test`, async function (assert) { - let result = await app.execute(`yarn test`); - assert.equal(result.exitCode, 0, result.output); + test(`ember test ran against dev build with custom unit test`, async function (assert) { + // here we build the app with environment set to dev so that we can use + // the build output directory as the input path to an `ember test` run + // later. This difference in environment is important because it's the + // only way for us to test ember-cli-build.js' `tests: true` behavior, + // and is equivalent to visiting the app's /tests page + let devBuildResult = await app.execute(`pnpm build --environment=development`); + assert.equal(devBuildResult.exitCode, 0, devBuildResult.output); + let testRunResult = await app.execute(`pnpm test:ember --path dist`); + assert.equal(testRunResult.exitCode, 0, testRunResult.output); }); }); });