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

Upgrade ember-cli to latest throughout test infrastructure #961

Merged
merged 16 commits into from
Sep 14, 2021

Conversation

stefanpenner
Copy link
Collaborator

No description provided.

@stefanpenner
Copy link
Collaborator Author

Failures appear related to engines.

@stefanpenner
Copy link
Collaborator Author

stefanpenner commented Sep 8, 2021

Oh boy, the failure here is juicy!


TL;DR embroider's test-setup has never worked correctly, and regularly mixes multiple ember-cli versions in it's test scenario node_module tree. In this case, we have:

project has the following (relevant) dependency links:

  • ember-cli -> linked to a specific [email protected] (per test scenario)
  • ember-engines -> the default ember-engines residing at embroider/node_modules/ember-engines/index.js
  • ember-engines -> ember-cli (resolved from embroider/node_modules/ember-engines/index.js, which is ~3.28)

This hasn't manifested before, because the relevant code in ember-cli hasn't changed recently, now that it has it has exposed the issue.

The fact the engines (and other) dependencies introduce a dependency cycle is problematic in general, given that ember-engines is essentially frozen and that embroider aims to replace it, means a work-around in this project is likely the most appropriate.

Given this, I believe the solution is to have fixture ember-engine dependencies which correctly link the given scenarios ember-cli's version

@stefanpenner stefanpenner marked this pull request as draft September 8, 2021 19:21
@ef4
Copy link
Contributor

ef4 commented Sep 10, 2021

I think the above commit fixes the peerDeps problem. It's not ready to merge until we get stable releases of fixturify-project and scenario-tester, after stefanpenner/node-fixturify-project#50 is reviewed and released. This is pointing at my own alpha releases just to test it.

@ef4 ef4 marked this pull request as ready for review September 13, 2021 22:40
@stefanpenner
Copy link
Collaborator Author

@ef4 I'm assuming you are looking into the failures:

   Error: ENOENT: no such file or directory, copyfile 'D:/a/embroider/embroider/packages/compat/src//add-to-tree.d.ts' -> 'C:\Users\RUNNER~1\AppData\Local\Temp\tmp-1716qQfSHqG6WFKD\node_modules\@embroider\compat\src\add-to-tree.d.ts'
        at Object.copyFileSync (fs.js:1991:3)
        at Project.hardLinkFile (D:\a\embroider\embroider\node_modules\scenario-tester\node_modules\fixturify-project\index.js:260:12)
        at Project.hardLinkContents (D:\a\embroider\embroider\node_modules\scenario-tester\node_modules\fixturify-project\index.js:243:22)
        at Project.hardLinkContents (D:\a\embroider\embroider\node_modules\scenario-tester\node_modules\fixturify-project\index.js:240:22)
        at Project.writeLinkedPackage (D:\a\embroider\embroider\node_modules\scenario-tester\node_modules\fixturify-project\index.js:221:14)
        

Copy link
Collaborator Author

@stefanpenner stefanpenner left a comment

Choose a reason for hiding this comment

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

LGTM

@ef4 ef4 merged commit 5cf3ec3 into master Sep 14, 2021
@ef4 ef4 deleted the ember-cli-upgrade branch September 14, 2021 19:35
@rwjblue rwjblue changed the title Upgrade to latest ember-cli Upgrade ember-cli to latest throughout test infrastructure Sep 21, 2021
@rwjblue rwjblue changed the title Upgrade ember-cli to latest throughout test infrastructure Upgrade ember-cli to latest throughout test infrastructure Sep 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants