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

Add unit/integration tests #3

Closed
majido opened this issue Mar 25, 2020 · 5 comments
Closed

Add unit/integration tests #3

majido opened this issue Mar 25, 2020 · 5 comments
Assignees
Labels
deliverable A medium+ feature or an item that can be broken into smaller tasks

Comments

@majido
Copy link
Collaborator

majido commented Mar 25, 2020

Currently the polyfill does not have any tests.

I suggest we look into adding test and setting up a CI pipeline to check new commits against those tests.

Tests

Should be fairly easy to add some basic tests around timeline time calculations and animations correctly running.

One idea is to import/adapt the wpt tests for ScrollTimeline and run them against the polyfill. I haven't fully thought this through but it seems possible albeit may require us to change tests in a way that we can't easily keep syncing them.

Test harness and CI

Key requirement here is to be able to test against all mainstream browsers.

In the past I have had success using Karma testing framework in conjunction with Sauce Labs to run tests against multiple browsers. But this was some times ago and there may be better solutions now.

@majido
Copy link
Collaborator Author

majido commented Mar 25, 2020

About the idea of reusing WPT idea. @flackr pointed out that web-animations-js polyfill is doing something like this.

The tricky bit is that we need to load the polyfill before the test runs. AFAICT, they are using Karma and configure that to load their own special testharnessreport.js that injects the polyfill using document.write() . This works because every wpt test loads testharnessreport.js at the top of the test.

Note that to use the above technique, there may be a need to change the polyfill so that it can be loaded synchronously though. From @flackr :

One other point of contention is that the scroll-timeline polyfill right now loads as a module (read: asynchronously). I'm not attached to this - it seems as though it would be more useful to load synchronously into the main context to make it easier to load prior to running tests.

@zouhir zouhir self-assigned this Mar 27, 2020
@zouhir zouhir added the deliverable A medium+ feature or an item that can be broken into smaller tasks label Apr 2, 2020
@flackr
Copy link
Owner

flackr commented Oct 23, 2020

@zouhir Is this complete now?

@zouhir
Copy link
Collaborator

zouhir commented Oct 26, 2020

Yes, having I recall we agreed in a past Slack conversation that local unit-tests are redundant given we run WPT. Do you still think so?

@flackr
Copy link
Owner

flackr commented Oct 26, 2020

Yes, I think so! Thanks again for all your work here.

We can close the dependent testing bugs too right?

@zouhir
Copy link
Collaborator

zouhir commented Oct 26, 2020

Yep!

@zouhir zouhir closed this as completed Oct 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deliverable A medium+ feature or an item that can be broken into smaller tasks
Projects
None yet
Development

No branches or pull requests

3 participants