Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Testing a custom attributes' valueChanged() hook #70

Open
MarcScheib opened this issue Jul 9, 2017 · 17 comments
Open

Testing a custom attributes' valueChanged() hook #70

MarcScheib opened this issue Jul 9, 2017 · 17 comments
Labels

Comments

@MarcScheib
Copy link

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
Copy link

suneelv commented Jul 16, 2017

@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
Copy link
Author

Yes, that worked @suneelv . Thank you!

@tomtomau
Copy link
Contributor

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

@RomkeVdMeulen
Copy link
Contributor

RomkeVdMeulen commented Sep 1, 2020

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
Copy link
Contributor

tomtomau commented Sep 1, 2020

@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
Copy link
Contributor

@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
Copy link
Member

bigopon commented Sep 2, 2020

@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
Copy link
Contributor

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

@RomkeVdMeulen
Copy link
Contributor

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

@bigopon
Copy link
Member

bigopon commented Sep 3, 2020

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
Copy link
Contributor

@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
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Member

bigopon commented Sep 6, 2020

@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);
}

@RomkeVdMeulen
Copy link
Contributor

@bigopon You're right, the repro works fine. I've been trying to narrow down where the delay is coming from, but as you can imagine with the delay only occuring randomly among lots of test runs it's very slow going.

Thanks for your help. If I do find the problem I'll let you know. For now just consider this closed from my side.

@lemoustachiste
Copy link

Hijacking this thread to improve my general knowledge with this tool.

I was trying to verify that the click callback on a button was being called.

My initial implementation was with

const clickEvent = new Event('click');
myButtonElement.dispatchEvent(clickEvent);

That didn't trigger the click handler (called with click.delegate).

With the help of this comment #70 (comment) I realised I could use the HTMLButtonElement.click method, which indeed worked (even without having to delay the assertion).

My question is, why is the classic DOM Event method not being a trigger? Is this a preferred approach for any event or would there be cases where dispatchEvent would work?

Thanks

@bigopon
Copy link
Member

bigopon commented Sep 28, 2020

@lemoustachiste there shouldn't be any issue with what you are trying to do. Can you help give a repro of what you want to do, based on the example here https://gist.dumber.app/?gist=650aecc59a32d08776692a4b5055b7c0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

7 participants