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

Make CLI tests passing on Windows #1359

Closed
Krinkle opened this issue Dec 30, 2018 · 4 comments · Fixed by #1674
Closed

Make CLI tests passing on Windows #1359

Krinkle opened this issue Dec 30, 2018 · 4 comments · Fixed by #1674

Comments

@Krinkle
Copy link
Member

Krinkle commented Dec 30, 2018

As part of #1347 I tried to consolidate the main tests and CLI tests under plainly npm test, but ran into the problem that the CLI tests don't currently pass on Windows.

For a while I tried to make them work, thinking it was an issue with Travis (because AppVeyor is green). Only to realise they would also fail on AppVeyor. It's just that we don't run them there.

So.. filing this as a separate issue to resolve later.

I ran into two issues:

  1. Running an executable from a sub directory doesn't work.

The bin/qunit test/cli/*.js part of grunt && bin/qunit test/cli/*.js isn't working on Windows. It fails as follows:

'bin' is not recognized as an internal or external command, operable program or batch file.

This is presumably due to the use of a forward-slash. Some sources on the internet suggested that wrapping them in quotes and/or using ./ would make it work, but that didn't seem to do the trick.

What did work is changing the invocation to node bin/qunit test/cli/*.js.

  1. The child processes created by the CLI tests fail.
  expected: undefined
  stack: Error: Command failed: C:\windows\system32\cmd.exe /s /c "../../../bin/qunit --reporter npm-reporter"
'..' is not recognized as an internal or external command,
operable program or batch file.
    at Promise.all.then.arr (C:\projects\qunit\node_modules\execa\index.js:236:11)
    at process._tickCallback (internal/process/next_tick.js:68:7)
  ...

This is presumably the same issue in essence. ../../../ doesn't work in that way on Windows. This might be an easy fix given that we invoke it from within Node.js, so we could easily vary the logic by process.platform and/or utilise something like path.join() to get the slashes right.

Krinkle added a commit that referenced this issue Dec 30, 2018
Update grunt-contrib-qunit to 3.1.0, because grunt-contrib-qunit 3.0.1
uses a pinned (older) version of puppeteer, which uses an older version
of Chrome, which is incompatible with Windows Server 2016.

For Windows limit the script to `test:main`. This is the same as before
on AppVeyor. The CLI tests aren't passing on Windows currently.
See #1359.
Krinkle added a commit that referenced this issue Dec 30, 2018
Update grunt-contrib-qunit to 3.1.0, because grunt-contrib-qunit 3.0.1
uses a pinned (older) version of puppeteer, which uses an older version
of Chrome, which is incompatible with Windows Server 2016.

For Windows limit the script to `test:main`. This is the same as before
on AppVeyor. The CLI tests aren't passing on Windows currently.
See #1359.
Krinkle added a commit that referenced this issue Dec 30, 2018
Update grunt-contrib-qunit to 3.1.0, because grunt-contrib-qunit 3.0.1
uses a pinned (older) version of puppeteer, which uses an older version
of Chrome, which is incompatible with Windows Server 2016.

For Windows limit the script to `test:main`. This is the same as before
on AppVeyor. The CLI tests aren't passing on Windows currently.
See #1359.
Krinkle added a commit that referenced this issue Dec 30, 2018
Update grunt-contrib-qunit to 3.1.0, because grunt-contrib-qunit 3.0.1
uses a pinned (older) version of puppeteer. The older puppteer used an
older version of Chrome, which is incompatible with Windows Server 2016,
which is what Travis CI offers. AppVeyor was using Windows Server 2012,
whih was not affected by that Chrome bug.

Ref gruntjs/grunt-contrib-qunit#154.

For Windows, the build script is limited to `test:main`. This is the
same as it was before with AppVeyor, because the CLI tests are not passing
on Windows currently. Ref #1359.

Fixes #1347.
Krinkle added a commit that referenced this issue Dec 30, 2018
Update grunt-contrib-qunit to 3.1.0, because grunt-contrib-qunit 3.0.1
uses a pinned (older) version of puppeteer. The older puppteer used an
older version of Chrome, which is incompatible with Windows Server 2016,
which is what Travis CI offers. AppVeyor was using Windows Server 2012,
whih was not affected by that Chrome bug.

Ref gruntjs/grunt-contrib-qunit#154.

For Windows, the build script is limited to `test:main`. This is the
same as it was before with AppVeyor, because the CLI tests are not passing
on Windows currently. Ref #1359.

Fixes #1347.
Krinkle added a commit that referenced this issue Dec 30, 2018
Update grunt-contrib-qunit to 3.1.0, because grunt-contrib-qunit 3.0.1
uses a pinned (older) version of puppeteer. The older puppteer used an
older version of Chrome, which is incompatible with Windows Server 2016,
which is what Travis CI offers. AppVeyor was using Windows Server 2012,
whih was not affected by that Chrome bug.

Ref gruntjs/grunt-contrib-qunit#154.

For Windows, the build script is limited to `test:main`. This is the
same as it was before with AppVeyor, because the CLI tests are not passing
on Windows currently. Ref #1359.

Fixes #1347.
Krinkle added a commit that referenced this issue Dec 30, 2018
Update grunt-contrib-qunit to 3.1.0, because grunt-contrib-qunit 3.0.1
uses a pinned (older) version of puppeteer. The older puppteer used an
older version of Chrome, which is incompatible with Windows Server 2016,
which is what Travis CI offers. AppVeyor was using Windows Server 2012,
whih was not affected by that Chrome bug.

Ref gruntjs/grunt-contrib-qunit#154.

For Windows, the build script is limited to `test:main`. This is the
same as it was before with AppVeyor, because the CLI tests are not passing
on Windows currently. Ref #1359.

Fixes #1347.
Krinkle added a commit that referenced this issue Dec 30, 2018
Update grunt-contrib-qunit to 3.1.0, because grunt-contrib-qunit 3.0.1
uses a pinned (older) version of puppeteer. The older puppteer used an
older version of Chrome, which is incompatible with Windows Server 2016,
which is what Travis CI offers. AppVeyor was using Windows Server 2012,
whih was not affected by that Chrome bug.

Ref gruntjs/grunt-contrib-qunit#154.

For Windows, the build script is limited to `test:main`. This is the
same as it was before with AppVeyor, because the CLI tests are not passing
on Windows currently. Ref #1359.

Rename bin/qunit to bin/qunit.js, because otherwise ESLint fails:

> Expected linebreaks to be 'LF' but found 'CRLF'  linebreak-style

In theory, it should be possible to add an entry to `.gitattributes`
that will apply to bin/qunit what we apply to *.js, but I wasn't
able to find a way.

Fixes #1347.
Krinkle added a commit that referenced this issue Dec 30, 2018
Update grunt-contrib-qunit to 3.1.0, because grunt-contrib-qunit 3.0.1
uses a pinned (older) version of puppeteer. The older puppteer used an
older version of Chrome, which is incompatible with Windows Server 2016,
which is what Travis CI offers. AppVeyor was using Windows Server 2012,
whih was not affected by that Chrome bug.

Ref gruntjs/grunt-contrib-qunit#154.

For Windows, the build script is limited to `test:main`. This is the
same as it was before with AppVeyor, because the CLI tests are not passing
on Windows currently. Ref #1359.

Rename bin/qunit to bin/qunit.js, because otherwise ESLint fails:

> Expected linebreaks to be 'LF' but found 'CRLF'  linebreak-style

In theory, it should be possible to add an entry to `.gitattributes`
that will apply to bin/qunit what we apply to *.js, but I wasn't
able to find a way.

Fixes #1347.
@trentmwillis
Copy link
Member

Thanks for working on this!

Krinkle added a commit that referenced this issue Jan 2, 2019
Update grunt-contrib-qunit to 3.1.0, because grunt-contrib-qunit 3.0.1
uses a pinned (older) version of puppeteer. The older puppteer used an
older version of Chrome, which is incompatible with Windows Server 2016,
which is what Travis CI offers. AppVeyor was using Windows Server 2012,
whih was not affected by that Chrome bug.

Ref gruntjs/grunt-contrib-qunit#154.

For Windows, the build script is limited to `test:main`. This is the
same as it was before with AppVeyor, because the CLI tests are not passing
on Windows currently. Ref #1359.

Rename bin/qunit to bin/qunit.js, because otherwise ESLint fails:

> Expected linebreaks to be 'LF' but found 'CRLF'  linebreak-style

In theory, it should be possible to add an entry to `.gitattributes`
that will apply to bin/qunit what we apply to *.js, but I wasn't
able to find a way.

Fixes #1347.
@XhmikosR
Copy link
Contributor

For what is worth, the problem is that Windows doesn't support shebangs. Which means bin/qunit test/cli/*.js is wrong. It should be node bin/qunit test/cli/*.js

@Krinkle
Copy link
Member Author

Krinkle commented Jun 27, 2020

Thanks, that makes a lot of sense!

@Krinkle
Copy link
Member Author

Krinkle commented Jul 30, 2020

I've made these two changes and it's getting somewhere, although still some test failures.

Draft commit: a04fb8e

[WIP] Re-enable CLI test on Windows

  • For subprocesses in the CLI tests, use process.execPath to
    use the same node program for subprocesses regardless of its
    name, extension or location (e.g. 'nodejs' or 'node.exe' etc.)

  • Use 'node bin/qunit.js' instead of 'bin/qunit.js' because
    the hashbang (#!/usr/bin/env) atop the executable is not
    supported on Windows.

https://travis-ci.com/github/qunitjs/qunit/builds/177733756

The first one is:

ok 1 find-reporter > correctly finds js-reporters reporter by name
ok 2 find-reporter > correctly finds npm package reporter by name
ok 3 CLI Reporter > runs tests using the specified reporter
ok 4 CLI Reporter > exits early and lists available reporters if reporter is not found
ok 5 CLI Reporter > exits early and lists available reporters if reporter option is used with no value
ok 6 CLI Main > defaults to running tests in 'test' directory
ok 7 CLI Main > errors if no test files are found to run
not ok 8 CLI Main > accepts globs for test files to run
  ---

  message: "Promise rejected during "accepts globs for test files to run": Command failed: C:\Windows\system32\cmd.exe /s /c "C:\ProgramData\nvs\node\14.7.0\x64\node.exe ../../../bin/qunit.js 'glob/**/*-test.js'"
No files were found matching: 'glob/**/*-test.js'
"
  severity: failed
  stack: Error: Command failed: C:\Windows\system32\cmd.exe /s /c "C:\ProgramData\nvs\node\14.7.0\x64\node.exe ../../../bin/qunit.js 'glob/**/*-test.js'"

No files were found matching: 'glob/**/*-test.js'

@Krinkle Krinkle pinned this issue Jul 30, 2020
@Krinkle Krinkle unpinned this issue Jun 7, 2021
Krinkle added a commit that referenced this issue Feb 15, 2022
* For subprocesses in the CLI tests, use `process.execPath` to
  use the same node program for subprocesses; regardless of its
  name, extension or location (e.g. '`odejs` or `node.exe` etc.)

* Use `node bin/qunit.js` instead of `bin/qunit.js` in package.json,
  because the hashbang (`#!/usr/bin/env`) atop the executable is not
  supported on Windows.

* Avoid a problem with quoted arguments in the test runner's
  creation of sub processes. Instead of going through a shell
  (and thus cross-platform differences), pass arguments explicitly
  using Node's built-in mechanism for child processes.
  I've removed the `exaca` dependency in the process and just the
  Node.js API directly.

* Improve output normalization to accomodate Windows' backslashes.

* Skip a handful of tests that don't yet pass, let's make iterative
  progress going-forward to avoid further regressions.

Closes #1359.
Krinkle added a commit that referenced this issue Feb 15, 2022
* For subprocesses in the CLI tests, use `process.execPath` to
  use the same node program for subprocesses; regardless of its
  name, extension or location (e.g. '`odejs` or `node.exe` etc.)

* Use `node bin/qunit.js` instead of `bin/qunit.js` in package.json,
  because the hashbang (`#!/usr/bin/env`) atop the executable is not
  supported on Windows.

* Avoid a problem with quoted arguments in the test runner's
  creation of sub processes. Instead of going through a shell
  (and thus cross-platform differences), pass arguments explicitly
  using Node's built-in mechanism for child processes.
  I've removed the `exaca` dependency in the process and just the
  Node.js API directly.

* Improve output normalization to accomodate Windows' backslashes.

* Skip a handful of tests that don't yet pass, let's make iterative
  progress going-forward to avoid further regressions.

Ref #1667.
Closes #1359.
Krinkle added a commit that referenced this issue Feb 15, 2022
* For subprocesses in the CLI tests, use `process.execPath` to
  use the same node program for subprocesses; regardless of its
  name, extension or location (e.g. '`odejs` or `node.exe` etc.)

* Use `node bin/qunit.js` instead of `bin/qunit.js` in package.json,
  because the hashbang (`#!/usr/bin/env`) atop the executable is not
  supported on Windows.

* Avoid a problem with quoted arguments in the test runner's
  creation of sub processes. Instead of going through a shell
  (and thus cross-platform differences), pass arguments explicitly
  using Node's built-in mechanism for child processes.
  I've removed the `exaca` dependency in the process and just the
  Node.js API directly.

* Improve output normalization to accomodate Windows' backslashes.

* Skip a handful of tests that don't yet pass, let's make iterative
  progress going-forward to avoid further regressions.

Ref #1667.
Closes #1359.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging a pull request may close this issue.

3 participants