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

✅ All tests that run the CLI should set the COREPACK_HOME environment variable to use the test-specific Corepack installation directory #233

Merged
merged 27 commits into from
Jul 3, 2023

Conversation

sounisi5011
Copy link
Owner

@sounisi5011 sounisi5011 commented Jul 2, 2023

Somehow the npm config get tag-version-prefix command executed by the CLI failed, and therefore the automated test also failed.
This does not happen in my development environment, so I assume it is caused by the environment in which the GitHub Actions are executed.

This pull request tries to fix this problem.

All tests in test/index.ts except the "CLI should add Git tag with customized tag prefix" test did not set the environment variable COREPACK_HOME when running the CLI.
As a result, when the CLI tries to run the npm config get tag-version-prefix command internally, Corepack downloads npm to the default Corepack installation directory.
This caused a segmentation fault and the npm config get tag-version-prefix command was aborted by the SIGSEGV signal.
To fix this, set the environment variable COREPACK_HOME in all tests within test/index.ts and use the test-specific Corepack installation directory.

We need to know why the `npm config get tag-version-prefix` command failing.
@sounisi5011
Copy link
Owner Author

Errors were thrown within the tryNpmConfigGet() function.

return await execFileAsync('npm', ['config', 'get', key], {
env: {
...process.env,
// In Corepack v0.14 and later, the environment variable "COREPACK_ENABLE_STRICT" can be used.
// This allows npm commands to be used even in projects with non-npm package managers defined.
// see https://github.com/nodejs/corepack/tree/v0.14.0#environment-variables
COREPACK_ENABLE_STRICT: '0',
},
})

But, why is the npm config get tag-version-prefix command terminated by the SIGSEGV signal?

@sounisi5011 sounisi5011 force-pushed the fix-ci/npm-config-command-fails branch from c4ca016 to 205c673 Compare July 2, 2023 10:34
…mand

Check to see if the error is caused by the `npm config get ...` command or if npm is throwing an error.
@sounisi5011 sounisi5011 force-pushed the fix-ci/npm-config-command-fails branch from 205c673 to 81b45bb Compare July 2, 2023 10:39
@sounisi5011
Copy link
Owner Author

The error seems to be thrown by the npm command. Maybe it is due to the use of corepack?

@sounisi5011
Copy link
Owner Author

Retrying did not resolve this problem.

The `task` command may be the cause of the segmentation fault.
…e `tryNpmConfigGet()` function

The `pnpm version` command executes the `npm version` command as is.
Therefore, the configuration to be used should be obtained by the `npm config get ...` command.

`tryNpmConfigGet()` tries the `npm config get ...` command for this purpose.
However, if the `npm config get ...` command fails, we should not force termination there.
We should be allowed to use the `pnpm config get ...` command instead.
@sounisi5011
Copy link
Owner Author

sounisi5011 commented Jul 3, 2023

Tests are not terminated by the SIGSEGV signal anymore. However, this does not fix the cause of this problem.

Try clearing the entire cache of GitHub Actions.

…uted by the `tryNpmConfigGet()` function"

This commit fixes the problem of the `npm config get ...` command being aborted by the `SIGSEGV` signal.
However, there may be other ways to do this.
We are reverting this commit to try them.

This reverts commit 7cee91c.
@sounisi5011
Copy link
Owner Author

@sounisi5011 sounisi5011 force-pushed the fix-ci/npm-config-command-fails branch from a19e0f1 to f133d91 Compare July 3, 2023 04:54
@sounisi5011 sounisi5011 force-pushed the fix-ci/npm-config-command-fails branch from f133d91 to 14501e5 Compare July 3, 2023 05:07
@sounisi5011
Copy link
Owner Author

PID 2195 received SIGSEGV for address: 0x10
/home/runner/work/package-version-git-tag/package-version-git-tag/node_modules/.pnpm/segfault-handler@1.3.0/node_modules/segfault-handler/build/Release/segfault-handler.node(+0x372d)[0x7f9ab85df72d]
/lib/x86_64-linux-gnu/libc.so.6(+0x42520)[0x7f9abc72c520]
npm[0xa8a3c4]
npm(_ZN2v88internal7Isolate38RunHostImportModuleDynamicallyCallbackENS0_6HandleINS0_6ScriptEEENS2_INS0_6ObjectEEENS0_11MaybeHandleIS5_EE+0x101)[0xebe9d1]
npm(_ZN2v88internal25Runtime_DynamicImportCallEiPmPNS0_7IsolateE+0x9b)[0x12d4f8b]
npm[0x16fb8f4]
PID 6620 received SIGSEGV for address: 0x14d93adf
SymInit: Symbol-SearchPath: '.;D:\a\package-version-git-tag\package-version-git-tag\test\.temp\cli-should-add-git-tag-normal;C:\hostedtoolcache\windows\node\18.16.1\x64;C:\Windows;C:\Windows\system32;SRV*C:\websymbols*http://msdl.microsoft.com/download/symbols;', symOptions: 530, UserName: 'runneradmin'
OS-Version: 10.0.20348 () 0x190-0x3
D:\a\package-version-git-tag\package-version-git-tag\node_modules\.pnpm\segfault-handler@1.3.0\node_modules\segfault-handler\src\StackWalker.cpp (941): StackWalker::ShowCallstack
D:\a\package-version-git-tag\package-version-git-tag\node_modules\.pnpm\segfault-handler@1.3.0\node_modules\segfault-handler\src\segfault-handler.cpp (242): segfault_handler
00007FFFDE15B642 (ntdll): (filename not available): LdrGetDllHandleByName
00007FFFDE1114D2 (ntdll): (filename not available): RtlVirtualUnwind2
00007FFFDE18343E (ntdll): (filename not available): KiUserExceptionDispatcher
00007FF714D93ADF (node): (filename not available): cppgc::internal::LargePage::ObjectHeader
00007FF7156D40D7 (node): (filename not available): v8::internal::Isolate::RunHostImportModuleDynamicallyCallback
00007FF715377194 (node): (filename not available): v8::internal::Runtime::SetObjectProperty
00007FF715897E7C (node): (filename not available): v8::internal::SetupIsolateDelegate::SetupHeap
00007FF7159357D5 (node): (filename not available): v8::internal::SetupIsolateDelegate::SetupHeap
00007FF71581B694 (node): (filename not available): v8::internal::SetupIsolateDelegate::SetupHeap
00007FF71584F7C3 (node): (filename not available): v8::internal::SetupIsolateDelegate::SetupHeap
00007FF7158E85B5 (node): (filename not available): v8::internal::SetupIsolateDelegate::SetupHeap
00007FF715840ECC (node): (filename not available): v8::internal::SetupIsolateDelegate::SetupHeap
00007FF715819B9B (node): (filename not available): v8::internal::SetupIsolateDelegate::SetupHeap
00007FF7156E415A (node): (filename not available): v8::internal::Execution::CallWasm
00007FF7156E427B (node): (filename not available): v8::internal::Execution::CallWasm
00007FF7156E502A (node): (filename not available): v8::internal::Execution::TryCallScript
00007FF7156BDB3A (node): (filename not available): v8::internal::MicrotaskQueue::RunMicrotasks
00007FF7156BD8DA (node): (filename not available): v8::internal::MicrotaskQueue::PerformCheckpointInternal
00007FF714DFBC0E (node): (filename not available): node::CallbackScope::~CallbackScope
00007FF714DFBADE (node): (filename not available): node::CallbackScope::~CallbackScope
00007FF714D4C0B8 (node): (filename not available): DSA_meth_get_flags
00007FF714D48D41 (node): (filename not available): DSA_meth_get_flags
00007FF714D3E648 (node): (filename not available): v8::base::CPU::has_fpu
00007FF714D4E415 (node): (filename not available): DSA_meth_get_flags
00007FF714E2CDE7 (node): (filename not available): uv_timer_stop
00007FF714E2925B (node): (filename not available): uv_update_time
00007FF714E28DA2 (node): (filename not available): uv_run
00007FF714DFB495 (node): (filename not available): node::SpinEventLoop
00007FF714D0A5B8 (node): (filename not available): EVP_CIPHER_CTX_get_cipher_data
00007FF714D8F3A2 (node): (filename not available): node::InitializeOncePerProcess
00007FF714D90BD3 (node): (filename not available): node::Start
00007FF714B97ECC (node): (filename not available): AES_cbc_encrypt
00007FF715DFDE58 (node): (filename not available): inflateValidate
00007FFFDC544DE0 (KERNEL32): (filename not available): BaseThreadInitThunk
00007FFFDE15E44B (ntdll): (filename not available): RtlUserThreadStart

So what does this output tell me?

@sounisi5011
Copy link
Owner Author

sounisi5011 commented Jul 3, 2023

I did it! Problem fixed!

We installed the latest version of npm using Corepack for our "CLI should add Git tag with customized tag prefix" test. However, it seems that the other tests within test/index.ts were using npm that existed in the test environment, not the npm we installed for testing.
In the directory where Corepack installs the package manager, there are two:

  1. Somewhere in the home directory. By default this is $HOME/.cache/node/corepack. If we use Corepack in our development environment, or if we enable Corepack with GitHub Actions, the required package managers should be installed here.
  2. The test/.temp/.corepack directory. To avoid interfering with the above, test code will use this.

The CLI will then attempt to run the npm config get ... command when run by pnpm. The test code is invoked with pnpm installed in 1, so npm installed in 1 is used (or downloaded if not already installed). This is likely the cause of the segmentation fault.


Edit: This assumption was correct. I tried the test after deleting the $HOME/.cache/node/corepack directory, and the segmentation fault also occurred in my development environment. However, this only happens once, and after the second time, the npm config get tag-version-prefix command terminates with exit code 1. Due to lack of reproducibility, I am unable to investigate the details.

…use the `COREPACK_HOME` environment variable

All CLI and package manager commands executed within `test/index.ts` should use the `COREPACK_HOME` environment variable.
To keep this in mind, we modified our test code as follows:

+ Add the `execDefaultEnv` option to the `initGit()` function.
+ Add the `COREPACK_HOME` environment variable to the `execDefaultEnv` option of the `initGit()` function called in `test/index.ts`.
@sounisi5011
Copy link
Owner Author

Why does the test fail on Windows?

…on should use the `COREPACK_HOME` environment variable"

This reverts commit f479280.
@sounisi5011
Copy link
Owner Author

Hmm, interesting.

…()` function should use the `COREPACK_HOME` environment variable""

This reverts commit 5f0bc0d.
…lled for testing

The error is as follows:

```
TypeError [ERR_INVALID_ARG_TYPE]: The "path" argument must be of type string. Received undefined
    at new NodeError (node:internal/errors:399:5)
    at validateString (node:internal/validators:163:11)
    at Object.resolve (node:path:167:9)
    at ../node_modules/.pnpm/@pnpm+npm-conf@2.0.4/node_modules/@pnpm/npm-conf/lib/defaults.js (D:\a\package-version-git-tag\package-version-git-tag\test\.temp\.corepack\pnpm\7.30.5\dist\pnpm.cjs:15236:23)
    at __require (D:\a\package-version-git-tag\package-version-git-tag\test\.temp\.corepack\pnpm\7.30.5\dist\pnpm.cjs:12:50)
    at ../node_modules/.pnpm/@pnpm+npm-conf@2.0.4/node_modules/@pnpm/npm-conf/index.js (D:\a\package-version-git-tag\package-version-git-tag\test\.temp\.corepack\pnpm\7.30.5\dist\pnpm.cjs:15385:21)
    at __require (D:\a\package-version-git-tag\package-version-git-tag\test\.temp\.corepack\pnpm\7.30.5\dist\pnpm.cjs:12:50)
    at ../config/config/lib/index.js (D:\a\package-version-git-tag\package-version-git-tag\test\.temp\.corepack\pnpm\7.30.5\dist\pnpm.cjs:26161:39)
    at __require (D:\a\package-version-git-tag\package-version-git-tag\test\.temp\.corepack\pnpm\7.30.5\dist\pnpm.cjs:12:50)
    at ../cli/cli-utils/lib/getConfig.js (D:\a\package-version-git-tag\package-version-git-tag\test\.temp\.corepack\pnpm\7.30.5\dist\pnpm.cjs:46134:20)
```

Probably some kind of bug caused by Windows file paths.
However, the pnpm versions are the same.
The only difference is the directory where pnpm is installed (`D:\a\package-version-git-tag\package-version-git-tag\test\.temp\.corepack` is defined in the `COREPACK_HOME` environment variable).
So I am not sure what kind of bug it is.
@sounisi5011
Copy link
Owner Author

sounisi5011 commented Jul 3, 2023

I have figured out why the test only fails on Windows. I seem to have accidentally deleted work done in 2613c17.

I inadvertently caused this problem to reappear in the work on commit f479280.
This is a problem that was previously fixed in commit 2613c17.

Is it back again? I am probably an idiot.
So I added a comment.
@sounisi5011 sounisi5011 force-pushed the fix-ci/npm-config-command-fails branch from 56e6585 to 333c03c Compare July 3, 2023 14:57
@sounisi5011 sounisi5011 changed the title 💚 Fix Unit Test errors when running on GitHub Workflow ✅ All tests that run the CLI should set the COREPACK_HOME environment variable to use the test-specific Corepack installation directory Jul 3, 2023
@sounisi5011 sounisi5011 marked this pull request as ready for review July 3, 2023 15:26
github-actions[bot]
github-actions bot previously approved these changes Jul 3, 2023
@codeclimate
Copy link

codeclimate bot commented Jul 3, 2023

Code Climate has analyzed commit c11a224 and detected 0 issues on this pull request.

View more on Code Climate.

@sounisi5011 sounisi5011 merged commit 5848d67 into master Jul 3, 2023
20 checks passed
@sounisi5011 sounisi5011 deleted the fix-ci/npm-config-command-fails branch July 3, 2023 15:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant