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

Doctool tests require absent eslint #7201

Closed
jellelicht opened this issue Jun 7, 2016 · 1 comment
Closed

Doctool tests require absent eslint #7201

jellelicht opened this issue Jun 7, 2016 · 1 comment
Labels
build Issues and PRs related to build files or the CI. doc Issues and PRs related to the documentations. test Issues and PRs related to the tests. tools Issues and PRs related to the tools directory.

Comments

@jellelicht
Copy link

  • Version: 6.2.1
  • Platform: Linux veritas 3.19.0-59-generic deprecate domains #66~14.04.1-Ubuntu SMP Fri May 13 17:27:10 UTC 2016 x86_64 GNU/Linux
  • Subsystem:

It seems that several doctool tests depend on the availability of js-yaml in the source tree. js-yaml is supposedly included from the path tools/eslint/node_modules/js-yaml, which is not available in the source tarball. Running make lint in the source tree leads to the following error message:

Linting is not available through the source tarball.
Use the git repo instead: git clone https://github.com/nodejs/node.git

The actual offending test outputs, runnable via /usr/bin/python tools/test.py --mode=release -J doctool:

=== release test-doctool-html ===                    
Path: doctool/test-doctool-html
module.js:442
    throw err;
    ^

Error: Cannot find module '/home/jelle/temp/node-v6.2.1/tools/eslint/node_modules/js-yaml'
    at Function.Module._resolveFilename (module.js:440:15)
    at Function.Module._load (module.js:388:25)
    at Module.require (module.js:468:17)
    at require (internal/module.js:20:19)
    at Object.<anonymous> (/home/jelle/temp/node-v6.2.1/tools/doc/node_modules/js-yaml/index.js:15:18)
    at Module._compile (module.js:541:32)
    at Object.Module._extensions..js (module.js:550:10)
    at Module.load (module.js:458:32)
    at tryModuleLoad (module.js:417:12)
    at Function.Module._load (module.js:409:3)
Command: out/Release/node /home/jelle/temp/node-v6.2.1/test/doctool/test-doctool-html.js
=== release test-doctool-json ===                    
Path: doctool/test-doctool-json
module.js:442
    throw err;
    ^

Error: Cannot find module '/home/jelle/temp/node-v6.2.1/tools/eslint/node_modules/js-yaml'
    at Function.Module._resolveFilename (module.js:440:15)
    at Function.Module._load (module.js:388:25)
    at Module.require (module.js:468:17)
    at require (internal/module.js:20:19)
    at Object.<anonymous> (/home/jelle/temp/node-v6.2.1/tools/doc/node_modules/js-yaml/index.js:15:18)
    at Module._compile (module.js:541:32)
    at Object.Module._extensions..js (module.js:550:10)
    at Module.load (module.js:458:32)
    at tryModuleLoad (module.js:417:12)
    at Function.Module._load (module.js:409:3)

I would expect that these tests should rather be skipped when running tests from an extracted tarball, or rewritten so they do not depend on eslint. Otherwise, eslint could possibly be included instead.

/cc @jbergstroem @silverwind @thealphanerd #6031

@silverwind
Copy link
Contributor

@silverwind silverwind added the tools Issues and PRs related to the tools directory. label Jun 7, 2016
@mscdex mscdex added doc Issues and PRs related to the documentations. build Issues and PRs related to build files or the CI. test Issues and PRs related to the tests. and removed tools Issues and PRs related to the tools directory. labels Jun 7, 2016
@addaleax addaleax added the tools Issues and PRs related to the tools directory. label Jun 8, 2016
addaleax added a commit to addaleax/node that referenced this issue Jul 6, 2016
Skip the doctool tests when js-yaml, which is currently `require()`d
from the eslint source tree, is missing. This can happen, for example,
because eslint is not included in the release source tarballs.

Fixes: nodejs#7201
Ref: nodejs#6495
PR-URL: nodejs#7218
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
evanlucas pushed a commit that referenced this issue Jul 12, 2016
Skip the doctool tests when js-yaml, which is currently `require()`d
from the eslint source tree, is missing. This can happen, for example,
because eslint is not included in the release source tarballs.

Fixes: #7201
Ref: #6495
PR-URL: #7218
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
addaleax added a commit to addaleax/node that referenced this issue Jul 12, 2016
Skip the doctool tests when js-yaml, which is currently `require()`d
from the eslint source tree, is missing. This can happen, for example,
because eslint is not included in the release source tarballs.

Fixes: nodejs#7201
Ref: nodejs#6495
PR-URL: nodejs#7218
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
MylesBorins pushed a commit that referenced this issue Jul 12, 2016
Skip the doctool tests when js-yaml, which is currently `require()`d
from the eslint source tree, is missing. This can happen, for example,
because eslint is not included in the release source tarballs.

Fixes: #7201
Ref: #6495
PR-URL: #7218
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
MylesBorins pushed a commit that referenced this issue Jul 12, 2016
Skip the doctool tests when js-yaml, which is currently `require()`d
from the eslint source tree, is missing. This can happen, for example,
because eslint is not included in the release source tarballs.

Fixes: #7201
Ref: #6495
PR-URL: #7218
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
MylesBorins pushed a commit that referenced this issue Jul 14, 2016
Skip the doctool tests when js-yaml, which is currently `require()`d
from the eslint source tree, is missing. This can happen, for example,
because eslint is not included in the release source tarballs.

Fixes: #7201
Ref: #6495
PR-URL: #7218
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
MylesBorins pushed a commit that referenced this issue Jul 14, 2016
Skip the doctool tests when js-yaml, which is currently `require()`d
from the eslint source tree, is missing. This can happen, for example,
because eslint is not included in the release source tarballs.

Fixes: #7201
Ref: #6495
PR-URL: #7218
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
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. test Issues and PRs related to the tests. tools Issues and PRs related to the tools directory.
Projects
None yet
Development

No branches or pull requests

4 participants