Opened 3 years ago

Closed 3 years ago

#4956 closed defect (fixed)

npm test test/browser/elemHideEmulation.js doesn't work

Reported by: hfiguiere Assignee: hfiguiere
Priority: Unknown Milestone:
Module: Core Keywords:
Cc: Blocked By:
Blocking: Platform: Unknown / Cross platform
Ready: yes Confidential: no
Tester: Unknown Verified working: no
Review URL(s):

https://codereview.adblockplus.org/29377951

Description (last modified by hfiguiere)

Environment

Fedora 25.
Configure things to run the Core tests.

How to reproduce

  1. npm test test/browser/elemHideEmulation.js

Observed behaviour

% npm test test/browser/elemHideEmulation.js

> adblockpluscore@ test /home/hub/source/eyeo/adblockpluscore
> node test_runner.js "test/browser/elemHideEmulation.js"

path.js:7
    throw new TypeError('Path must be a string. Received ' + inspect(path));
    ^

TypeError: Path must be a string. Received [Function: cwd]
    at assertPath (path.js:7:11)
    at Object.resolve (path.js:1146:7)
    at browserFiles.map.file (/home/hub/source/eyeo/adblockpluscore/test_runner.js:78:22)
    at Array.map (native)
    at Object.<anonymous> (/home/hub/source/eyeo/adblockpluscore/test_runner.js:73:27)
    at Module._compile (module.js:570:32)
    at Object.Module._extensions..js (module.js:579:10)
    at Module.load (module.js:487:32)
    at tryModuleLoad (module.js:446:12)
    at Function.Module._load (module.js:438:3)
npm ERR! Test failed.  See above for more details

Expected behaviour

Test should run

Cause

test_runner.js:78, process.cwd is used instead of the function call process.cwd()
This is only triggered because a *relative* path is passed to npm test, which mean that path.resolve() build an absolute path using arguments right to left. By default (no arguments) the right argument is an absolute path. Here it is not, so it takes the first argument and it is of type "function".

Will submit patch

Change History (6)

comment:1 Changed 3 years ago by hfiguiere

I should add a note that this only is triggered if you pass a path to *browser* test. (in */browser/*.js)

comment:2 Changed 3 years ago by hfiguiere

  • Review URL(s) modified (diff)
  • Status changed from new to reviewing

comment:3 Changed 3 years ago by fhd

I can reproduce this, will look at the patch.

comment:4 Changed 3 years ago by hfiguiere

  • Description modified (diff)

comment:5 Changed 3 years ago by abpbot

A commit referencing this issue has landed:
Issue 4956 - Fix test_runner.js

comment:6 Changed 3 years ago by hfiguiere

  • Resolution set to fixed
  • Status changed from reviewing to closed
Note: See TracTickets for help on using tickets.