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

fix: add better embroider test and fix embroider compat #8410

Merged
merged 9 commits into from
Feb 25, 2023

Conversation

mkszepp
Copy link
Contributor

@mkszepp mkszepp commented Feb 17, 2023

Add embroider try scenarios for tests

@mkszepp
Copy link
Contributor Author

mkszepp commented Feb 17, 2023

@runspired maybe we get already with this tests the same error like explained in #8396, otherwise we need to add some other tests, which should fail

@runspired runspired force-pushed the add-embroider-try-scenarios branch 2 times, most recently from 7c92f18 to 28ba8db Compare February 25, 2023 03:19
@runspired runspired added 🎯 canary PR is targeting canary (default) 🏷️ test This PR primarily adds tests for a feature labels Feb 25, 2023
@runspired
Copy link
Contributor

@mkszepp ended up adding a test which failed because we can't use @embroider/test-setup currently as it results in downstream build errors: glimmerjs/glimmer.js#406

I think the solution here is to not use moduleExists (since no module does exist) and to use dependencySatisfies. This was failing because (as you noticed but I'm not sure quite communicated well enough in #8403 for me to understand why that was what you were doing) there is no module for moduleExists to find.

@runspired runspired added 🏷️ bug This PR primarily fixes a reported issue 🎯 release PR should be backported to release 🎯 lts The PR should be backported to the most recent LTS labels Feb 25, 2023
@runspired runspired changed the title Add embroider try scenarios fix: add better embroider test and fix embroider compat Feb 25, 2023
@runspired runspired merged commit 6143956 into emberjs:master Feb 25, 2023
@mkszepp mkszepp deleted the add-embroider-try-scenarios branch February 27, 2023 06:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🎯 canary PR is targeting canary (default) 🎯 lts The PR should be backported to the most recent LTS 🎯 release PR should be backported to release 🏷️ bug This PR primarily fixes a reported issue 🏷️ test This PR primarily adds tests for a feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants