ember-cli-memory-leak-detector icon indicating copy to clipboard operation
ember-cli-memory-leak-detector copied to clipboard

Causes tests to hang in PNPM repos

Open lflo5727 opened this issue 2 years ago • 6 comments

I found that this package causes the ember test command to hang indefinitely after successfully completing all tests in Ember packages existing within a PNPM monorepo with no error output.

We had successfully used ember-cli-memory-leak-detector in individual Yarn repos, but as we moved our packages into a monorepo with PNPM, we noticed tests hanging. After removing this package we were able to complete our tests. I forked this repo and was able to reproduce the issue.

To reproduce this simply go ahead and clone my fork here.

Then on the main branch, run the following commands:

  • pnpm install This installs all the node packages
  • pnpm --filter limber test:ember this will run the tests for the limber project. You can also cd into frontend and run ember test

You should see all tests complete and the command properly exit.

Then, go ahead and switch to the hung-tests branch. Follow the same steps above and you will see that the test run just hangs after completing. Viewing the branch diff, you can see that the only change is adding ember-cli-memory-leak-detector.

lflo5727 avatar Dec 29 '22 17:12 lflo5727

thanks for reporting. I am not sure whether this is actually do to pnpm or monorepo structure. It is more likely related to Qunit (see #49). Can you check if your pnpm monorepo is using a newer Qunit version than the repos you've had success using this on in the past?

steveszc avatar Jan 09 '23 19:01 steveszc

I don't think this is related to Qunit. We are using the exact same version of Qunit between the app in the monorepo and on its own with Yarn. Works with Yarn, hangs with PNPM monorepo. QUnit Version: 2.14.0

lflo5727 avatar Jan 10 '23 18:01 lflo5727

Okay... well I think we can safely rule out monorepo structure having to do with it, since the addon itself is a monorepo and the test app is a package within the monorepo.

I am not very familiar with pnpm and I don't think I will have time to dig in to what the issue might be anytime soon. I would gladly accept a PR to fix whatever the issue might be, if you are able to track down a root cause and fix it.

A good place to start looking might be the logic that injects the failed test into Qunit https://github.com/steveszc/ember-cli-memory-leak-detector/blob/main/packages/ember-cli-memory-leak-detector/lib/templates/test-body-footer.html#L73

steveszc avatar Jan 10 '23 20:01 steveszc

I don't think this is related to Qunit. We are using the exact same version of Qunit between the app in the monorepo and on its own with Yarn. Works with Yarn, hangs with PNPM monorepo. QUnit Version: 2.14.0

i've been trying to test this out in a fresh ember application with yarn and the same issue persists.

don't know how you were able to get it running in yarn but it's not possible right now it seems

ahemed-haneen avatar Oct 20 '23 11:10 ahemed-haneen

Okay... well I think we can safely rule out monorepo structure having to do with it, since the addon itself is a monorepo and the test app is a package within the monorepo.

I am not very familiar with pnpm and I don't think I will have time to dig in to what the issue might be anytime soon. I would gladly accept a PR to fix whatever the issue might be, if you are able to track down a root cause and fix it.

A good place to start looking might be the logic that injects the failed test into Qunit https://github.com/steveszc/ember-cli-memory-leak-detector/blob/main/packages/ember-cli-memory-leak-detector/lib/templates/test-body-footer.html#L73

i've been going through the code that you mentioned and testem.afterTests itself seems not to work. i could look a bit deeper if someone could point out to me how and when this script in the html file gets invoked

ahemed-haneen avatar Oct 20 '23 11:10 ahemed-haneen

Also seeing tests hang when this addon is installed. Using regular old npm rather than pnpm.

alexabreu avatar Mar 28 '24 18:03 alexabreu