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.

Sign up for GitHub

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

Jump to bottom

Don't ignore errors in the Jasmine suite start/end stages #18321

Merged
merged 1 commit into from
Jun 24, 2024
Merged

Don't ignore errors in the Jasmine suite start/end stages #18321

merged 1 commit into from
Jun 24, 2024

Conversation

timvandermeij
Copy link
Contributor

Currently errors in afterAll are logged, but don't fail the tests. This could cause new errors during test teardown to go by unnoticed.

Moreover, the integration test use a different reporting mechanism which also handled errors differently (this is extra reason to do #12730).

This patch fixes the issues by consistently handling errors in suiteStarted and suiteDone in both reporting mechanisms.

Fixes #18319.

@timvandermeij
Copy link
Contributor Author

Before this patch:

Suite error: Stamp Editor Copy/paste from a tab to an other
  Message:
    ProtocolError: Runtime.callFunctionOn timed out. Increase the 'protocolTimeout' setting in launch/connect calls for a higher timeout if needed.
  Stack:
        at <instance_members_initializer> (file:///home/timvandermeij/Documenten/Ontwikkeling/pdf.js/Code/node_modules/puppeteer-core/lib/esm/puppeteer/common/CallbackRegistry.js:89:14)
        at new Callback (file:///home/timvandermeij/Documenten/Ontwikkeling/pdf.js/Code/node_modules/puppeteer-core/lib/esm/puppeteer/common/CallbackRegistry.js:93:16)
        at CallbackRegistry.create (file:///home/timvandermeij/Documenten/Ontwikkeling/pdf.js/Code/node_modules/puppeteer-core/lib/esm/puppeteer/common/CallbackRegistry.js:19:26)
        at Connection._rawSend (file:///home/timvandermeij/Documenten/Ontwikkeling/pdf.js/Code/node_modules/puppeteer-core/lib/esm/puppeteer/cdp/Connection.js:86:26)
        at CdpCDPSession.send (file:///home/timvandermeij/Documenten/Ontwikkeling/pdf.js/Code/node_modules/puppeteer-core/lib/esm/puppeteer/cdp/CDPSession.js:63:33)
        at #evaluate (file:///home/timvandermeij/Documenten/Ontwikkeling/pdf.js/Code/node_modules/puppeteer-core/lib/esm/puppeteer/cdp/ExecutionContext.js:353:50)
        at ExecutionContext.evaluate (file:///home/timvandermeij/Documenten/Ontwikkeling/pdf.js/Code/node_modules/puppeteer-core/lib/esm/puppeteer/cdp/ExecutionContext.js:268:36)
        at IsolatedWorld.evaluate (file:///home/timvandermeij/Documenten/Ontwikkeling/pdf.js/Code/node_modules/puppeteer-core/lib/esm/puppeteer/cdp/IsolatedWorld.js:96:30)
        at CdpFrame.evaluate (file:///home/timvandermeij/Documenten/Ontwikkeling/pdf.js/Code/node_modules/puppeteer-core/lib/esm/puppeteer/api/Frame.js:335:43)
        at CdpFrame.<anonymous> (file:///home/timvandermeij/Documenten/Ontwikkeling/pdf.js/Code/node_modules/puppeteer-core/lib/esm/puppeteer/util/decorators.js:94:27)

Ran 1 of 193 specs
1 spec, 1 failure
Finished in 25.025 seconds

Run 193 tests
All integration tests passed.
integration tests runtime was 28.8 second

After this patch:

Suite error: Stamp Editor Copy/paste from a tab to an other
  Message:
    ProtocolError: Runtime.callFunctionOn timed out. Increase the 'protocolTimeout' setting in launch/connect calls for a higher timeout if needed.
  Stack:
        at <instance_members_initializer> (file:///home/timvandermeij/Documenten/Ontwikkeling/pdf.js/Code/node_modules/puppeteer-core/lib/esm/puppeteer/common/CallbackRegistry.js:89:14)
        at new Callback (file:///home/timvandermeij/Documenten/Ontwikkeling/pdf.js/Code/node_modules/puppeteer-core/lib/esm/puppeteer/common/CallbackRegistry.js:93:16)
        at CallbackRegistry.create (file:///home/timvandermeij/Documenten/Ontwikkeling/pdf.js/Code/node_modules/puppeteer-core/lib/esm/puppeteer/common/CallbackRegistry.js:19:26)
        at Connection._rawSend (file:///home/timvandermeij/Documenten/Ontwikkeling/pdf.js/Code/node_modules/puppeteer-core/lib/esm/puppeteer/cdp/Connection.js:86:26)
        at CdpCDPSession.send (file:///home/timvandermeij/Documenten/Ontwikkeling/pdf.js/Code/node_modules/puppeteer-core/lib/esm/puppeteer/cdp/CDPSession.js:63:33)
        at #evaluate (file:///home/timvandermeij/Documenten/Ontwikkeling/pdf.js/Code/node_modules/puppeteer-core/lib/esm/puppeteer/cdp/ExecutionContext.js:353:50)
        at ExecutionContext.evaluate (file:///home/timvandermeij/Documenten/Ontwikkeling/pdf.js/Code/node_modules/puppeteer-core/lib/esm/puppeteer/cdp/ExecutionContext.js:268:36)
        at IsolatedWorld.evaluate (file:///home/timvandermeij/Documenten/Ontwikkeling/pdf.js/Code/node_modules/puppeteer-core/lib/esm/puppeteer/cdp/IsolatedWorld.js:96:30)
        at CdpFrame.evaluate (file:///home/timvandermeij/Documenten/Ontwikkeling/pdf.js/Code/node_modules/puppeteer-core/lib/esm/puppeteer/api/Frame.js:335:43)
        at CdpFrame.<anonymous> (file:///home/timvandermeij/Documenten/Ontwikkeling/pdf.js/Code/node_modules/puppeteer-core/lib/esm/puppeteer/util/decorators.js:94:27)

Ran 1 of 193 specs
1 spec, 1 failure
Finished in 25.006 seconds

Run 193 tests
OHNOES!  Some integration tests failed!
  1 of 193 failed
integration tests runtime was 28.4 seconds

@timvandermeij
Copy link
Contributor Author

/botio unittest

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Linux m4)

Received

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

Live output at: http://54.241.84.105:8877/78fea9e555a09fe/output.txt

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Windows)

Received

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

Live output at: http://54.193.163.58:8877/069db26419a504a/output.txt

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Linux m4)

Success

Full output at http://54.241.84.105:8877/78fea9e555a09fe/output.txt

Total script time: 2.46 mins

  • Unit Tests: Passed

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Windows)

Success

Full output at http://54.193.163.58:8877/069db26419a504a/output.txt

Total script time: 9.03 mins

  • Unit Tests: Passed

@timvandermeij
Copy link
Contributor Author

/botio integrationtest

@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/32a622470750b14/output.txt

@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/ed68e4d04bcad93/output.txt

@timvandermeij timvandermeij mentioned this pull request Jun 23, 2024
Open
@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Linux m4)

Failed

Full output at http://54.241.84.105:8877/ed68e4d04bcad93/output.txt

Total script time: 8.11 mins

  • Integration Tests: FAILED

@timvandermeij
Don't ignore errors in the Jasmine suite start/end stages
2f3bf6f 
Currently errors in `afterAll` are logged, but don't fail the tests.
This could cause new errors during test teardown to go by unnoticed.

Moreover, the integration test use a different reporting mechanism which
also handled errors differently (this is extra reason to do mozilla#12730).

This patch fixes the issues by consistently handling errors in
`suiteStarted` and `suiteDone` in both reporting mechanisms.

Fixes mozilla#18319.
@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Windows)

Failed

Full output at http://54.193.163.58:8877/32a622470750b14/output.txt

Total script time: 19.08 mins

  • Integration Tests: FAILED

@timvandermeij timvandermeij force-pushed the integration-test-ignored-errors branch from 7d6a4b3 to 2f3bf6f Compare June 23, 2024 19:06
@timvandermeij
Copy link
Contributor Author

Notice how the two runs above now fail correctly because this patch was deliberately not rebased onto #18320 yet: they now both log TEST-UNEXPECTED-FAIL | Copy/paste from a tab to an other and both end with OHNOES! Some integration tests failed!.

I have now rebased onto the current master branch with #18320, so now the integration tests should succeed for that test:

/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/29c184eadb031ca/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/61a2ff740e0b544/output.txt

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Linux m4)

Failed

Full output at http://54.241.84.105:8877/29c184eadb031ca/output.txt

Total script time: 8.13 mins

  • Integration Tests: FAILED

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Windows)

Failed

Full output at http://54.193.163.58:8877/61a2ff740e0b544/output.txt

Total script time: 18.69 mins

  • Integration Tests: FAILED

@Snuffleupagus
Copy link
Collaborator

/botio integrationtest

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Windows)

Received

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

Live output at: http://54.193.163.58:8877/ce23b2095f6d0b0/output.txt

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Linux m4)

Received

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

Live output at: http://54.241.84.105:8877/17aea804e0ca9b0/output.txt

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Linux m4)

Failed

Full output at http://54.241.84.105:8877/17aea804e0ca9b0/output.txt

Total script time: 8.25 mins

  • Integration Tests: FAILED

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Windows)

Failed

Full output at http://54.193.163.58:8877/ce23b2095f6d0b0/output.txt

Total script time: 18.84 mins

  • Integration Tests: FAILED

Snuffleupagus
Snuffleupagus approved these changes Jun 24, 2024
View reviewed changes
Copy link
Collaborator

@Snuffleupagus Snuffleupagus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

r=me, since the failures should be unrelated and this patch really is the correct thing to do; thank you!

@timvandermeij timvandermeij merged commit 8923f9a into mozilla:master Jun 24, 2024
7 checks passed
@timvandermeij timvandermeij deleted the integration-test-ignored-errors branch June 24, 2024 13:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Snuffleupagus Snuffleupagus Snuffleupagus approved these changes

Assignees
No one assigned
Labels
test
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Fix the integration tests passing despite one test failure
3 participants
@timvandermeij @moz-tools-bot @Snuffleupagus