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 test for drawing delay with CSS-only zoom #18129

Merged
merged 1 commit into from
May 21, 2024

Conversation

nicolo-ribaudo
Copy link
Contributor

@nicolo-ribaudo nicolo-ribaudo commented May 21, 2024

This commit adds a test for nicolo-ribaudo@0603d1a. Before the fix the pagerendered events would be fired just 2-3 milliseconds after the call to increaseScale/decreaseScale.

I know that you are trying to move away from waitForTimeout, but I don't know how else to test this. I cannot simply wait for an event to be emitted, because I am testing that it is not emitted.

@nicolo-ribaudo nicolo-ribaudo marked this pull request as draft May 21, 2024 11:53
@nicolo-ribaudo
Copy link
Contributor Author

Ok I think I can remove the waitForTimeouts

@nicolo-ribaudo nicolo-ribaudo force-pushed the test-css-only-zoom branch 2 times, most recently from 7d4e7ed to 247709b Compare May 21, 2024 12:15
@nicolo-ribaudo nicolo-ribaudo marked this pull request as ready for review May 21, 2024 12:15
@timvandermeij
Copy link
Contributor

/botio integrationtest

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Linux m4)


Received

Command cmd_integrationtest from @timvandermeij received. Current queue size: 0

Live output at: http://54.241.84.105:8877/2d8dd9ddf79998b/output.txt

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Windows)


Received

Command cmd_integrationtest from @timvandermeij received. Current queue size: 0

Live output at: http://54.193.163.58:8877/2b85a9305b92866/output.txt

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Linux m4)


Success

Full output at http://54.241.84.105:8877/2d8dd9ddf79998b/output.txt

Total script time: 7.35 mins

  • Integration Tests: Passed

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Windows)


Failed

Full output at http://54.193.163.58:8877/2b85a9305b92866/output.txt

Total script time: 20.96 mins

  • Integration Tests: FAILED

@nicolo-ribaudo
Copy link
Contributor Author

Oh windows, I'll check what it is about.

@timvandermeij
Copy link
Contributor

Isn't the problem that the first page can already be done rendering before the promise in the test is awaited because we already register the event handler in beforeEach and increment the counter there? Unless I'm mistaken the beforeEach handler can already eat the first pagerendered event before createPromiseForFirstPageRendered is called and then we basically attach to the event bus too late.

@calixteman
Copy link
Contributor

Maybe you could use a one-page pdf (for example empty.pdf) and check for pair of events pagerender/pagerendered.

This commit adds a test for 0603d1a.
Before the fix the `pagerendered` events would be fired just 2-3
milliseconds after the call to `increaseScale`/`decreaseScale`.
@nicolo-ribaudo
Copy link
Contributor Author

I noticed that pagerendered events have a timestamp, so I rewrote the test to instead measure that the timestamp is at least drawingDelayms after I called increaseScale/decreaseScale.

@timvandermeij
Copy link
Contributor

/botio integrationtest

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Linux m4)


Received

Command cmd_integrationtest from @timvandermeij received. Current queue size: 0

Live output at: http://54.241.84.105:8877/4b4c22a25aed16c/output.txt

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Windows)


Received

Command cmd_integrationtest from @timvandermeij received. Current queue size: 0

Live output at: http://54.193.163.58:8877/83ed3ac09b607fb/output.txt

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Linux m4)


Success

Full output at http://54.241.84.105:8877/4b4c22a25aed16c/output.txt

Total script time: 7.40 mins

  • Integration Tests: Passed

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Windows)


Success

Full output at http://54.193.163.58:8877/83ed3ac09b607fb/output.txt

Total script time: 20.56 mins

  • Integration Tests: Passed

@timvandermeij timvandermeij merged commit 2ad4601 into mozilla:master May 21, 2024
7 checks passed
@timvandermeij
Copy link
Contributor

Thank you for implementing this test!

@timvandermeij timvandermeij linked an issue Jun 19, 2024 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Introduce tests for CSS-only zooming
5 participants