Testing a custom attributes' valueChanged() hook

[MarcScheib]

I'm submitting a bug report

• Library Version:
  ^1.0.0-beta.3.0.1

Please tell us about your environment:

• Operating System:
  Windows 10

• Node Version:
  8.1.3

• NPM Version:
  5.0.3

• JSPM OR Webpack AND Version
  JSPM 0.16.53

• Browser:
  Chrome

• Language:
  ESNext

Current behavior:

When testing a custom attribute with one bound value and valueChanged() method, the method is not called during the spec, although the value binding is working in e.g. an input field.

• What is the expected behavior?

The valueChanged() is called during testing when changing the bound value.

• What is the motivation / use case for changing the behavior?

Testing the logic of a custom attributes valueChanged() hook.

I am using the following code so far and binding is also working for the input, but not for the custom attribute:

import {bootstrap} from 'aurelia-bootstrapper';
import {Aurelia} from 'aurelia-framework';
import {StageComponent} from 'aurelia-testing';
import {DOM} from 'aurelia-pal';

import {BackgroundChanger} from '../../src/background-changer';

describe('BackgroundChange', () => {
  let component;
  let vm = { bg: 'blue' };

  beforeEach(() => {
    component = StageComponent
      .withResources('src/background-changer')
      .inView(`
<input id="bg-input" value.bind="bg">
<div id="bg" bg-change.bind="code"></div>
`)
      .boundTo(vm);
  });

  afterEach(() => {
    component.dispose();
  });

  it('changes the background on input change', done => {
    component.create(bootstrap)
      .then(() => {
        const inputEl = DOM.getElementById('bg-input');
        const divEl = DOM.getElementById('bg');

        expect(inputEl.value).toEqual(vm.bg);

        const newBg = 'red';

        inputEl.value = newBg;
        expect(inputEl.value).toEqual(newBg);
        inputEl.dispatchEvent(DOM.createCustomEvent('change'));
        expect(vm.bg).toEqual(newBg);
        expect(divEl.style.background).toEqual(newBg); // this one fails with expected blue to be red
      })
      .then(done);
  });
});

[suneelv]

@MarcScheib doing the assertion after a timeout might work as valueChanged callbacks are not called synchronously

setTimeout(() => {
  expect(divEl.style.background).toEqual(newBg); 
  done();
}, 50)

[MarcScheib]

Yes, that worked @suneelv . Thank you!

[tomtomau]

This affect me as well, and the above work around does not resolve it.

[RomkeVdMeulen]

I've run into a similar problem countless times in my own tests:

1. Update some value on the VM
2. Check the DOM: it hasn't been updated yet

The go-to workaround is to put step 2 inside a setTimeout or queue it as a macro task on Aurelia's TaskQueue. And that will solve the problem most of the time. But every once in a blue moon our CI server will report a test failing when it has succesfully run a thousand times before.

I'm getting very tired of it. I want to find a solution where the test will deterministically succeed.

I've been trying to figure out where the delay occurs, but I can't figure it out. From my understanding, each model update should synchronously trigger Aurelia's property observer, which puts a microtask on the queue to update linked bindings. The microtask queue is flushed with a DOM mutation observer which is a JS microtask. So by the time the setTimeout in my test, which is a macro task, resolves, the binding updates and the corresponding DOM updates should have run already, and if I test the DOM for the updated value that should succeed 100% of the time. And yet it doesn't. What am I missing?

Could someone explain it to me, please? @EisenbergEffect perhaps, or @fkleuver? Is there any way for me to delay the DOM check in my test so that the binding update is guaranteed to have been resolved?

[tomtomau]

@RomkeVdMeulen

Our solution has been to waitFor your assertion.

// Rather than
expect(divEl.style.background).toEqual(newBg); 



// waitFor instead, we have a helper function as the aurelia-testing is asserting against whether something is null rather than truthy/falsy
import { waitFor } from 'aurelia-testing';

export class WaitForBool {
  static async truthy(callback) {
    return await waitFor(() => callback() ? true : null);
  }

  static async falsy(callback) {
    return await waitFor(() => callback() ? null : false);
  }
}

// Then your actual assertion
await WaitForBool.truthy(() => divEl.style.background === newBg);

[RomkeVdMeulen]

@tomtomau Thanks for the suggestion. I'm already using that construct in some places. But we have tens of thousands of assertions in our tests, and I fear if we use waitFor for all of them it will slow down the runtime of our tests to the point that it becomes impractical to run them.

[bigopon]

@RomkeVdMeulen this is an issue with how our v1 setup the task queue. One trick that you can use to eliminate this problem forever is to reuse a single task queue instance in tests. There' many ways to do it, depends on your bootstrapping, one way is to wrap bootstrapper function with your one:

import {bootstrap as $bootstrap} from 'aurelia-bootstrapper';
import { TaskQueue } from 'aurelia-task-queue';

let taskQueue: TaskQueue;
export function bootstrap(aurelia: Aurelia) {
  if (taskQueue) {
    aurelia.container.registerInstance(TaskQueue, taskQueue);
  } else {
    taskQueue = aurelia.container.get(TaskQueue);
  }
  return $bootstrap(aurelia);
}

And later in your code, instead of setTimeout, you can inject the task queue from wherever, and do:

taskQueue.flushMicroTaskQueue();

[RomkeVdMeulen]

@bigopon Thanks for the tip! I'll try it out.

[RomkeVdMeulen]

Also does that mean this won't be an issue anymore in V2?

[bigopon]

Not at all, at least i believe so. For the above, make sure you flush all existing queues when disposing an app at the end of a test as well.

[RomkeVdMeulen]

@bigopon I tried your suggestion: I re-used one task queue throughout the test run, making sure to manually flush the micro queue before assertions and during tear down. It didn't seem to make a difference. I'm still seeing intermittent failures where the DOM wasn't updated in time for the assertion.

I'm also trying to understand why flushing the queue manually should make a difference. Isn't the micro task queue always flushed before the start of the next event loop? (I run my tasks in Chrome, so MutationObservers are supported)

[RomkeVdMeulen]

I've been trying to create a minimal reproducable case, but so far without much luck. I've looked into all kinds of contributing factors, like async/await or computed properties, but as far as I can see none of those should be able to create a delay beyond the current event loop.

Perhaps the issue isn't with timing, but with some state not being properly cleaned between test runs, like with issue #93. Because these errors are so infrequent, I'm having a devil of a time debugging them.

[RomkeVdMeulen]

It doesn't seem to be a case of stale view state. Let me give a simplified example of what I'm seeing:

export class MyVM {
  myProp = {
    id: Math.round(Math.random() * 1000),
    loading: false,
  };

  myMethod() {
    console.log("flipping boolean");
    this.myProp.loading = true;
  }
}

<button click.trigger="myMethod()" data-loading="${myProp.loading}" data-id="${myProp.id}">click here</button>

it("changes the boolean", async () => {
  // setup...
  console.log("click");
  myComponentTest.element.querySelector("button").click();
  console.log("wait...");
  await new Promise(res => setTimeout(res);
  console.log(myComponentTest.viewModel.myProp);
  console.log(myComponentTest.element.querySelector("button"));
});

And once in a dozen runs the output is:

LOG: click
LOG: flipping boolean
LOG: wait...
LOG: {id: 5231, loading: true}
LOG: <button click.trigger="myMethod()" data-loading="false" data-id="5231">click here</button>

Of course the real code has composition, more indirection, DI, computed properties, etc. but in essence this is what it boils down to.

[bigopon]

@RomkeVdMeulen here's a test that can be used to verify your repro. You can un-comment the method call in attached setInterval and decrease the interval to verify. It seems working fine to me
https://gist.dumber.app/?gist=735a4a0bc868bde5cc7469bd11692562

Edit: i gave you a wrong example code though, sorry! It should have been

import { bootstrap as $bootstrap } from 'aurelia-bootstrapper';

let taskQueue;
export function bootstrap(configure) {
  const $configure = aurelia => {
    if (taskQueue) {
      aurelia.container.registerInstance(TaskQueue, taskQueue);
    } else {
      taskQueue = aurelia.container.get(TaskQueue);
    }
    return configure(aurelia);
  };
  return $bootstrap($configure);
}