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.

Sign up for GitHub

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

Jump to bottom

test/sequential/test-heapdump.js lines 27 through 39 is bound to fail #41643

Closed
mawaregetsuka opened this issue Jan 22, 2022 · 3 comments · Fixed by #41772
Closed

test/sequential/test-heapdump.js lines 27 through 39 is bound to fail #41643

mawaregetsuka opened this issue Jan 22, 2022 · 3 comments · Fixed by #41772
Labels
test Issues and PRs related to the tests.

Comments

@mawaregetsuka
Copy link
Contributor

mawaregetsuka commented Jan 22, 2022 •
edited by RaisinTen
Loading

Version

v18.0.0-pre

Platform

No response

Subsystem

No response

What steps will reproduce the bug?

Run tests as root

How often does it reproduce? Is there a required condition?

No response

What is the expected behavior?

No response

What do you see instead?

Path: sequential/test-heapdump
node:assert:123
  throw new AssertionError(obj);
  ^

AssertionError [ERR_ASSERTION]: Missing expected exception.
    at Object.<anonymous> (/root/Documents/node/test/sequential/test-heapdump.js:30:10)
    at Module._compile (node:internal/modules/cjs/loader:1097:14)
    at Object.Module._extensions..js (node:internal/modules/cjs/loader:1149:10)
    at Module.load (node:internal/modules/cjs/loader:975:32)
    at Function.Module._load (node:internal/modules/cjs/loader:822:12)
    at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:81:12)
    at node:internal/main/run_main_module:17:47 {
  generatedMessage: false,
  code: 'ERR_ASSERTION',
  actual: undefined,
  expected: [Function (anonymous)],
  operator: 'throws'
}

Additional information

This test is introduced in the commit 74b9baa
I think the test didn't work because root ignored file permissions
@Mesteery Mesteery added the test Issues and PRs related to the tests. label Jan 22, 2022
@sapics
Copy link
Contributor

sapics commented Jan 30, 2022 •
edited
Loading

I had a same error with current master branch by root in Ubuntu 20.04 + gcc-11 + ./configure --experimental-enable-pointer-compression --enable-lto + -march=native

RaisinTen added a commit to RaisinTen/node that referenced this issue Jan 30, 2022
@RaisinTen
test: fix failure in test/sequential/test-heapdump.js
c6e4596 
The test was failing when it was being run with superuser privileges,
so this changes the test from attempting to write to a read-only file to
attempting to write to a file with the same name as that of an existing
directory, as that is a more reliable way of making
v8.writeHeapSnapshot() throw even when run with sudo.

Fixes: nodejs#41643
Signed-off-by: Darshan Sen <raisinten@gmail.com>
@RaisinTen RaisinTen mentioned this issue Jan 30, 2022
Merged
@RaisinTen
Copy link
Contributor

PR: #41772

@sapics
Copy link
Contributor

sapics commented Jan 30, 2022 •
edited
Loading

Thank you very much for very quick fix and making PR!

I have confirmed that there are NO ERRORs in master branch + chrry-pick c6e4596 (#41772) when I run make -j60 test as root

nodejs-github-bot pushed a commit that referenced this issue Mar 14, 2022
@RaisinTen
test: fix failure in test/sequential/test-heapdump.js
82181bb 
The test was failing when it was being run with superuser privileges,
so this changes the test from attempting to write to a read-only file to
attempting to write to a file with the same name as that of an existing
directory, as that is a more reliable way of making
v8.writeHeapSnapshot() throw even when run with sudo.

Fixes: #41643
Signed-off-by: Darshan Sen <raisinten@gmail.com>

PR-URL: #41772
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
xtx1130 pushed a commit to xtx1130/node that referenced this issue Apr 25, 2022
@RaisinTen @xtx1130
test: fix failure in test/sequential/test-heapdump.js
01cc7ff 
The test was failing when it was being run with superuser privileges,
so this changes the test from attempting to write to a read-only file to
attempting to write to a file with the same name as that of an existing
directory, as that is a more reliable way of making
v8.writeHeapSnapshot() throw even when run with sudo.

Fixes: nodejs#41643
Signed-off-by: Darshan Sen <raisinten@gmail.com>

PR-URL: nodejs#41772
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
test Issues and PRs related to the tests.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

test: fix failure in test/sequential/test-heapdump.js RaisinTen/node
4 participants
@sapics @mawaregetsuka @RaisinTen @Mesteery