Opened 2 years ago

Closed 2 years ago

#5857 closed defect (fixed)

ensure_dependencies.py does not resolve Node.js dependencies if a previous installation failed

Reported by: tlucas Assignee:
Priority: P3 Milestone:
Module: Automation Keywords:
Cc: sebastian, kzar Blocked By:
Blocking: Platform: Unknown / Cross platform
Ready: yes Confidential: no
Tester: Unknown Verified working: no
Review URL(s):

https://codereview.adblockplus.org/29573912

Description (last modified by tlucas)

How to reproduce

  1. Make sure to have an environment where nodejs / npm is not installed
  2. Clone the adblockpluschrome repository hg clone https://hg.adblockplus.org/adblockpluschrome
  3. Go into the repository's root folder and resolve any needed dependencies python ensure_dependencies.py
  4. Install Node.js / npm
  5. Try to resolve the depencies again: python ensure_dependencies.py

Observed behaviour

  • At step 3, an Error is raised:
    ERROR: Failed to install Node.js dependencies for /home/foo/adblockpluschrome/buildtools, please ensure Node.js is installed.
    
  • At step 5, nothing happens

Expected behaviour

If npm install previously failed, ensure_dependencies.py should call it again regardless of dependency changes.

Hints for testers

In order to let npm install fail, you can edit the buildtools' package.json to contain a package which does not exist:

{
  "name": "buildtools",
  "repository": "https://hg.adblockplus.org/buildtools",
  "license": "MPL-2.0",
  "dependencies": {
    "foodoesntexist": "1.3"
  },
  "scripts": {
    "jsdoc": "jsdoc"
  }
}

The behavior on failure can then be verified, as well as the desired behavior on retrial (if you undo the changes to package.json)

Change History (5)

comment:1 Changed 2 years ago by sebastian

  • Description modified (diff)
  • Priority changed from Unknown to P3
  • Ready set

comment:2 Changed 2 years ago by tlucas

  • Description modified (diff)

comment:3 Changed 2 years ago by tlucas

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

comment:4 Changed 2 years ago by abpbot

A commit referencing this issue has landed:
Issue 5857 - Retry npm install on failure

comment:5 Changed 2 years ago by tlucas

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