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

test/doctool/test-doctool-html requires internet connection? #29918

Closed
joyeecheung opened this issue Oct 10, 2019 · 10 comments · Fixed by #30214
Closed

test/doctool/test-doctool-html requires internet connection? #29918

joyeecheung opened this issue Oct 10, 2019 · 10 comments · Fixed by #30214
Labels
build Issues and PRs related to build files or the CI. doc Issues and PRs related to the documentations. tools Issues and PRs related to the tools directory.

Comments

@joyeecheung
Copy link
Member

I got this today:

/Applications/Xcode.app/Contents/Developer/usr/bin/make -s test-doc
Running JS linter...
Running C++ linter...
Running Markdown linter on misc docs...
Running Markdown linter on docs...
=== release test-doctool-html ===
Path: doctool/test-doctool-html
Failed to add alternative version links to foo
Failed to add alternative version links to foo
Failed to add alternative version links to foo
Failed to add alternative version links to foo
/Users/joyee/projects/node/test/common/index.js:710
const crashOnUnhandledRejection = (err) => { throw err; };
                                             ^

Error: connect ETIMEDOUT 151.101.228.133:443
    at TCPConnectWrap.afterConnect [as oncomplete] (net.js:1128:14) {
  errno: -60,
  code: 'ETIMEDOUT',
  syscall: 'connect',
  address: '151.101.228.133',
  port: 443
}
Command: out/Release/node /Users/joyee/projects/node/test/doctool/test-doctool-html.js
[01:17|% 100|+   3|-   1]: Done
make[1]: *** [test-doc] Error 1
make: *** [test] Error 2

And if I turn off Wifi..

NODE_DEBUG=net out/Release/node /Users/joyee/projects/node/test/doctool/test-doctool-html.js
Failed to add alternative version links to foo
Failed to add alternative version links to foo
NET 34507: pipe false null
NET 34507: connect: find host raw.githubusercontent.com
NET 34507: connect: dns options { family: undefined, hints: 1024 }
NET 34507: _read
NET 34507: _read wait for connection
Failed to add alternative version links to foo
Failed to add alternative version links to foo
NET 34507: destroy
NET 34507: close
NET 34507: close handle
/Users/joyee/projects/node/test/common/index.js:710
const crashOnUnhandledRejection = (err) => { throw err; };
                                             ^

Error: getaddrinfo ENOTFOUND raw.githubusercontent.com
    at GetAddrInfoReqWrap.onlookup [as oncomplete] (dns.js:60:26) {
  errno: -3008,
  code: 'ENOTFOUND',
  syscall: 'getaddrinfo',
  hostname: 'raw.githubusercontent.com'
}

Has there been any changes that add this new requirement? It seems weird to require internet connection to run the doc tests.

@richardlau
Copy link
Member

Has there been any changes that add this new requirement? It seems weird to require internet connection to run the doc tests.

#27661

@joyeecheung joyeecheung added build Issues and PRs related to build files or the CI. doc Issues and PRs related to the documentations. tools Issues and PRs related to the tools directory. labels Oct 11, 2019
@joyeecheung
Copy link
Member Author

joyeecheung commented Oct 11, 2019

cc @nodejs/testing

@richardlau Thanks for the pointer. Can we use something in the file system instead? (That is to go back to what #27661 originally did, cc @BridgeAR ). In my opinion, the default test suites should not require an Internet connection , otherwise it should be placed under test/internet - not every access point guarantees stable, unpolluted connection to raw.githubusercontent.com and requiring the connection to it in the default test suite leads to false failures that have nothing to do with the code.

@richardlau
Copy link
Member

It's not the test suite but the actual doctool that is making the connection (the test added by that PR was placed in internet but obviously with hindsight existing tests that ran the tool were affected). Also doctool tests are not in the default set of test suites.

I guess the question is whether we care whether or not that the API docs can point to a newer release line? e.g. the 12.x docs pointing to the 13.x equivalent in the "View another version" drop down -- the issue with the file system is the changelog in the 12.x branch will not have a reference to 13.x (even after it is released as we don't backport those kinds of changes).

Open to any suggestions. The original aim of #27661 was to reduce the manual effort in keeping the version picker in the docs accurate.

@Trott

This comment has been minimized.

@Trott

This comment has been minimized.

@Trott

This comment has been minimized.

@Trott

This comment has been minimized.

@joyeecheung
Copy link
Member Author

joyeecheung commented Nov 2, 2019

I am preparing for the code and learn tomorrow and I noticed that this may cause issues with the conference WiFi where connections to raw.githubusercontent.com result in ETIMEOUT. I'll ask the participants to use make test-only for now (even then this still shows up but it does not stop the build)

@richardlau
Copy link
Member

PR: #30214

@joyeecheung
Copy link
Member Author

#30214 still does not fix the issue: versions() is run in multiple processes when building the docs and it's still excruciatingly slow to wait for the fallback (this already took minutes for me and it is still running).

Screen Shot 2019-11-10 at 4 01 47 PM

@joyeecheung joyeecheung reopened this Nov 10, 2019
MylesBorins pushed a commit that referenced this issue Nov 17, 2019
Allow doctool to fallback to use local files if not building a release
build.

PR-URL: #30214
Fixes: #29918
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Gus Caplan <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Jiawen Geng <[email protected]>
targos pushed a commit that referenced this issue Dec 1, 2019
Allow doctool to fallback to use local files if not building a release
build.

PR-URL: #30214
Fixes: #29918
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Gus Caplan <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Jiawen Geng <[email protected]>
MylesBorins pushed a commit that referenced this issue Dec 17, 2019
Allow doctool to fallback to use local files if not building a release
build.

PR-URL: #30214
Fixes: #29918
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Gus Caplan <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Jiawen Geng <[email protected]>
codebytere pushed a commit that referenced this issue Feb 27, 2020
At the moment the doc builder tries to access the internet
for CHANGELOG information and only falls back to local sources
after the connection fails or a 5 second timeout. This means
that the doc building could take at least 7 minutes on a
machine with hijacked connection to Github for useless network
attempts. This patch adds a NODE_TEST_NO_INTERNET environment
variable to directly bypass these attempts so that docs can be
built in reasonable time on a machine like that.

PR-URL: #31849
Fixes: #29918
Reviewed-By: Matheus Marchini <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
codebytere pushed a commit that referenced this issue Mar 15, 2020
At the moment the doc builder tries to access the internet
for CHANGELOG information and only falls back to local sources
after the connection fails or a 5 second timeout. This means
that the doc building could take at least 7 minutes on a
machine with hijacked connection to Github for useless network
attempts. This patch adds a NODE_TEST_NO_INTERNET environment
variable to directly bypass these attempts so that docs can be
built in reasonable time on a machine like that.

PR-URL: #31849
Fixes: #29918
Reviewed-By: Matheus Marchini <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
codebytere pushed a commit that referenced this issue Mar 17, 2020
At the moment the doc builder tries to access the internet
for CHANGELOG information and only falls back to local sources
after the connection fails or a 5 second timeout. This means
that the doc building could take at least 7 minutes on a
machine with hijacked connection to Github for useless network
attempts. This patch adds a NODE_TEST_NO_INTERNET environment
variable to directly bypass these attempts so that docs can be
built in reasonable time on a machine like that.

PR-URL: #31849
Fixes: #29918
Reviewed-By: Matheus Marchini <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
codebytere pushed a commit that referenced this issue Mar 30, 2020
At the moment the doc builder tries to access the internet
for CHANGELOG information and only falls back to local sources
after the connection fails or a 5 second timeout. This means
that the doc building could take at least 7 minutes on a
machine with hijacked connection to Github for useless network
attempts. This patch adds a NODE_TEST_NO_INTERNET environment
variable to directly bypass these attempts so that docs can be
built in reasonable time on a machine like that.

PR-URL: #31849
Fixes: #29918
Reviewed-By: Matheus Marchini <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
richardlau added a commit to richardlau/node-1 that referenced this issue Apr 6, 2020
Allow doctool to fallback to use local files if not building a release
build.

Backport-PR-URL: nodejs#32642
PR-URL: nodejs#30214
Fixes: nodejs#29918
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Gus Caplan <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Jiawen Geng <[email protected]>
richardlau pushed a commit to richardlau/node-1 that referenced this issue Apr 6, 2020
At the moment the doc builder tries to access the internet
for CHANGELOG information and only falls back to local sources
after the connection fails or a 5 second timeout. This means
that the doc building could take at least 7 minutes on a
machine with hijacked connection to Github for useless network
attempts. This patch adds a NODE_TEST_NO_INTERNET environment
variable to directly bypass these attempts so that docs can be
built in reasonable time on a machine like that.

Backport-PR-URL: nodejs#32642
PR-URL: nodejs#31849
Fixes: nodejs#29918
Reviewed-By: Matheus Marchini <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
BethGriggs pushed a commit that referenced this issue Apr 6, 2020
Allow doctool to fallback to use local files if not building a release
build.

Backport-PR-URL: #32642
PR-URL: #30214
Fixes: #29918
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Gus Caplan <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Jiawen Geng <[email protected]>
BethGriggs pushed a commit that referenced this issue Apr 6, 2020
At the moment the doc builder tries to access the internet
for CHANGELOG information and only falls back to local sources
after the connection fails or a 5 second timeout. This means
that the doc building could take at least 7 minutes on a
machine with hijacked connection to Github for useless network
attempts. This patch adds a NODE_TEST_NO_INTERNET environment
variable to directly bypass these attempts so that docs can be
built in reasonable time on a machine like that.

Backport-PR-URL: #32642
PR-URL: #31849
Fixes: #29918
Reviewed-By: Matheus Marchini <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Issues and PRs related to build files or the CI. doc Issues and PRs related to the documentations. tools Issues and PRs related to the tools directory.
Projects
None yet
3 participants