Skip to content

test(devtools): Fix intermittent DevTools API test timeout#177

Merged
Rob--W merged 6 commits intomozilla:masterfrom
EB-Forks:test/fix-devtools-timeout
Mar 20, 2019
Merged

test(devtools): Fix intermittent DevTools API test timeout#177
Rob--W merged 6 commits intomozilla:masterfrom
EB-Forks:test/fix-devtools-timeout

Conversation

@ExE-Boss
Copy link
Contributor

@ExE-Boss ExE-Boss commented Feb 5, 2019

This fixes the intermittent timeout issue that caused a #171 test to fail.

External links:

@Rob--W
Copy link
Member

Rob--W commented Feb 6, 2019

What makes you think that 1000ms fixes the issue (where 500ms did not)? The test waits for the devtools page to initialize. I would not be surprised if this takes more than a second on very slow machines (in rare cases). What about raising the timeout to, say, 3 seconds? It's still relatively low, but significantly more than 0.5 or 1 second.

@Rob--W
Copy link
Member

Rob--W commented Feb 7, 2019

Your patch doesn't work because the test function is defined here, without return value:

// Export as a global a wrapped test function which enforces a timeout by default.
window.test = (desc, fn) => {
tape(`${desc} (${browser})`, async (t) => {
t.timeoutAfter(DEFAULT_TIMEOUT);
await fn(t);
});
};

To fix this, I think that you can use the timeout option instead of explicitly calling timeoutAfter, see https://github.com/substack/tape/blob/v4.5.0/readme.markdown#testname-opts-cb and _timeout / timeoutAfter in https://github.com/substack/tape/blob/v4.5.0/lib/test.js

@ExE-Boss ExE-Boss changed the title test(devtools): Fix non‑deterministic devtools API test timeout test(devtools): Fix intermittent devtools API test timeout Feb 7, 2019
@ExE-Boss ExE-Boss force-pushed the test/fix-devtools-timeout branch from e3f1fd9 to 36dfe0a Compare February 8, 2019 22:43
@ExE-Boss ExE-Boss changed the title test(devtools): Fix intermittent devtools API test timeout test(devtools): Fix intermittent DevTools API test timeout Feb 8, 2019
@Rob--W
Copy link
Member

Rob--W commented Feb 28, 2019

@ExE-Boss Have you already made some changes? The PR hasn't been updated yet.

Copy link
Member

@Rob--W Rob--W left a comment

Choose a reason for hiding this comment

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

Almost there!

Copy link
Member

@Rob--W Rob--W left a comment

Choose a reason for hiding this comment

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

Thanks for your patch.

@Rob--W Rob--W merged commit 3aad241 into mozilla:master Mar 20, 2019
@ExE-Boss ExE-Boss deleted the test/fix-devtools-timeout branch March 20, 2019 11:40
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

Successfully merging this pull request may close these issues.

2 participants