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.

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

Run the internet tests on CI from time to time? #13061

Closed
Trott opened this issue May 16, 2017 · 15 comments
Closed

Run the internet tests on CI from time to time? #13061

Trott opened this issue May 16, 2017 · 15 comments
Assignees
Labels
test Issues and PRs related to the tests.

Comments

@Trott
Copy link
Member

Trott commented May 16, 2017

Ideas for making this happen:

  • Run them when cutting a release? @nodejs/release
  • Run them once a day on CI, perhaps as part of the node-daily-master job? @nodejs/build

That's all I have so far.

/cc @nodejs/testing

@Trott Trott added discuss Issues opened for discussions and feedbacks. test Issues and PRs related to the tests. labels May 16, 2017
@cjihrig
Copy link
Contributor

cjihrig commented May 16, 2017

+1 to running them once a day.

@targos
Copy link
Member

targos commented Sep 3, 2017

Ping. Did anything happen to resolve this?

@gibfahn gibfahn self-assigned this Sep 3, 2017
@Trott Trott removed the discuss Issues opened for discussions and feedbacks. label Mar 11, 2018
@Trott
Copy link
Member Author

Trott commented Jun 3, 2018

Since we're talking about running debug tests nightly, and/or tests with --gc-interval nightly, I guess this is a good time to raise the profile of this request too. @nodejs/build

@maclover7
Copy link
Contributor

Wasn't too hard to setup, 18/21 tests are passing: https://ci.nodejs.org/job/node-test-commit-custom-suites/6/

@Trott
Copy link
Member Author

Trott commented Jun 3, 2018

Awesome, thanks!

The two dgram failures may be making assumptions about network configuration that aren't true on our CI hosts. I can try to mess around with those a bit to see what might be done.

The dns failure is a straight up failing test that we just haven't noticed until now because we don't run the internet tests ever. :-D I'll take a look at that one too.

@Trott
Copy link
Member Author

Trott commented Jun 3, 2018

PR to fix dns test: #21116

@maclover7
Copy link
Contributor

Looks like all internet tests are passing now -- should this be added onto node-daily-master runs?

@Trott
Copy link
Member Author

Trott commented Jul 12, 2018

Looks like all internet tests are passing now -- should this be added onto node-daily-master runs?

The results say that internet/test-inspector-help-page is being skipped because it is being run inside a worker. That can't be right, can it? Is this a bug somehow in common.skipIfInspectorDisabled()/common.isMainThread()/worker_threads.isMainThread?

/cc @addaleax

@Trott
Copy link
Member Author

Trott commented Jul 12, 2018

Aside from the worker_threads question above, I'm 👍 on moving to node-daily-master!

@Trott
Copy link
Member Author

Trott commented Jul 12, 2018

The inspector test runs fine on my computer and is not skipped, so I'm guessing either the possible bug is OS-specific or else there is no bug and we really are running it in a worker thread?

@addaleax
Copy link
Member

@Trott Yes, looks like that’s what’s happening:

python tools/test.py -j 4 -p tap --logfile test.tap --mode=release --flaky-tests=dontcare --worker internet

But then again --worker was also passed in as TEST_ARGS in the UI, it seems?

@Trott
Copy link
Member Author

Trott commented Jul 12, 2018

Ah, OK, yeah, I can emulate the skipping on my machine with those command line options. Should have pulled that from the console in the first place. Sorry/thanks!

@maclover7 Running the tests locally without --worker, they all pass and that one that currently gets skipped is not skipped anymore. Any chance we can make that change?

@maclover7
Copy link
Contributor

@maclover7 Running the tests locally without --worker, they all pass and that one that currently gets skipped is not skipped anymore. Any chance we can make that change?

Ah, forgot that --worker is a default argument for the job. Re-ran without the arg, all tests seem to pass still.

Is there any advantage (since the job only takes ~2 minutes) to running once with --worker, and once without?

@addaleax
Copy link
Member

@maclover7 I don’t think there’s any particular benefit to running the internet tests in those configurations … we should keep going on nodejs/build#1318 to get --worker support into node-test-commit and then we don’t really need to worry about it for other jobs anymore

@maclover7
Copy link
Contributor

First run of node-daily-master with the internet tests is all green 🎉🎉🎉 https://ci.nodejs.org/job/node-daily-master/1221/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
test Issues and PRs related to the tests.
Projects
None yet
Development

No branches or pull requests

6 participants