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

ember-pickr makes our test suite brittle #49

Open
tschoartschi opened this issue Jun 25, 2019 · 9 comments
Open

ember-pickr makes our test suite brittle #49

tschoartschi opened this issue Jun 25, 2019 · 9 comments

Comments

@tschoartschi
Copy link

Hey!

We just encountered that our tests which rely or render ember-pickr are brittle. We think this happens because pickr.min.js is instantiated within a requestAnimationFrame and in combination with the default parameter different timings can happen. Therefore our test suite sometimes pass and sometimes not.

I'm not sure if it's a ember-pickr issue or something else. Also I do not know how to work around it. Would be great if someone could help us 😃

Here is the stacktrace from the test:

async-color-pickr

@astronomersiva
Copy link
Owner

Hi @tschoartschi, thanks for the detailed issue write-up! Could you also help with a dummy repo with a sample test so that I can look into this?

@tschoartschi
Copy link
Author

@astronomersiva I'll try to create one 😃 I'm not sure if I can create a reproduction because it's a timing issue. We discussed the issue in Discord as well and we thought that maybe it's a problem on the following line:

this.set('_value', this.formatColor(this._pickr.getColor()));

Because of this.set is called after requestAnimationFrame the timing is not consistent and it can happen that the default param overrides an already set property.

I'm not sure if it's an issue with ember-pickr or if the ember test runner should be aware of requestAnimationFrame. Because in our test file:

        await render(hbs`
        <Some::Form::With::A::Colorpicker
            @model={{this.data}}
            @update={{this.update}}
        />`);

Because await render does not wait for ember-pickr this timing problems arise.

@tschoartschi
Copy link
Author

@astronomersiva I tried to create a repo which has similar problems like we have. I'm not sure if it's really the same as with our code but it behaves definitely the same. It's super hard to create a repo with a reproduction because it's a timing issue and I'm also not 100% sure what causes the timing problems. You can have a look here:

https://github.com/tschoartschi/ember-pickr-test-issues

The following screencast should show how it behaves:

https://github.com/tschoartschi/ember-pickr-test-issues/blob/master/pickr.mov

@astronomersiva
Copy link
Owner

Thanks a lot @tschoartschi, that video helped me understand what this is about. I've been encountering this as well. Let me try and fix this 👍

@tschoartschi
Copy link
Author

@astronomersiva cool 😎in case you need more information let me know. Maybe we need to write a test helper or something. I'm not sure what the best solution is but if you need something just reach out.

@astronomersiva
Copy link
Owner

Yes, this addon needs to expose a test waiter so that ember-test-helpers's wait is aware of this addon's behaviour. vertical-collection does something similar.

@tschoartschi
Copy link
Author

Just wanted to follow up. Do you think this will be easy to fix, since we have some template from vertical-collection or do you need some help?

@tschoartschi
Copy link
Author

@astronomersiva any update on this issue or do you need help?

@astronomersiva
Copy link
Owner

@tschoartschi, sorry I have been held up with some work of late. Would certainly appreciate help with this!

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

No branches or pull requests

2 participants