Acceptance tests do not wait for all requests during a store merge

[chbonser]

The Problem

I'm chasing a bug in an existing project where a number of acceptance tests started failing because some (all?) of the async test helpers from ember-test-helpers lost the ability to know the app is settled. For example a await click line in a test will continue forward while async stuff (ember-concurrency task and ajax calls) are still running.

Backstory

I say 'lost' above because this occurred in an existing project where we've been using orbit for a few months. The tests have been rock solid until... I added ember-auto-import as a top level dependency. Super weird right? I couldn't make heads of tails of that one, so I started a new project to reproduce the issue in an app with fewer dependencies.

Reproduction App

I've been able to reproduce the failure in this public app https://github.com/forge512/async-test-bug.

The app has component which runs an ember-concurrency task to create 4 posts component. And it has a test which verifies it works.

Experiment 1 (ember-data parallel save)

I used ember-data to fire requests to create 4 posts with parallel requests. See the use of ember-concurrency all here.

Tests pass.

Experiment 2 (ember-data parallel save)

In this branch, I adapted the component to fire the requests in serial.

Tests pass.

Experiment 3 (orbit with store forking)

In this branch, I switch to using orbit with a blocking remote store. The component forks the store, adds 4 posts, then merges the store.

Tests fail.

Summary

I'm not sure why one app (the private repo) worked until adding ember-auto-import. I'm also not sure why the example app failed in experiment 3. I had expected experiment 3 to pass, but then mysteriously fail when I added ember-auto-import.

[dgeb]

Thanks for the detailed report and the app to repro the test issue.

The following changes to your test seem to fix the problem:

import { module, test } from "qunit";
import { click, visit, settled } from "@ember/test-helpers";
import { setupApplicationTest } from "ember-qunit";
import setupMirage from "ember-cli-mirage/test-support/setup-mirage";

module("Acceptance | create many posts", function(hooks) {
  setupApplicationTest(hooks);
  setupMirage(hooks);

  test("it makes the posts", async function(assert) {
    server.timing = 400;
    await visit("/");

    await click("button");

    // await request processing
    await this.owner.lookup('service:store').source.requestQueue.process();

    // await app settled state
    await settled();

    assert.dom(".post").exists({ count: 4 });
  });
});

Waiting for the store's requestQueue to finish processing is necessary since the action triggers multiple fetch requests. I'll try to merge something like #164 soon to handle this kind of waiting. Obviously, the waiting for settledness above is just too verbose.

Note that settled in ember-test-helpers still seems to be only xhr-aware. That's why await settled() does not work on its own - it needs to be preceded by awaiting the requestQueue to process. It's also why you didn't experience issues with ember-data. We should really update ember-data and the settled helper to be fetch-aware.

Beyond this, I'm not sure exactly what's changed and why in your app, especially regarding ember-auto-import. However, if you can reproduce more issues in your demo app, please let me know and I'll investigate.

[chbonser]

Thanks @dgeb. I've been mulling over your response all week. It does indeed solve the problem in the example app.

But I do not understand (a) why my tests were ever passing in our real app, given that I wasn't awaiting requestQueue.process(), or (b) why adding ember-auto-import made them start failing. I'll update again when I understand more.

[dgeb]

@chbonser Those are mysteries to me as well. My suspicion is that it is related to a recent change in ember-test-helpers - perhaps emberjs/ember-test-helpers#482 ?

If you want to dig in and can reproduce the problem in your example repo, I'd be glad to take a look.

[simonihmig]

Maybe we could provide a setupOrbit() function, that sets up a test waiter (https://emberjs.com/api/ember/3.7/functions/@ember%2Ftest/registerWaiter) which checks that the request queue is empty? This way a simple await settled() would "just work"(tm).

Fwiw, we also had to fight issues of Ember's test helpers having no knowledge of async stuff happening (Iirc they will track $.ajax automatically, but no fetch or so, happening outside of model hooks). We implemented a waitFor decorator we wrap around everything async that's not in a model hook, using those waiter APIs, and it proved to work quite fine...

[dgeb]

Thanks to @cibernox, we have a new waitForSource test helper that you can use to wait for a source's request and sync queues to flush. You can import it to your tests as follows:

import { waitForSource } from 'ember-orbit/test-support';

And then use it to wait for a source by reference or by name:

await waitForSource(store); // by reference
await waitForSource('backup'); // by name

Maybe we could provide a setupOrbit() function, that sets up a test waiter (https://emberjs.com/api/ember/3.7/functions/@ember%2Ftest/registerWaiter) which checks that the request queue is empty? This way a simple await settled() would "just work"(tm).

The challenge with such a global method is that an orbit app may have many sources that need to settle. I suppose one option would be to wait for all the sources assigned to a given coordinator. For now, let's see how the source-specific waitForSource works out and continue to think about this.

[chbonser]

@dgeb There is still a mystery as to why my tests were ever passing, but we can close this ticket. waitForSource does the trick.

[cibernox]

@dgeb In my app i've created a waitForAll helper that, as the name suggests, it waits for every source to finish processing its jobs (and I added a return settled() just to be extra sure the UI has updated).

I use that helper instead of waitForSource all the time except in those tests where I explicitly want to test intermediate states while sources are out of sync.

[dgeb]

@cibernox It's tricky to have a general concept of "all sources" in orbit, since sources may not even be managed by a single coordinator. However, a waitForAll helper makes a lot of sense in the context of a specific app with a known set of sources 👍

[RobbieTheWagner]

@cibernox would you be able to share that helper?

[tchak]

Something like this:

async function waitForAll() {
  const { owner } = getContext();
  const { services } = owner.resolveRegistration('ember-orbit:config');
  const coordinator = owner.lookup(`service:${services.coordinator}`);

  for (let source of coordinator.sources) {
    await waitForSource(source);
  }
}

[RobbieTheWagner]

Thanks @tchak! I'm currently hitting a lot of issues with testing and timing, so trying to properly wait for things and debug.

[cibernox]

@tchak beat me to it.

This is my helper, because I only have two sources.

import { settled } from '@ember/test-helpers';
import { waitForSource } from 'ember-orbit/test-support';

export async function waitForAll() {
  await waitForSource('remote');
  await waitForSource('store');
  return settled();
}

[RobbieTheWagner]

Thanks @cibernox! Any tips on setting up data for testing? I've been trying to get things working with mirage but hitting some issues.

[cibernox]

For this specific app I used https://netflix.github.io/pollyjs instead of mirage, and I liked it a lot.

It's like VCR in ruby, but for javascript. I records the real requests and replays them. And unlike mirage (last time i checked) it supports fetch, while mirage only supported XHR.

The only caveat of recording and replaying requests, is that you have to mock the UUID generation so it's deterministic, otherwise requests will have different payloads and won't match the recorded ones. Same thing if you send dates/times.