theintern / intern

A next-generation code testing stack for JavaScript.
https://theintern.io/
Other
4.36k stars 311 forks source link

Add configuration for websocket timeout #1176

Closed rhpijnacker closed 4 years ago

rhpijnacker commented 4 years ago

There are cases where it is useful/necessary to set the timeout on acknowledgments from the server when data is sent over the WebSocket. This PR makes it possible to configure this value.

codecov-io commented 4 years ago

Codecov Report

Merging #1176 into 4.x will decrease coverage by 17.21%. The diff coverage is 97.05%.

Impacted file tree graph

@@             Coverage Diff             @@
##              4.x    #1176       +/-   ##
===========================================
- Coverage   66.70%   49.48%   -17.22%     
===========================================
  Files          61       61               
  Lines        4980     4987        +7     
  Branches     1109     1111        +2     
===========================================
- Hits         3322     2468      -854     
- Misses       1658     2519      +861     
Impacted Files Coverage Δ
src/lib/RemoteSuite.ts 0.00% <ø> (ø)
src/lib/channels/WebSocket.ts 96.15% <66.66%> (-1.89%) :arrow_down:
src/lib/common/util.ts 88.61% <100.00%> (+0.02%) :arrow_up:
src/lib/executors/Node.ts 92.26% <100.00%> (+0.59%) :arrow_up:
src/loaders/dojo.ts 100.00% <100.00%> (ø)
src/loaders/dojo2.ts 100.00% <100.00%> (ø)
src/loaders/systemjs.ts 95.23% <100.00%> (+0.23%) :arrow_up:
src/lib/reporters/html/icons.ts 0.00% <0.00%> (-100.00%) :arrow_down:
src/lib/reporters/Dom.ts 0.00% <0.00%> (-93.45%) :arrow_down:
src/lib/browser/util.ts 0.00% <0.00%> (-93.43%) :arrow_down:
... and 12 more

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update ef37c6a...3457f55. Read the comment docs.

jason0x43 commented 4 years ago

Thanks for fixing the schema and line endings.

Is there a need to use || in this.timeout = options.timeout || 10000; rather than ???

rhpijnacker commented 4 years ago

When I use ?? it does not compile anymore. (I commented on that above. Also, I would like your input on some other questions.)

jason0x43 commented 4 years ago

Hmm...what version of TS are you building with (?? is supported by TS 3.7+), and what error does it give you?

rhpijnacker commented 4 years ago

Looks like I'm on "Version 3.5.2" (after doing a fresh npm ci).

The error is:

[0] 5:21:12 PM - [tsc:.] src/lib/channels/WebSocket.ts(21,37): error TS1109: Expression expected.
[0] 5:21:12 PM - [tsc:.] src/lib/channels/WebSocket.ts(21,44): error TS1005: ':' expected.
[0] 5:21:12 PM - Error running tsc:., exiting...
jason0x43 commented 4 years ago

Hmmm...yeah, the TS version is set by intern-dev. Let's restore the original logic, then. While ?? is equivalent to the original, || is not (it won't allow a timeout of 0, for example).