emberjs / ember-test-helpers

Test-framework-agnostic helpers for testing Ember.js applications
Apache License 2.0
187 stars 257 forks source link

ember-try scenarios failed after installing @ember/test-helpers@4.0.2 #1489

Closed ijlee2 closed 2 months ago

ijlee2 commented 2 months ago

As reported in https://github.com/ember-intl/ember-intl/issues/1916, when a consumer of ember-intl tried to update @ember/test-helpers to v4, they encountered the error message,

ember-intl tried to import "@ember/test-helpers" in "ember-intl/test-support/add-translations.js" from addon code, but "@ember/test-helpers" is a devDependency. You may need to move it into dependencies.

The reason is, ember-intl (a v1 addon) hadn't declared @ember/test-helpers as a (peer) dependency. I can verify this assumption in https://github.com/ember-intl/ember-intl/pull/1918, but the fix can't be released because several ember-try scenarios for ember-intl failed with another error (CI run):

> test-app-for-ember-intl@1.3.18 test /home/runner/work/ember-intl/ember-intl/tests/ember-intl
> ember test --test-port=0

not ok 1 Chrome 127.0 - [124 ms] - Acceptance | smoke-tests > de-de: We can see translations
    ---
        actual: >
            null
        stack: >
            Error: You must set up the ember-test-helpers environment with either `setResolver` or `setApplication` before running any tests.

Could you tell me what the error message wants me to do? Is this perhaps a bug in @ember/test-helpers@4.0.2?

NullVoxPopuli commented 2 months ago

Do you have multiple copies of @ember/test-helpers in your dep graph? ember-qunit is what provides setupApplicationTest (and friends) -- what you need to use to ultimately call setResolver / setApplication -- and ember-qunit has a peer range on test-helpers using >= -- so try out pnpm dedupe and/or post back here with pnpm why @ember/test-helpers -r

elwayman02 commented 2 months ago

Related issue, but I filed it separately so this ticket can focus on the setResolver bug: https://github.com/emberjs/ember-test-helpers/issues/1490

NullVoxPopuli commented 2 months ago

Do your tests happen to run against embroider optimized?

LucasHill commented 2 months ago

I'm encountering this error with ember-intl in my project when i upgrade to v4 ember test helpers.

NullVoxPopuli commented 2 months ago

I believe we're waiting on: https://github.com/emberjs/ember-test-helpers/pull/1486 Which is, in turn, waiting on https://github.com/embroider-build/embroider/pull/2075

ijlee2 commented 2 months ago

Hi, @NullVoxPopuli .

try out pnpm dedupe and/or post back here with pnpm why @ember/test-helpers -r

I already use dedupe when updating dependencies. Here is the output for why:

Output ```sh ❯ pnpm why @ember/test-helpers -r Legend: production dependency, optional only, dev only docs-app-for-ember-intl@1.3.12 /ember-intl/docs/ember-intl devDependencies: @ember/test-helpers 4.0.2 ember-cli-addon-docs 7.0.1 ├─┬ ember-data 5.3.8 peer │ └── @ember/test-helpers 4.0.2 peer └─┬ ember-keyboard 8.2.1 └── @ember/test-helpers 4.0.2 peer ember-data 5.3.8 └── @ember/test-helpers 4.0.2 peer ember-qunit 8.1.0 └── @ember/test-helpers 4.0.2 peer my-app@1.0.11 /ember-intl/docs/my-app devDependencies: @ember/test-helpers 4.0.2 ember-qunit 8.1.0 └── @ember/test-helpers 4.0.2 peer my-app-with-fallbacks@1.0.3 /ember-intl/docs/my-app-with-fallbacks devDependencies: @ember/test-helpers 4.0.2 ember-qunit 8.1.0 └── @ember/test-helpers 4.0.2 peer my-app-with-lazy-loaded-translations@1.1.0 /ember-intl/docs/my-app-with-lazy-loaded-translations devDependencies: @ember/test-helpers 4.0.2 ember-qunit 8.1.0 └── @ember/test-helpers 4.0.2 peer my-app-with-namespace-from-folders@1.0.8 /ember-intl/docs/my-app-with-namespace-from-folders devDependencies: @ember/test-helpers 4.0.2 ember-qunit 8.1.0 └── @ember/test-helpers 4.0.2 peer my-classic-app@1.1.7 /ember-intl/docs/my-classic-app devDependencies: @ember/test-helpers 4.0.2 ember-qunit 8.1.0 └── @ember/test-helpers 4.0.2 peer my-classic-app-with-lazy-loaded-translations@1.0.0 /ember-intl/docs/my-classic-app-with-lazy-loaded-translations devDependencies: @ember/test-helpers 4.0.2 ember-qunit 8.1.0 └── @ember/test-helpers 4.0.2 peer my-v1-addon@1.0.10 /ember-intl/docs/my-v1-addon devDependencies: @ember/test-helpers 4.0.2 ember-qunit 8.1.0 └── @ember/test-helpers 4.0.2 peer my-v1-engine@1.0.8 /ember-intl/docs/my-v1-engine devDependencies: @ember/test-helpers 4.0.2 ember-qunit 8.1.0 └── @ember/test-helpers 4.0.2 peer my-v2-addon@1.0.11 /ember-intl/docs/my-v2-addon devDependencies: @ember/test-helpers 4.0.2 ember-intl@7.0.5 /ember-intl/packages/ember-intl devDependencies: @ember/test-helpers 4.0.2 ember-qunit 8.1.0 └── @ember/test-helpers 4.0.2 peer test-app-for-ember-intl@1.3.18 /ember-intl/tests/ember-intl devDependencies: @ember/test-helpers 4.0.2 ember-qunit 8.1.0 └── @ember/test-helpers 4.0.2 peer ```

Do your tests happen to run against embroider optimized?

In the aforementioned CI run, the following had failed:

(embroider-optimized had passed.)

I tried downgrading @ember/test-helpers to 3.3.1 in the test app. ember-lts-3.28 would fail due to Could not find module @ember/renderer imported from @ember/test-helpers, and the lint script would fail due to type errors. (CI run, ember-try config for 3.28)

mkszepp commented 2 months ago

@ijlee2 embroider fix is published... maybe your issue is now resolved

ijlee2 commented 2 months ago

@mkszepp Thanks for letting me know.

I'm still getting issues after updating @embroider/* to the latest, so I think there may need to be a fix from @ember/test-helpers.

https://github.com/ember-intl/ember-intl/actions/runs/10662230469

mkszepp commented 2 months ago

@ijlee2 yes i haven seen the same issue in ember-power-select...

I have tested today using strict v4.0.0 (instead of 4.0.2) see In this case my issue for ember v3.28 is still present, but for 4.4... it was solved...

It looks like the adding externals in test-helpers was at the begin good, because it has solved embroider-safe, but now after the embroider fix was released, maybe its not anymore necessary and is causing an other issue...

NullVoxPopuli commented 2 months ago

ould fail due to Could not find module @ember/renderer imported from @ember/test-helpers,

You'll want to upgrade your @embroider/* deps - pnpm update @embroider/* -r

script would fail due to type errors

test-helpers does not support TS < 5 -- which versions are you using?

3.28

test-helpers v3.2.1 can still be used


Related -- I find that when I'm in a bit of a dependency snaffu, I'll try out pnpm's slower

shared-workspace-lockfile=false

which better isolates dependencies from the different packages in your repo.

Additionally, with wide-peer-support in libraries comes some fun problems -- we often need to use injected dependencies to get peers resolving correctly (ie: not resolving dev deps).

ijlee2 commented 2 months ago

Additionally, with wide-peer-support in libraries comes some fun problems -- we often need to use injected dependencies to get peers resolving correctly (ie: not resolving dev deps).

@NullVoxPopuli What fixed the ember-try issue for me was to add dependenciesMeta.injected. (https://github.com/ember-intl/ember-intl/pull/1918/commits/20d19b38e39c59885887463fb941e61e811814b9)

I'm not sure yet if we should be encouraging people to use an opt-in feature in pnpm (what about npm and yarn users?), because (1) ember-try for ember-intl worked fine with @ember/test-helpers@3.3.1, and (2) other v2 addons that I maintain have been working fine without dependenciesMeta.injected in the test-app (where ember-try is run also).

Anyway, I can close this issue now. I appreciate your help. ✨

gossi commented 1 month ago

I can across the same error message, when migrating our huge monorepo from yarn -> pnpm:

Error: You must set up the ember-test-helpers environment with either `setResolver` or `setApplication` before running any tests.

I'm posting the solution here, as this is the first result, when googling for the error message.

The reason for the error is, that two (or more) copies of @ember/test-helpers exists (as per pnpm peer resolving algorithm).

The solution requires one copy of @ember/test-helpers (it is a singleton instance). The obvious point is to remove version duplicates. pnpm/pnpm#2443 has quite some ideas so here is what I did to do that:

I still needed to get rid of one copy for @ember/test-helpers and none of these settings helped, so I was manually following the dependencies in pnpm-lock.yaml. Looking for @ember/test-helpers reveals the two instances being installed. I showcase the diff here:

+ '@ember/test-helpers@4.0.4(@babel/core@7.25.7)(@glint/template@1.4.0)(ember-source@5.12.0(@glimmer/component@1.1.2(@babel/core@7.25.7))(@glint/template@1.4.0)(rsvp@4.8.5)(webpack@5.95.0(@swc/core@1.7.35)))':
- '@ember/test-helpers@4.0.4(@babel/core@7.25.7)(@glint/template@1.4.0)(ember-source@5.12.0(@glimmer/component@1.1.2(@babel/core@7.25.7))(@glint/template@1.4.0)(rsvp@4.8.5))':
    dependencies:
      '@ember/test-waiters': 3.1.0(@babel/core@7.25.7)
      '@embroider/addon-shim': 1.8.9
      '@embroider/macros': 1.16.9(@babel/core@7.25.7)(@glint/template@1.4.0)
      '@simple-dom/interface': 1.4.0
      decorator-transforms: 2.2.2(@babel/core@7.25.7)
      dom-element-descriptors: 0.5.1
+     ember-source: 5.12.0(@glimmer/component@1.1.2(@babel/core@7.25.7))(@glint/template@1.4.0)(rsvp@4.8.5)(webpack@5.95.0(@swc/core@1.7.35))
-     ember-source: 5.12.0(@glimmer/component@1.1.2(@babel/core@7.25.7))(@glint/template@1.4.0)(rsvp@4.8.5)(webpack@5.95.0)
    transitivePeerDependencies:
      - '@babel/core'
      - '@glint/template'
      - supports-color

I used diffchecker to help me copy these snippets over to see the difference between the instances.

you might notice the addition of @swc/core for one instance.

I traced this down, ember-source -> ember-auto-import -> babel-loader|css-loader|mini-css-extract-plugin|style-loader -> webpack -> terser-webpack-plugin, and the latter looks like this:

-  terser-webpack-plugin@5.3.10(webpack@5.95.0):
+  terser-webpack-plugin@5.3.10(@swc/core@1.7.35)(webpack@5.95.0(@swc/core@1.7.35)):
    dependencies:
      '@jridgewell/trace-mapping': 0.3.25
      jest-worker: 27.5.1
      schema-utils: 3.3.0
      serialize-javascript: 6.0.2
      terser: 5.34.1
      webpack: 5.95.0(@swc/core@1.7.35)
+    optionalDependencies:
+      '@swc/core': 1.7.35

Apparently this has to do with how pnpm resolves and dedupes peers. In one package, there is a @swc/core present which leads to two instances being present. Since this is based on peer resolution, there is basically no pnpm setting I found that could help with this. After exchanging with @NullVoxPopuli he mentioned that optionalDependencies are little adventureous in pnpm. That was the right search cue for me to find this issue: pnpm/pnpm#7231 - Now it is mentioned to use .pnpmfile.cjs and "modify" dependencies on-the-fly while they are being resolved. In the end, here is mine now:

// .pnpmfile.cjs

module.exports = {
  hooks: {
    readPackage: (pkg) => {
      if (pkg.name === "terser-webpack-plugin") {
        delete pkg.peerDependenciesMeta["@swc/core"];
      }

      return pkg;
    }
  }
};

I'm removing the optional peer dependency for terser-webpack-plugin. Now that stops pnpm from its usual peer deduping and now. finally. I'm left with one copy of @ember/test-helpers and tests are running without the message initially stated in this issue.

I must say, this is quite counter-intuitive solution. I've seen some improvement for handling optional deps in pnpm (as per my research). If you come across this, there may already be another solution provided by pnpm itself. However, if not, I hope this is helpful 😊