ember.js icon indicating copy to clipboard operation
ember.js copied to clipboard

[Bug] ember-try canary scenarios failing for dangling destroyables

Open scalvert opened this issue 2 years ago • 3 comments

🐞 Describe the Bug

The master branch of ember-lifeline started failing its canary ember-try scenario with the following error:

https://github.com/ember-lifeline/ember-lifeline/runs/6181216496?check_suite_focus=true#step:6:116

Note the (EventDispatcher) string to identify the destroyable.

I'm unsure if it's related to:

https://github.com/emberjs/ember.js/commit/9fb35c41563d450293d991d313bd99f82b4acece

Which has the string (EventDispatcher) in its toString.

🔬 Minimal Reproduction

Run an addon with canary enabled.

😕 Actual Behavior

Dangling destroyables remain after tests are completed.

🤔 Expected Behavior

All destroyables are correctly destroyed (calling assertDestroyablesDestroyed passes).

🌍 Environment

  • Ember: - 3.28.0
  • Node.js/npm: - 12.22.1
  • OS: - osx 12.2.1
  • Browser: - Chrome 100

➕ Additional Context

N/A

scalvert avatar Apr 28 '22 19:04 scalvert

cc/ @wagenet

scalvert avatar Apr 28 '22 19:04 scalvert

@scalvert are you able to make a failing test for this? Alternatively, could you bisect ember and confirm the breaking commit?

wagenet avatar Apr 28 '22 19:04 wagenet

Unlikely right now as I'm juggling a billion things.

To add some more info, the reason this is likely manifesting in ember-lifeline is because there's a validation test helper that lifeline exports that identifies any undestroyed destroyables. The problem with the current destroyables/assertion system is that we need the ability to label destroyables so that we're only asserting on destroyables we've registered ourselves, not framework destroyables.

Probably more information than you need, but it may make sense to add some tests into the framework that validate that no new dangling destroyables were added.

scalvert avatar Apr 28 '22 19:04 scalvert