Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
More Tests for WebRTC Data Channels #13499
More Tests for WebRTC Data Channels #13499
Changes from 86 commits
c6f99c9
76b2993
7dc4f16
3fd7117
f9c4238
8a3b2e4
1cd5a0b
e4c78a1
b048994
db84808
1c9b829
69eaff6
64b09f9
544d55a
60a2351
301f8fc
7cc5689
473b628
a56f956
f7341e4
04e1b39
9845d35
7ebfa52
387430a
31e9a22
cf96229
8e8a5e8
40a71ed
3ce8320
9212587
b94e547
b1fe750
249722e
3111c80
29a7968
3584fb1
b6f2f42
96bad22
85a3e30
32a1206
b86327a
ae1a97a
42240d8
0d2e28e
8ec6602
ce28e05
3342dba
44b5417
853f116
205b39a
c3b55b9
75658d5
13a886e
47bf993
93d9f87
7919b3f
b37ba46
b4d8cc0
7bcbcef
42bbfb9
d1ca251
528ea03
5de34a6
37b6557
e885c51
d15ee5d
5d97a94
0b1288b
06974b4
d0eb5c4
7eca2a0
e88f65a
e92ed40
0d56280
2c48917
ab8b310
d7bf409
547b006
8d29c80
4837794
99bd7d9
188d507
2106a86
4ed8674
0ccfa92
305373c
04639f2
cab92a7
e1fc283
8519923
99d792a
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we need this one?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed (in this file).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
However, in general this exists to allow a per-test timeout instead of the inflexible global per-file timeout which can be either 10 seconds or 60 seconds. This is insufficient for a test file with many promise-based test cases that can only run sequentially. Therefore, these files provide a timeout for each test case and disable the global timeout.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we fear that timing out subtests is an issue, we should simply split subtests in different files.
The per-file timeout is different depending on the CI/platform/configuration which makes it difficult to get per subtest meaningful values.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't forget that a
promise_test
blocks all subsequent tests. This means, if apromise_test
does not have a local timeout and it does time out, it prevents all other tests from running. If we don't want local timeouts, then we have to move each test case into its own file. And since there are a lot of tests that share the vast majority of their code, that does not seem feasible.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In practice, timing out subtests do happen.
Browser implementors tend to try fixing the corresponding issues sooner than regular failures.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I very much like the thought of browser vendors fixing their implementations due to tests timing out but don't consider that approach a good thing. Anyhow, I have moved this discussion further down into the comments since I want to hear some more opinions on that.