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

Test against embroider #479

Merged
merged 5 commits into from
May 6, 2024
Merged

Test against embroider #479

merged 5 commits into from
May 6, 2024

Conversation

NullVoxPopuli
Copy link
Contributor

@NullVoxPopuli NullVoxPopuli commented May 5, 2024

Tests against strictest embroider -- which we weren't doing before.

This is important because in the v2 addon conversion, I ran in to

which is actually good, because test-waiters needs to be resilient to:

  • copies of itself being loaded
  • copies of other libraries using the same waiter names

Adding embroider to the matrix helps verify that the problem is with auto-import having duplicate test-time dependencies, vs embroider fixing the problem and having only one copy of a dependency.

Why not use the try matrix, like in the blueprint?

  • try matrix already has/had embroider specified, but none of the test-apps were set up with maybeEmbroider -- it turns out as well that the ember-concurrency stuff isn't compatible (it's an old version, and I'm considering a separate PR to move all the thennable and generator tests into the main test app, but without using ember-concurrency, so that it's better declared what we're actually supporting, and more unit-testy, rather than e2e-y)
  • because debugging is easier this way -- ember-try messes with the local node_modules which makes getting back to a non-ember-try state a bit harder (sometimes pnpm i isn't enough).
  • because we want to be able to debug both classic and embroider quickly, it makes sense that someone would even have both running at the same time.

@NullVoxPopuli NullVoxPopuli changed the title For the main test app, also test against embroider Test against embroider May 5, 2024
@NullVoxPopuli NullVoxPopuli force-pushed the also-test-against-embroider branch from 2f46f9a to 98070df Compare May 6, 2024 17:19
@NullVoxPopuli NullVoxPopuli merged commit 120b599 into master May 6, 2024
31 checks passed
@NullVoxPopuli NullVoxPopuli deleted the also-test-against-embroider branch May 6, 2024 17:41
@github-actions github-actions bot mentioned this pull request May 6, 2024
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.

2 participants