steveszc / ember-cli-memory-leak-detector

Automatic memory leak detection for ember apps
https://www.npmjs.com/package/ember-cli-memory-leak-detector
MIT License
38 stars 5 forks source link

Causes tests to hang in PNPM repos #51

Open lflo5727 opened 1 year ago

lflo5727 commented 1 year ago

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:

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.

steveszc commented 1 year ago

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?

lflo5727 commented 1 year ago

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

steveszc commented 1 year ago

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

ahemed-haneen commented 1 year ago

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 commented 1 year ago

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

alexabreu commented 7 months ago

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