ci: avoid using same port number for different net tests #4147

keroxp · 2020-02-26T14:50:13Z

Some net tests (cli,std) are unstable in any platform. I doubt that one of causes is they are using same port numbers during tests. I added port picker that iterates different port number in private port range (49152 - 65535) for cli and std.

And note that when cargo test is running, we are using addresses below by tools/http_server.ts:

PORT = 4545
REDIRECT_PORT = 4546
ANOTHER_REDIRECT_PORT = 4547
DOUBLE_REDIRECTS_PORT = 4548
INF_REDIRECTS_PORT = 4549
HTTPS_PORT = 5545

I also doubt this is the main cause of windows's too flaky CI on net tests. 🤔

keroxp · 2020-02-26T14:53:06Z

#4107 is related to this.

…icker-for-tests

keroxp · 2020-03-04T15:50:06Z

@ry How do you think?

…icker-for-tests

bartlomieju

@keroxp LGTM, with a few minor nitpicks

bartlomieju · 2020-03-18T10:39:12Z

cli/js/tests/test_util.ts

+}
+const it = portIterator();
+/** Obtain (maybe) safe port number for net tests */
+export function usePort(): number {


Please rename to randomPort

bartlomieju · 2020-03-18T10:39:53Z

std/http/internal/test_util.ts

@@ -0,0 +1,20 @@
+import { assert } from "../../testing/asserts.ts";


No need to put it inside internal/ directory, please lift one level up

bartlomieju · 2020-03-18T10:40:02Z

std/http/internal/test_util.ts

+}
+const it = portIterator();
+/** Obtain (maybe) safe port number for net tests */
+export function usePort(): number {


s/usePort/randomPort/

nayeemrmn · 2020-03-18T11:03:40Z

Are you sure this won't hide bugs? If resources are cleaned properly it should be okay to use the same port. Or maybe it can't be helped due to shutdowns not working properly on Windows, or something?

keroxp · 2020-03-18T13:09:51Z

@nayeemrmn No, thank you for pointing. I know tests around network are very unstable on windows. But what you concern about should be tested by one for that.

The motivation to use random port is that there are no rules to pick port from host and sometimes it conflicts existing local services. Port number between 1000 ~ 10000 are often taken by developer. It's slightly annoying to stop other services before running Deno's test.

Of course, that doesn't mean those ports are safe.
Basically 49152 (+20~30) for js/cli, 55001 (+20 ~ 30) for std will be used.

…icker-for-tests

keroxp · 2020-03-18T13:52:04Z

@bartlomieju Thank you for reviewing. I've applied reviews, please check!

ry · 2020-03-18T19:25:09Z

This should wait until #4373 is fixed.

I'm hesitant on this patch because it complicates the test setup without a clear benefit.

Sorry for the delayed review @keroxp ...

…icker-for-tests

ry · 2020-03-21T13:50:15Z

std/http/test_util.ts

+  const { value } = it.next();
+  assert(value != null);
+  return value;
+}


This could be done more simply without an iterator and a global variable...

ry

I'm a little unsure about what consequences this will have, but it seems fine. I misunderstood the issue at first, I thought this was about the ports used for tools/http_server.py which I think must be fixed (since they are shared across process and language boundaries).

LGTM - sorry for the delay @keroxp

This reverts commit 60cee4f.

Ref #4467 This reverts commit 60cee4f.

* denoland/master: (35 commits) Ignore flaky test cafile_info (denoland#4517) fix(inspector): proper error message on port collision (denoland#4514) feat: Added colors to doc output (denoland#4518) v0.38.0 feat: Add "deno doc" subcommand (denoland#4500) Update to Prettier 2 and use ES Private Fields (denoland#4498) upgrade: dprint 0.9.6 (denoland#4509) upgrade: rusty_v8 to v0.3.9 (denoland#4505) feat: Support Inspector / Chrome Devtools (denoland#4484) Improve isatty and kill API docs; Deno.kill() - throw on Windows (denoland#4497) refactor: rename ConsoleOptions to InspectOptions (denoland#4493) upgrade: dprint 0.9.5 (denoland#4491) feat: window.close() (denoland#4474) errors: replace .lines with explicit .split newline (denoland#4483) doc: improve various API docs and include examples (denoland#4486) hide source line if error message longer than 150 chars (denoland#4487) fix: add fsEvent notify::Error casts (denoland#4488) feat: add queueMicrotask to d.ts (denoland#4477) Revert "avoid using same port number for test (denoland#4147)" docs: update manual about how to run tests for std (denoland#4462) ...

Ref denoland/deno#4467 This reverts commit 60cee4f.

keroxp added 8 commits February 25, 2020 22:57

reorg: [http] move io functions to http/io.ts

8521877

Update mod.ts

05ec6bd

revert testGroup

49ba9d6

Update cookie_test.ts

21045f3

misc: reduce unnecessary stout output on tests (Part1)

473d614

wip

849c594

Merge branch 'reorg-http-io' into port-picker-for-tests

ac60fdd

misc: avoid using same port number for different net tests

f2d1f1b

keroxp added 4 commits February 27, 2020 00:25

Update file_server_test.ts

cf6db83

Update server_test.ts

1d57f3b

Merge branch 'master' of https://github.com/denoland/deno into port-p…

0132755

…icker-for-tests

Merge branch 'master' of https://github.com/denoland/deno into port-p…

e3f33bb

…icker-for-tests

bartlomieju requested a review from ry February 29, 2020 15:00

Merge branch 'master' of https://github.com/denoland/deno into port-p…

0b65b4e

…icker-for-tests

keroxp added 2 commits March 5, 2020 21:16

Merge branch 'master' of https://github.com/denoland/deno into port-p…

217ae4c

…icker-for-tests

Update net_test.ts

7f59c39

uki00a mentioned this pull request Mar 15, 2020

[WIP] fix: CI failures on windows #4107

Closed

keroxp added 3 commits March 17, 2020 01:52

Merge branch 'master' of https://github.com/denoland/deno into port-p…

21e7c98

…icker-for-tests

Update net_test.ts

4769e74

Update net_test.ts

b8ff099

bartlomieju reviewed Mar 18, 2020

View reviewed changes

keroxp added 3 commits March 18, 2020 22:12

Merge branch 'master' of https://github.com/denoland/deno into port-p…

5352225

…icker-for-tests

apply PR reviews

209e5fe

fmt

a3286e9

keroxp requested a review from bartlomieju March 18, 2020 13:50

keroxp added 3 commits March 20, 2020 12:39

Merge branch 'master' of https://github.com/denoland/deno into port-p…

fbbc9e9

…icker-for-tests

fmt

a68c25b

Merge branch 'master' of https://github.com/denoland/deno into port-p…

c09c931

…icker-for-tests

ry reviewed Mar 21, 2020

View reviewed changes

ry approved these changes Mar 21, 2020

View reviewed changes

ry merged commit 60cee4f into denoland:master Mar 21, 2020

keroxp mentioned this pull request Mar 22, 2020

fix: re-enable chast server example tests #4457

Closed

bartlomieju added a commit to bartlomieju/deno that referenced this pull request Mar 24, 2020

Revert "avoid using same port number for test (denoland#4147)"

cc044af

This reverts commit 60cee4f.

bartlomieju added a commit to bartlomieju/deno that referenced this pull request Mar 24, 2020

Revert "avoid using same port number for test (denoland#4147)"

71fc15b

This reverts commit 60cee4f.

ry pushed a commit that referenced this pull request Mar 24, 2020

Revert "avoid using same port number for test (#4147)"

30bcf6a

Ref #4467 This reverts commit 60cee4f.

caspervonb pushed a commit to caspervonb/deno_std that referenced this pull request Jan 21, 2021

avoid using same port number for test (denoland/deno#4147)

b291133

caspervonb pushed a commit to caspervonb/deno_std that referenced this pull request Jan 21, 2021

Revert "avoid using same port number for test (denoland/deno#4147)"

aba948a

Ref denoland/deno#4467 This reverts commit 60cee4f.

caspervonb pushed a commit to caspervonb/deno_std that referenced this pull request Jan 24, 2021

avoid using same port number for test (denoland/deno#4147)

f34db0d

caspervonb pushed a commit to caspervonb/deno_std that referenced this pull request Jan 24, 2021

Revert "avoid using same port number for test (denoland/deno#4147)"

ce6ca71

Ref denoland/deno#4467 This reverts commit 60cee4f.

caspervonb pushed a commit to caspervonb/deno_std that referenced this pull request Jan 24, 2021

avoid using same port number for test (denoland/deno#4147)

19f44b4

caspervonb pushed a commit to caspervonb/deno_std that referenced this pull request Jan 24, 2021

Revert "avoid using same port number for test (denoland/deno#4147)"

fb161cf

Ref denoland/deno#4467 This reverts commit 60cee4f.

caspervonb pushed a commit to caspervonb/deno_std that referenced this pull request Jan 24, 2021

avoid using same port number for test (denoland/deno#4147)

352c071

caspervonb pushed a commit to caspervonb/deno_std that referenced this pull request Jan 24, 2021

Revert "avoid using same port number for test (denoland/deno#4147)"

e321f85

Ref denoland/deno#4467 This reverts commit 60cee4f.

caspervonb pushed a commit to caspervonb/deno_std that referenced this pull request Jan 31, 2021

avoid using same port number for test (denoland/deno#4147)

71689fd

caspervonb pushed a commit to caspervonb/deno_std that referenced this pull request Jan 31, 2021

Revert "avoid using same port number for test (denoland/deno#4147)"

3b353f2

Ref denoland/deno#4467 This reverts commit 60cee4f.

caspervonb pushed a commit to caspervonb/deno_std that referenced this pull request Jan 31, 2021

avoid using same port number for test (denoland/deno#4147)

fbd6bca

caspervonb pushed a commit to caspervonb/deno_std that referenced this pull request Jan 31, 2021

Revert "avoid using same port number for test (denoland/deno#4147)"

5a4217e

Ref denoland/deno#4467 This reverts commit 60cee4f.

caspervonb pushed a commit to caspervonb/deno_std that referenced this pull request Jan 31, 2021

avoid using same port number for test (denoland/deno#4147)

d6e2981

caspervonb pushed a commit to caspervonb/deno_std that referenced this pull request Jan 31, 2021

Revert "avoid using same port number for test (denoland/deno#4147)"

a2be114

Ref denoland/deno#4467 This reverts commit 60cee4f.

caspervonb pushed a commit to caspervonb/deno_std that referenced this pull request Jan 31, 2021

avoid using same port number for test (denoland/deno#4147)

e6da178

caspervonb pushed a commit to caspervonb/deno_std that referenced this pull request Jan 31, 2021

Revert "avoid using same port number for test (denoland/deno#4147)"

528e4ae

Ref denoland/deno#4467 This reverts commit 60cee4f.

caspervonb pushed a commit to caspervonb/deno_std that referenced this pull request Feb 1, 2021

avoid using same port number for test (denoland/deno#4147)

c915706

caspervonb pushed a commit to caspervonb/deno_std that referenced this pull request Feb 1, 2021

Revert "avoid using same port number for test (denoland/deno#4147)"

2187125

Ref denoland/deno#4467 This reverts commit 60cee4f.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ci: avoid using same port number for different net tests #4147

ci: avoid using same port number for different net tests #4147

keroxp commented Feb 26, 2020 •

edited

Loading

keroxp commented Feb 26, 2020

keroxp commented Mar 4, 2020

bartlomieju left a comment

bartlomieju Mar 18, 2020

bartlomieju Mar 18, 2020

bartlomieju Mar 18, 2020

nayeemrmn commented Mar 18, 2020

keroxp commented Mar 18, 2020

keroxp commented Mar 18, 2020

ry commented Mar 18, 2020

ry Mar 21, 2020

ry left a comment

		@@ -0,0 +1,20 @@
		import { assert } from "../../testing/asserts.ts";

ci: avoid using same port number for different net tests #4147

ci: avoid using same port number for different net tests #4147

Conversation

keroxp commented Feb 26, 2020 • edited Loading

keroxp commented Feb 26, 2020

keroxp commented Mar 4, 2020

bartlomieju left a comment

Choose a reason for hiding this comment

bartlomieju Mar 18, 2020

Choose a reason for hiding this comment

bartlomieju Mar 18, 2020

Choose a reason for hiding this comment

bartlomieju Mar 18, 2020

Choose a reason for hiding this comment

nayeemrmn commented Mar 18, 2020

keroxp commented Mar 18, 2020

keroxp commented Mar 18, 2020

ry commented Mar 18, 2020

ry Mar 21, 2020

Choose a reason for hiding this comment

ry left a comment

Choose a reason for hiding this comment

keroxp commented Feb 26, 2020 •

edited

Loading