Closed Bug 1737419 Opened 7 months ago Closed 7 months ago

Some newtab npm test don't work on windows due to shell difference

Categories

(Firefox :: New Tab Page, defect)

defect

Tracking

()

RESOLVED FIXED
95 Branch
Tracking Status
firefox95 --- fixed

People

(Reporter: arai, Assigned: arai)

References

Details

Attachments

(1 file)

https://firefox-source-docs.mozilla.org/browser/components/newtab/docs/index.html#running-tests

Running ./mach npm test --prefix=browser/components/newtab fails on windows, for the following scripts:

https://searchfox.org/mozilla-central/rev/5e7976773895d72e47a76a9c912fbf3d0171e610/browser/components/newtab/package.json#103-106

  "scripts": {
...
    "lint:codespell": "(cd $npm_package_config_mc_root && ./mach lint -l codespell $npm_package_config_newtab_path)",
    "lint:eslint": "(cd $npm_package_config_mc_root && ./mach lint -l eslint $npm_package_config_newtab_path)",
    "lint:l10n": "(cd $npm_package_config_mc_root && ./mach lint -l l10n --warnings browser/locales/en-US/browser/newtab)",
    "lint:license": "(cd $npm_package_config_mc_root && ./mach lint -l license $npm_package_config_newtab_path)",

There are two issues:

(1) Variable substitution

npm test uses Windows command prompt style command invocation,
that uses %foo% instead of $foo for variable substitution.

So it tries to cd to literally $npm_package_config_mc_root directory and fails.

(2) Command invocation

Even if the variable substitution is fixed by replacing them with raw value,
it still fails because / is treated as delimiter between command and parameter, instead of directory separator,
and it tries to execute . as command.

Then given that it doesn't use msys shell anyway, I think mach command doesn't work there,
and we need to at least mention that npm test doesn't work on windows, in the document.

See Also: → 1698208
See Also: → 1694772
Pushed by arai_a@mac.com:
https://hg.mozilla.org/integration/autoland/rev/91af1d089234
Add note about mach npm test for newtab on Windows. r=Mardak
Status: ASSIGNED → RESOLVED
Closed: 7 months ago
Resolution: --- → FIXED
Target Milestone: --- → 95 Branch
You need to log in before you can comment on or make changes to this bug.