node icon indicating copy to clipboard operation
node copied to clipboard

meta: replace build-windows for test-windows

Open H4ad opened this issue 1 year ago • 7 comments

Closes #50489

As the reference to build and test, I used the following files:

  • https://github.com/nodejs/build/blob/main/jenkins/scripts/windows/ci-run.cmd
  • https://github.com/nodejs/build/blob/main/jenkins/scripts/windows/node-test-binary-windows-js-suites.cmd
  • https://github.com/nodejs/build/blob/main/jenkins/scripts/windows/node-test-binary-windows-native-suites.cmd
  • https://ci.nodejs.org/job/node-test-commit-windows-fanned/configure
  • https://ci.nodejs.org/job/node-test-binary-windows-js-suites/configure
  • https://ci.nodejs.org/job/node-test-binary-windows-native-suites/nodes=win11-vs2022-arm64-COMPILED_BY-vs2022-arm64/20185/consoleFull
  • https://ci.nodejs.org/job/node-test-binary-windows-js-suites/RUN_SUBSET=0,nodes=win2016-COMPILED_BY-vs2022-x86/24289/console

H4ad avatar Nov 02 '23 01:11 H4ad

Review requested:

  • [ ] @nodejs/actions

nodejs-github-bot avatar Nov 02 '23 01:11 nodejs-github-bot

 failed 10 out of 10
not ok 394 parallel/test-child-process-exec-any-shells-windows
  ---
  duration_ms: 662.63700
  severity: fail
  exitcode: 1
  stack: |-
    node:assert:991
        throw newErr;
        ^
    
    AssertionError [ERR_ASSERTION]: ifError got unwanted exception: Command failed: echo foo bar
    
        at D:\a\node\node\test\common\index.js:439:12
        at D:\a\node\node\test\common\index.js:476:15
        at ChildProcess.exithandler (node:child_process:430:5)
        at ChildProcess.exithandler (node:child_process:422:12)
        at ChildProcess.emit (node:events:519:28)
        at maybeClose (node:internal/child_process:1105:16)
        at ChildProcess._handle.onexit (node:internal/child_process:305:5) {
      generatedMessage: false,
      code: 'ERR_ASSERTION',
      actual: Error: Command failed: echo foo bar
      
          at ChildProcess.exithandler (node:child_process:422:12)
          at ChildProcess.emit (node:events:519:28)
          at maybeClose (node:internal/child_process:1105:16)
          at ChildProcess._handle.onexit (node:internal/child_process:305:5) {
        code: 1,
        killed: false,
        signal: null,
        cmd: 'echo foo bar'
      },
      expected: null,
      operator: 'ifError'
    }
    
    Node.js v22.0.0-pre
  ...

targos avatar Nov 02 '23 09:11 targos

@targos This test is probably failing because of this error:

C:\Users\vinic\Desktop\Projects\node>"C:\Program Files\Git\usr\bin\bash.exe" ls
/usr/bin/ls: /usr/bin/ls: cannot execute binary file

But I didn't find a way to get that error message, at least when I print the error returned by exec, is just:

Error: Command failed: echo foo bar

    at ChildProcess.exithandler (node:child_process:422:12)
    at ChildProcess.emit (node:events:519:28)
    at maybeClose (node:internal/child_process:1105:16)
    at ChildProcess._handle.onexit (node:internal/child_process:305:5) {
  code: 1,
  killed: false,
  signal: null,
  cmd: 'echo foo bar'
}

If I try to inspect the message:

O Subsistema do Windows para Linux n�o tem distribui��es instaladas.
As distribui��es podem ser instaladas visitando a Microsoft Store:
https://aka.ms/wslstore

Is just a generic error in Portuguese about WSL, so I don't think searching and testing for bash is useful, maybe we can remove that part of the test.


By the way, the CI is green because I stopped calling the test-child-process-exec-any-shells-windows.js, from what I look on other CIs in Windows, they don't call that test (I put the link on description).

H4ad avatar Nov 03 '23 02:11 H4ad

@targos @richardlau

Is it okay to remove a part of the test?

Based on https://github.com/nodejs/node/pull/50519#issuecomment-1791806986, I don't think that test will run on Github CI, that test doesn't even run on my machine.

Basically, we just need to remove the part that is testing bash on WSL.

H4ad avatar Nov 04 '23 02:11 H4ad

Is it okay to remove a part of the test?

No. We need to install Git bash. It's a requirement for running the tests: https://github.com/nodejs/node/blob/main/BUILDING.md#option-1-manual-install

targos avatar Nov 04 '23 13:11 targos

@targos Bash is already installed: https://github.com/nodejs/node/actions/runs/6751686476/job/18356112178?pr=50519#step:4:50

As I mentioned on https://github.com/nodejs/node/pull/50519#issuecomment-1791806986, I don't think the test will be able to run when Git Bash is installed or when WSL is enabled but not configured.

I suspect the environment we run the tests doesn't have the Git Bash, or at least, doesn't have the first one:

$ where bash <-- running on my machine
C:\Program Files\Git\usr\bin\bash.exe <-- this one that causes issues on the test because of WSL
C:\Windows\System32\bash.exe

H4ad avatar Nov 04 '23 23:11 H4ad

This needs a rebase.

aduh95 avatar May 11 '24 18:05 aduh95

Closing this one since I have no clue on how I should properly fix those issues on Win :/

H4ad avatar Jul 20 '24 00:07 H4ad