Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

embroider ember-try scenarios #308

Merged
merged 3 commits into from
Aug 17, 2021
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .github/workflows/ci-build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ jobs:
- ember-canary
- ember-classic
- ember-default-with-jquery
- embroider-safe

steps:
- uses: actions/checkout@v2
Expand Down
53 changes: 53 additions & 0 deletions config/ember-try.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,45 @@

const getChannelURL = require('ember-source-channel-url');

const EMBROIDER_VERSION = '^0.43.4';
const embroider = {
safe: {
name: 'embroider-safe',
npm: {
devDependencies: {
'@embroider/core': EMBROIDER_VERSION,
'@embroider/webpack': EMBROIDER_VERSION,
'@embroider/compat': EMBROIDER_VERSION,
'@embroider/test-setup': EMBROIDER_VERSION,

// Webpack is a peer dependency of `@embroider/webpack`
webpack: '^5.0.0',
},
},
env: {
EMBROIDER_TEST_SETUP_OPTIONS: 'safe',
},
},

optimized: {
name: 'embroider-optimized',
npm: {
devDependencies: {
'@embroider/core': EMBROIDER_VERSION,
'@embroider/webpack': EMBROIDER_VERSION,
'@embroider/compat': EMBROIDER_VERSION,
'@embroider/test-setup': EMBROIDER_VERSION,

// Webpack is a peer dependency of `@embroider/webpack`
webpack: '^5.0.0',
},
},
env: {
EMBROIDER_TEST_SETUP_OPTIONS: 'optimized',
},
},
};

module.exports = async function () {
return {
useYarn: true,
Expand Down Expand Up @@ -103,6 +142,20 @@ module.exports = async function () {
devDependencies: {},
},
},
embroider.safe,
// disable embroider optimized test scenarios, as the dynamism these
// tests use is not compatible with embroider we are still exploring
// appropriate paths forward.
//
// Steps to re-enable:
//
// 1. have a strategy to make this work
// 2. uncomment the next line
// embroider.optimized,
//
// 3. add "embroider-optimized" to .github/workflows/ci-build.yml's
// ember-try-scenario list.
//
],
};
};
25 changes: 13 additions & 12 deletions ember-cli-build.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,21 +3,22 @@
const EmberAddon = require('ember-cli/lib/broccoli/ember-addon');

module.exports = function (defaults) {
let app = new EmberAddon(defaults, {
autoImport: {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we don't need this, if we include qunit via auto-import

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the reason we had this here was that we weren't sure of broad auto-import support at the time of writing. Makes sense if it's better supported now.

exclude: ['qunit'],
},
});

app.import('node_modules/qunit/qunit/qunit.js', {
type: 'test',
});
let app = new EmberAddon(defaults);

app.import('node_modules/qunit/qunit/qunit.css', {
type: 'test',
});

app.import('vendor/shims/qunit.js', { type: 'test' });
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use auto-import instead


return app.toTree();
try {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is verbose, because this library cannot yet drop node 10 and instead requires @embroider/test-support be provided via the ember-try-scenario configuration

const { maybeEmbroider } = require('@embroider/test-setup'); // eslint-disable-line node/no-missing-require
return maybeEmbroider(app);
} catch (e) {
// This exists, so that we can continue to support node 10 for some of our
// test scenarios. Specifically those not scenario testing embroider. As
// @embroider/test-setup and @embroider in no longer supports node 10
if (e !== null && typeof e === 'object' && e.code === 'MODULE_NOT_FOUND') {
return app.toTree();
}
throw e;
}
};
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,6 @@
"ember-source": "~3.24.1",
"ember-source-channel-url": "^3.0.0",
"ember-try": "^1.4.0",
"es6-promise": "^4.2.8",
"eslint": "^7.32.0",
"eslint-config-prettier": "^8.3.0",
"eslint-plugin-ember": "^7.8.1",
Expand All @@ -85,6 +84,7 @@
"loader.js": "^4.7.0",
"npm-run-all": "^4.1.5",
"prettier": "^2.2.1",
"promise-polyfill": "^8.2.0",
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. now matches @ember/test-helpers polyfill
  2. works around ie11 + es6-promise + babel + webpack "glitch" that causes some fun to debug issues.

"qunit": "^2.16.0",
"release-it": "^14.11.5",
"release-it-lerna-changelog": "^3.1.0",
Expand Down
16 changes: 16 additions & 0 deletions tests/dummy/vendor/ember-cli/test-support-suffix.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// eslint-disable-next-line no-unused-vars, @typescript-eslint/no-unused-vars
/* global runningTests: true */

/*
used to determine if the application should be booted immediately when `app-name.js` is evaluated
when `runningTests` the `app-name.js` file will **not** import the applications `app/app.js` and
call `Application.create(...)` on it. Additionally, applications can opt-out of this behavior by
setting `autoRun` to `false` in their `ember-cli-build.js`
*/
runningTests = true;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mimic @ember/test-helpers

/*
This file overrides a file built into ember-cli's build pipeline and prevents
this built-in `Testem.hookIntoTestFramework` invocation:

https://github.com/ember-cli/ember-cli/blob/v3.20.0/lib/broccoli/test-support-suffix.js#L3-L5
*/
10 changes: 5 additions & 5 deletions tests/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,13 @@
{{content-for "test-body"}}

<div id="qunit"></div>
<div id="qunit-fixture"></div>

<div id="ember-testing-container">
<div id="ember-testing"></div>
<div id="qunit-fixture">
<div id="ember-testing-container">
<div id="ember-testing"></div>
</div>
</div>

<script src="/testem.js" integrity=""></script>
<script src="/testem.js" integrity="" data-embroider-ignore></script>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does this do?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This ensures embroider doesn't warn that it can't find this, as this is a dynamic files provided by a middleware which shouldn't and cannot be bundled.

<script src="{{rootURL}}assets/vendor.js"></script>
<script src="{{rootURL}}assets/test-support.js"></script>
<script src="{{rootURL}}assets/dummy.js"></script>
Expand Down
12 changes: 9 additions & 3 deletions tests/test-helper.js
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
/* globals Testem */

import QUnit from 'qunit';
import AbstractTestLoader from 'ember-cli-test-loader/test-support/index';
import { polyfill } from 'es6-promise';

import PromisePolyfill from 'promise-polyfill';
// When running under IE11, our tests are transpiled to use `Promise` (due to
// asyncToGenerator helper in Babel)
if (typeof Promise === 'undefined') {
polyfill();
self.Promise = PromisePolyfill;
}

let moduleLoadFailures = [];
Expand Down Expand Up @@ -34,3 +35,8 @@ QUnit.testDone(function () {
let testElementReset = testElementContainer.outerHTML;
testElementContainer.innerHTML = testElementReset;
});

QUnit.start();
if (typeof Testem !== 'undefined') {
Testem.hookIntoTestFramework();
}
Empty file removed vendor/.gitkeep
Empty file.
18 changes: 0 additions & 18 deletions vendor/shims/qunit.js

This file was deleted.

7 changes: 6 additions & 1 deletion yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -6733,7 +6733,7 @@ es-to-primitive@^1.2.1:
is-date-object "^1.0.1"
is-symbol "^1.0.2"

es6-promise@^4.0.3, es6-promise@^4.2.8:
es6-promise@^4.0.3:
version "4.2.8"
resolved "https://registry.yarnpkg.com/es6-promise/-/es6-promise-4.2.8.tgz#4eb21594c972bc40553d276e510539143db53e0a"
integrity sha512-HJDGx5daxeIvxdBxvG2cb9g4tEvwIk3i8+nhX0yGrYmZUzbkdg8QbDevheDB8gd0//uPj4c1EQua8Q+MViT0/w==
Expand Down Expand Up @@ -11286,6 +11286,11 @@ promise-map-series@^0.3.0:
resolved "https://registry.yarnpkg.com/promise-map-series/-/promise-map-series-0.3.0.tgz#41873ca3652bb7a042b387d538552da9b576f8a1"
integrity sha512-3npG2NGhTc8BWBolLLf8l/92OxMGaRLbqvIh9wjCHhDXNvk4zsxaTaCpiCunW09qWPrN2zeNSNwRLVBrQQtutA==

promise-polyfill@^8.2.0:
version "8.2.0"
resolved "https://registry.yarnpkg.com/promise-polyfill/-/promise-polyfill-8.2.0.tgz#367394726da7561457aba2133c9ceefbd6267da0"
integrity sha512-k/TC0mIcPVF6yHhUvwAp7cvL6I2fFV7TzF1DuGPI8mBh4QQazf36xCKEHKTZKRysEoTQoQdKyP25J8MPJp7j5g==

promise-retry@^1.1.1:
version "1.1.1"
resolved "https://registry.yarnpkg.com/promise-retry/-/promise-retry-1.1.1.tgz#6739e968e3051da20ce6497fb2b50f6911df3d6d"
Expand Down