Failing test case for lazily loaded engines with loading states

[rwjblue]

Creates a separate set of tests that leverage async loaded routes (roughly akin to what ember-engines does) to ensure model loading and loading states work correctly in that scenario.

Currently, this emits the following error:

TypeError: Cannot read property '_stashNames' of undefined
    at stashParamNames (http://localhost:7021/tests/ember.js:23926:13)
    at Class.setup (http://localhost:7021/tests/ember.js:20365:37)
    at _routeEnteredOrUpdated (http://localhost:7021/tests/ember.js:62215:17)
    at PrivateRouter.routeEnteredOrUpdated (http://localhost:7021/tests/ember.js:62230:9)
    at PrivateRouter.setupContexts (http://localhost:7021/tests/ember.js:62159:16)
    at PrivateRouter.getTransitionByIntent (http://localhost:7021/tests/ember.js:61969:14)
    at PrivateRouter.transitionByIntent (http://localhost:7021/tests/ember.js:61905:21)
    at PrivateRouter.doTransition (http://localhost:7021/tests/ember.js:62040:19)
    at PrivateRouter.intermediateTransitionTo (http://localhost:7021/tests/ember.js:62522:19)
    at Class.intermediateTransitionTo (http://localhost:7021/tests/ember.js:22389:28)

This was introduced by fixing another issue (that the original state was not preserved when doing intermediated transitions) over in:

• [BUGFIX beta] provide transition to setupContext for internal transit… tildeio/router.js#308
• [BUGFIX release] QP's bug for intermediate transitions #19249

Those changes don't directly cause the issue, but the issue was masked due to the bug that they fixed.

---

This affects Ember 3.22.1 (and current 3.23 betas).

[scalvert]

@rwjblue any idea why that job is failing? I tried to parse the logs but they're making my eyes bleed...

[sly7-7]

@rwjblue I've forked this branch and rebase against master. https://github.com/sly7-7/ember.js/tree/failing-test-case-for-engines

I turns out there are 2 failing tests. The first one is resolved by adding an undefined check in the router microlib

I think it's legit, because after reading a bit the router code, I find out that when route are lazily loaded, they can be undefined at first (seems it's like this by design);

Concerning the second failing test, I have doubt, if either it's a brittle test, or if the bug is in the router.

If I understand correctly your goal when adding those tests, you would simulate routes lazy loading. In this case, I guess this involves some kind of asynchronous behavior. But seeing those three lines:

https://github.com/sly7-7/ember.js/blob/failing-test-case-for-engines/packages/ember/tests/routing/model_loading_test.js#L827

I think we should be waiting for the async process to finish before sending the second and third actions.
I've tried to use the await runLoopSettled();, but either it's not enough, or not the right 'waiter', or a bug in the router.

As you can see, the first call to editPost triggers the lazy load of the edit route, but there is a missing 'waiter' somewhere, so I think the transition is aborted by the 'showPost' action. After that, the second call to edit post works, because this time, the route has been loaded.

So at this point, I'm just confused, because I don't know if it's a bug, or if the test should be adapted to take the async behavior in account.

[sly7-7]

Ok, so there is definitely some kind of mess with asynchrony. The test now "passes", but probably for wrong reason, because I must call await runLoopSettled(); twice after the first call to editPost().
I've also replace the expectDeprecation()by expectDeprecationAsync which seems legit because of the route lazy loading.
You can see the diff here: sly7-7@c4b4d3b

[sly7-7]

@rwjblue Unless I'm missing something, this is the thing landed in 3.25.3 right ?

[rwjblue]

I think we still need to add the guard in router_js setupContexts (from your screenshot).

[rwjblue]

I've rebased against master (fixing up some merge conflict issues).

[rwjblue]

Also, FWIW, it is easiest to review without whitespace (https://github.com/emberjs/ember.js/pull/19266/files?diff=split&w=1) because I re-indented a bunch of the tests...

[rwjblue]

I think we still need to add the guard in router_js setupContexts (from your screenshot).

I think that will fix this test:

Test failed: Route - model loading (simulated within lazy engine):  Nested callbacks are not exited when moving to siblings", source:  (49)
[0309/145618.057284:INFO:CONSOLE(52)] "    Failed assertion: Promise rejected during " Nested callbacks are not exited when moving to siblings": Cannot convert undefined or null to object
TypeError: Cannot convert undefined or null to object
    at PrivateRouter.setupContexts (http://localhost:13141/tests/ember.js:64725:9)
    at PrivateRouter.finalizeTransition (http://localhost:13141/tests/ember.js:64653:14)
    at http://localhost:13141/tests/ember.js:64592:21
    at invokeCallback (http://localhost:13141/tests/ember.js:65801:17)

[sly7-7]

@rwjblue Yeah, I'm into that right now, and sorry, I thought I had pushed a PR in the router.js.
Done here: tildeio/router.js#322

[kategengler]

Is this still relevant? It is a draft and seems to have a successor PR