Add MU support to blueprints test helpers by dcyriller · Pull Request #153 · ember-cli/ember-cli-blueprint-test-helpers

dcyriller · 2018-07-05T11:25:25Z

This PR unlocks the ability to do:
emberNew({ isModuleUnification: true })
or
emberGenerateDestroy({ isModuleUnification: true })

It would be especially usefull for testing blueprints in ember.js and ember-data repos + addons.

dcyriller · 2018-07-05T12:18:50Z

🤔 the tests failure doesn't seem to be related to the PR

Turbo87 · 2018-07-05T12:37:54Z

can you add tests for that?

dcyriller · 2018-07-05T15:18:57Z

Sure, I'll do it. I have to figure out how to test emberNew.

dcyriller · 2018-07-05T17:06:34Z

Okay @Turbo87 I added a test for that.

Turbo87

thanks for working on this @dcyriller! the change itself looks fine to me, but I'd prefer if we could extract the unrelated CI changes to a dedicated PR.

Turbo87 · 2018-07-10T10:28:07Z

.travis.yml

+  - '10'
+  - '8'
  - '6'
-  - '4'


can you extract these changes to a separate PR? if we bump the required Node versions then it's a breaking change and deserves a dedicated PR so that we can label it properly in the changelog.

dcyriller · 2018-07-10T11:40:46Z

I updated the PR, thanks for the review @Turbo87

Here is the link to the CI update: #155

It was required since this PR upgrades ember-cli to v3.2 (do not support nodejs v4 anymore).

runspired · 2018-07-10T16:40:47Z

Just wanted to drop by and say thanks to @dcyriller for doing this :) Looking forward to having MU blueprints for ember-data :D

rwjblue · 2018-07-11T13:50:05Z

lib/ember-new.js

    disableDependencyChecker: true
  };

+  process.env["MODULE_UNIFICATION"] = !!options.isModuleUnification;


Should be EMBER_CLI_MODULE_UNIFICATION I think 🤔...

The tests I added are failing when updating to EMBER_CLI_MODULE_UNIFICATION. I'll investigate why.

Okay, I get it! ember-cli/ember-cli#7869 is still on ember-cli's beta branch. Next ember-cli release, it should be good. :)

mixonic · 2018-07-23T16:25:37Z

Had some discussion about this with @rwjblue at our meeting this morning. The generators need to test a matrix of things:

If the module unification feature flag for Ember CLI is enabled, new projects with generate with a src/.
Existing projects with an app/ should continue to use classic blueprints regardless of if the feature flag is enabled. If there is a src/ folder, then the mu blueprints should be used.

The test helper needs to have a way to test both of these things (flag enabled or not, type of directory).

Edit: Upon some further discussion!

Let's make sure the ENV value gets cleaned up at the end of each test.
Let's also add these arguments to emberGenerate and emberGenerateDestroy and friends. Basically you will want to be able to run emberNew().then(() => emberGenerateDestroy({isModuleUnification: true})) and assert that the classic blueprints work (b/c the app only has an app/ dir, since it was generated as a classic app).

dcyriller · 2018-07-23T17:17:21Z

Ok @mixonic I'll update the PR to cover these different scenarios soon.

Something remains unclear to me though. It looks to me that experiments are enabled on ember-cli's master branch only. I mean, I can't use them from released ember-cli versions. How will I be able to set EMBER_CLI_MODULE_UNIFICATION variable?

dcyriller · 2018-08-06T13:45:36Z

Taken individually, all tests are passing! 🎉

But because they are run on the same process, they fail when run in sequence.

Indeed, the experiments object is put in cache by nodejs runtime. And when execution comes to the MU part of the tests, it fails because it resolves experiments.MODULE_UNIFICATION to undefined.

A solution would be to turn this object into a function. I implemented it in ember-cli/ember-cli#7961

What do you think @rwjblue?

dcyriller · 2018-08-08T07:30:59Z

If using ember-cli canary is not too much of an issue, the PR is now ready for review! All tests are passing.

dcyriller · 2018-08-08T12:31:32Z

An example of usage here: warp-drive-data/warp-drive#5558

dcyriller · 2018-08-16T07:40:44Z

@rwjblue @mixonic if you have time for this, your reviews are welcomed :)

This would unlock MU support for ember-data blueprints (+ prepare the ground to improve ember-source blueprints testing).

mixonic · 2018-09-11T13:19:59Z

package.json

  },
  "devDependencies": {
-    "ember-cli": "~2.16.0",
+    "ember-cli": "github:ember-cli/ember-cli",


Can this reference a specific version, or does it still need to be pinned to canary?

It still needs to be pinned to canary for now, as it requires this ember-cli's PR ember-cli/ember-cli#7961 (not yet on beta branch).

Chatted with @rwjblue and he is 👍 on this, at least as a temporary solution.

Great! Thank you @mixonic. I'll update to a published ember-cli version when available.

Do you think it could be released?

dcyriller mentioned this pull request Jul 5, 2018

[Feature] Add MU support for model and model-test blueprints warp-drive-data/warp-drive#5491

Merged

Turbo87 reviewed Jul 10, 2018

View reviewed changes

Turbo87 requested a review from rwjblue July 10, 2018 10:31

Turbo87 added the enhancement label Jul 10, 2018

rwjblue reviewed Jul 11, 2018

View reviewed changes

dcyriller mentioned this pull request Jul 26, 2018

Module Unification: Blueprints ember-cli/ember-cli#7530

Closed

29 tasks

dcyriller changed the title ~~Add MU support to ember new test helper~~ Add MU support to blueprints test helpers Aug 6, 2018

dcyriller mentioned this pull request Aug 6, 2018

Turn experiments object into a function ember-cli/ember-cli#7961

Merged

Add MU support to blueprint test helpers

7463a66

dcyriller mentioned this pull request Aug 8, 2018

Unskip MU adapters tests warp-drive-data/warp-drive#5558

Merged

mixonic reviewed Sep 11, 2018

View reviewed changes

mixonic merged commit 4b31f1e into ember-cli:master Sep 14, 2018

dcyriller deleted the ember-new-mu-support branch September 14, 2018 18:01

This was referenced Sep 14, 2018

Fix typo in README #157

Merged

Use ember-cli v3.5.0-beta.1 #159

Merged

ppcano mentioned this pull request Dec 6, 2018

Update ember-cli to v3.5.1 emberjs/ember.js#17253

Merged

Uh oh!

Conversation

dcyriller commented Jul 5, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dcyriller commented Jul 5, 2018

Uh oh!

Turbo87 commented Jul 5, 2018

Uh oh!

dcyriller commented Jul 5, 2018

Uh oh!

dcyriller commented Jul 5, 2018

Uh oh!

Turbo87 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dcyriller commented Jul 10, 2018

Uh oh!

runspired commented Jul 10, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mixonic commented Jul 23, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dcyriller commented Jul 23, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dcyriller commented Aug 6, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dcyriller commented Aug 8, 2018

Uh oh!

dcyriller commented Aug 8, 2018

Uh oh!

dcyriller commented Aug 16, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dcyriller Sep 11, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

dcyriller commented Jul 5, 2018 •

edited

Loading

mixonic commented Jul 23, 2018 •

edited

Loading

dcyriller commented Jul 23, 2018 •

edited

Loading

dcyriller commented Aug 6, 2018 •

edited

Loading

dcyriller Sep 11, 2018 •

edited

Loading