Skip to content

New command respects module unification experiment#7869

Merged
rwjblue merged 4 commits intoember-cli:masterfrom
cyk:respect-mu-flag-exp
Jun 10, 2018
Merged

New command respects module unification experiment#7869
rwjblue merged 4 commits intoember-cli:masterfrom
cyk:respect-mu-flag-exp

Conversation

@cyk
Copy link
Copy Markdown
Contributor

@cyk cyk commented Jun 8, 2018

This removes the need for specifying the additional MODULE_UNIFICATION env for the new command by respecting the new Experiments feature toggling.

It also exposes gaps in MU testing and some needed fixture verification.

(Thank you @mixonic for your help today.)

@cyk cyk changed the title Respect MU feature from Experiments (remove MODULE_UNIFICATION env) Replace MODULE_UNIFICATION env w/ Experiment for commands/new Jun 8, 2018
@cyk cyk changed the title Replace MODULE_UNIFICATION env w/ Experiment for commands/new [WIP] New command respects module unification experiment Jun 9, 2018
cyk added 3 commits June 9, 2018 14:33
* Make confirmBlueprinted app specific and MU aware in acceptance/new
* Guard or skip for other failing tests when is MU enabled
@cyk cyk force-pushed the respect-mu-flag-exp branch from e2695ef to 9d628e5 Compare June 9, 2018 21:35
@cyk cyk force-pushed the respect-mu-flag-exp branch from fb9dd87 to 4c4a656 Compare June 9, 2018 23:23

if (experiments.MODULE_UNIFICATION) {
describe('Acceptance: addon-mu-smoke-test', function() {
describe.skip('Acceptance: addon-mu-smoke-test', function() {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

just pointing out, this .skip will need to be removed before landing

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure what to do about this. It seems that addon-mu-smoke-test-slow fails for MU. Prior to this PR, MODULE_UNIFICATION env was never set, so the new command in createTestTargets would actually generate a classic app. Outside of skipping, I can either comment it out or just remove it completely?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

OK, I see. Thank you for explaining! Can you create an issue (cross-linking this comment thread and linking to this describe.skip) so we can track fixing that separately?

@cyk cyk changed the title [WIP] New command respects module unification experiment New command respects module unification experiment Jun 10, 2018
@rwjblue rwjblue merged commit ea35885 into ember-cli:master Jun 10, 2018
@rwjblue
Copy link
Copy Markdown
Member

rwjblue commented Jun 10, 2018

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants