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: move tmpdir functionality to submodule of common #17856

Closed
wants to merge 1 commit into from
Closed

test: move tmpdir functionality to submodule of common #17856

wants to merge 1 commit into from

Conversation

Trott
Copy link
Member

@Trott Trott commented Dec 25, 2017

Move tmpdir functionality to its own module (common/tmpdir).

(Trying to move to focused small modules and reduce unwieldy monoliths. Not sure how this will be received...)

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • documentation is changed or added
  • commit message follows commit guidelines
Affected core subsystem(s)

test

@nodejs-github-bot nodejs-github-bot added addons Issues and PRs related to native addons. async_hooks Issues and PRs related to the async hooks subsystem. benchmark Issues and PRs related to the benchmark subsystem. http Issues or PRs related to the http subsystem. module Issues and PRs related to the module subsystem. test Issues and PRs related to the tests. labels Dec 25, 2017
@Trott
Copy link
Member Author

Trott commented Dec 25, 2017

CI (if CI gets unstuck): https://ci.nodejs.org/job/node-test-pull-request/12291/

@Trott
Copy link
Member Author

Trott commented Dec 27, 2017

@nodejs/testing

@Trott
Copy link
Member Author

Trott commented Dec 27, 2017

CI: https://ci.nodejs.org/job/node-test-pull-request/12322/

gibfahn
gibfahn approved these changes Dec 28, 2017
View reviewed changes
Copy link
Member

@gibfahn gibfahn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@gibfahn
Copy link
Member

gibfahn commented Dec 28, 2017

+1 on this, the tmpdir stuff isn't used enough that it's annoying to have to require it separately, and it's complex enough that having a separate module makes sense (IMHO).

joyeecheung
joyeecheung approved these changes Dec 28, 2017
View reviewed changes
Copy link
Member

@joyeecheung joyeecheung left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with a green CI, also this should be backported ASAP

@Trott
Copy link
Member Author

Trott commented Dec 29, 2017

Rebased, force-pushed, CI: https://ci.nodejs.org/job/node-test-pull-request/12339/

@Trott
Copy link
Member Author

Trott commented Dec 29, 2017

Implausible as it seemed at first, I do think the CI failures here are somehow related. Marking as in progress until that gets resolved.

@Trott Trott added the wip Issues and PRs that are still a work in progress. label Dec 29, 2017
@Trott
Copy link
Member Author

Trott commented Dec 30, 2017

I'm puzzled as to how this change can be causing a crash in test/async-hooks/test-zlib.zlib-binding.deflate.js on some Linux machines, but it sure seems to be. (I re-ran CI on Linux only a third time to be sure. https://ci.nodejs.org/job/node-test-commit-linux/15279/)

Does anyone have an idea?

not ok 57 async-hooks/test-zlib.zlib-binding.deflate
  ---
  duration_ms: 0.199
  severity: fail
  stack: |-
    *** Error in `out/Release/node': corrupted double-linked list: 0x0000000002ada6e0 ***
    ======= Backtrace: =========
    /lib64/libc.so.6(+0x7ab54)[0x7f1bc80f1b54]
    /lib64/libc.so.6(+0x7de72)[0x7f1bc80f4e72]
    /lib64/libc.so.6(__libc_malloc+0x4c)[0x7f1bc80f710c]
    /lib64/libstdc++.so.6(_ZnwmRKSt9nothrow_t+0x1d)[0x7f1bc8bcd24d]
    out/Release/node(_ZN2v88internal7Scanner13LiteralBuffer12ExpandBufferEv+0x42)[0x103c832]
    out/Release/node(_ZN2v88internal7Scanner16ScanTemplateSpanEv+0x718)[0x10456a8]
    out/Release/node(_ZN2v88internal10ParserBaseINS0_6ParserEE20ParseTemplateLiteralEPNS0_10ExpressionEibPb+0x378)[0x100fdc8]
    out/Release/node(_ZN2v88internal10ParserBaseINS0_6ParserEE22ParsePrimaryExpressionEPbS4_+0x10f)[0x1001c6f]
    out/Release/node(_ZN2v88internal10ParserBaseINS0_6ParserEE21ParseMemberExpressionEPbS4_+0x2e)[0xffc87e]
    out/Release/node(_ZN2v88internal10ParserBaseINS0_6ParserEE36ParseMemberWithNewPrefixesExpressionEPbS4_+0x32)[0xffcbd2]
    out/Release/node(_ZN2v88internal10ParserBaseINS0_6ParserEE27ParseLeftHandSideExpressionEPb+0x30)[0xffd180]
    out/Release/node(_ZN2v88internal10ParserBaseINS0_6ParserEE22ParsePostfixExpressionEPb+0x27)[0xffe107]
    out/Release/node(_ZN2v88internal10ParserBaseINS0_6ParserEE20ParseUnaryExpressionEPb+0x3f)[0xffe3af]
    out/Release/node(_ZN2v88internal10ParserBaseINS0_6ParserEE21ParseBinaryExpressionEibPb+0x31)[0xffea81]
    out/Release/node(_ZN2v88internal10ParserBaseINS0_6ParserEE26ParseConditionalExpressionEbPb+0x66)[0x1006d96]
    out/Release/node(_ZN2v88internal10ParserBaseINS0_6ParserEE25ParseAssignmentExpressionEbPb+0x137)[0x1002457]
    out/Release/node(_ZN2v88internal10ParserBaseINS0_6ParserEE14ParseArgumentsEPNS0_7Scanner8LocationEbPbS7_+0xda)[0x100a80a]
    out/Release/node(_ZN2v88internal10ParserBaseINS0_6ParserEE36ParseMemberWithNewPrefixesExpressionEPbS4_+0x3ad)[0xffcf4d]
    out/Release/node(_ZN2v88internal10ParserBaseINS0_6ParserEE27ParseLeftHandSideExpressionEPb+0x30)[0xffd180]
    out/Release/node(_ZN2v88internal10ParserBaseINS0_6ParserEE22ParsePostfixExpressionEPb+0x27)[0xffe107]
    out/Release/node(_ZN2v88internal10ParserBaseINS0_6ParserEE20ParseUnaryExpressionEPb+0x3f)[0xffe3af]
    out/Release/node(_ZN2v88internal10ParserBaseINS0_6ParserEE21ParseBinaryExpressionEibPb+0x31)[0xffea81]
    out/Release/node(_ZN2v88internal10ParserBaseINS0_6ParserEE26ParseConditionalExpressionEbPb+0x66)[0x1006d96]
    out/Release/node(_ZN2v88internal10ParserBaseINS0_6ParserEE25ParseAssignmentExpressionEbPb+0x137)[0x1002457]
    out/Release/node(_ZN2v88internal10ParserBaseINS0_6ParserEE25ParseAssignmentExpressionEbPb+0x762)[0x1002a82]
    out/Release/node(_ZN2v88internal10ParserBaseINS0_6ParserEE27ParseExpressionCoverGrammarEbPb+0xb7)[0x100eb07]
    out/Release/node(_ZN2v88internal10ParserBaseINS0_6ParserEE34ParseExpressionOrLabelledStatementEPNS0_8ZoneListIPKNS0_12AstRawStringEEENS0_30AllowLabelledFunctionStatementEPb+0xba)[0x101692a]
    out/Release/node(_ZN2v88internal10ParserBaseINS0_6ParserEE14ParseStatementEPNS0_8ZoneListIPKNS0_12AstRawStringEEENS0_30AllowLabelledFunctionStatementEPb+0x24)[0xffff84]
    out/Release/node(_ZN2v88internal10ParserBaseINS0_6ParserEE10ParseBlockEPNS0_8ZoneListIPKNS0_12AstRawStringEEEPb+0x129)[0x1001979]
    out/Release/node(_ZN2v88internal10ParserBaseINS0_6ParserEE14ParseStatementEPNS0_8ZoneListIPKNS0_12AstRawStringEEENS0_30AllowLabelledFunctionStatementEPb+0x335)[0x1000295]
    out/Release/node(_ZN2v88internal10ParserBaseINS0_6ParserEE20ParseScopedStatementEPNS0_8ZoneListIPKNS0_12AstRawStringEEEPb+0x34)[0x1012cb4]
    out/Release/node(_ZN2v88internal10ParserBaseINS0_6ParserEE16ParseIfStatementEPNS0_8ZoneListIPKNS0_12AstRawStringEEEPb+0x1cb)[0x1012f7b]
    out/Release/node(_ZN2v88internal10ParserBaseINS0_6ParserEE14ParseStatementEPNS0_8ZoneListIPKNS0_12AstRawStringEEENS0_30AllowLabelledFunctionStatementEPb+0x20d)[0x100016d]
    out/Release/node(_ZN2v88internal10ParserBaseINS0_6ParserEE10ParseBlockEPNS0_8ZoneListIPKNS0_12AstRawStringEEEPb+0x129)[0x1001979]
    out/Release/node(_ZN2v88internal10ParserBaseINS0_6ParserEE14ParseStatementEPNS0_8ZoneListIPKNS0_12AstRawStringEEENS0_30AllowLabelledFunctionStatementEPb+0x335)[0x1000295]
    out/Release/node(_ZN2v88internal10ParserBaseINS0_6ParserEE20ParseScopedStatementEPNS0_8ZoneListIPKNS0_12AstRawStringEEEPb+0x34)[0x1012cb4]
    out/Release/node(_ZN2v88internal10ParserBaseINS0_6ParserEE16ParseIfStatementEPNS0_8ZoneListIPKNS0_12AstRawStringEEEPb+0x1cb)[0x1012f7b]
    out/Release/node(_ZN2v88internal10ParserBaseINS0_6ParserEE14ParseStatementEPNS0_8ZoneListIPKNS0_12AstRawStringEEENS0_30AllowLabelledFunctionStatementEPb+0x20d)[0x100016d]
    out/Release/node(_ZN2v88internal10ParserBaseINS0_6ParserEE10ParseBlockEPNS0_8ZoneListIPKNS0_12AstRawStringEEEPb+0x129)[0x1001979]
    out/Release/node(_ZN2v88internal10ParserBaseINS0_6ParserEE14ParseStatementEPNS0_8ZoneListIPKNS0_12AstRawStringEEENS0_30AllowLabelledFunctionStatementEPb+0x335)[0x1000295]
    out/Release/node(_ZN2v88internal10ParserBaseINS0_6ParserEE20ParseScopedStatementEPNS0_8ZoneListIPKNS0_12AstRawStringEEEPb+0x34)[0x1012cb4]
    out/Release/node(_ZN2v88internal10ParserBaseINS0_6ParserEE16ParseIfStatementEPNS0_8ZoneListIPKNS0_12AstRawStringEEEPb+0x1cb)[0x1012f7b]
    out/Release/node(_ZN2v88internal10ParserBaseINS0_6ParserEE14ParseStatementEPNS0_8ZoneListIPKNS0_12AstRawStringEEENS0_30AllowLabelledFunctionStatementEPb+0x20d)[0x100016d]
    out/Release/node(_ZN2v88internal10ParserBaseINS0_6ParserEE18ParseStatementListEPNS0_8ZoneListIPNS0_9StatementEEEibPb+0x98)[0x10005a8]
    out/Release/node(_ZN2v88internal10ParserBaseINS0_6ParserEE17ParseFunctionBodyEPNS0_8ZoneListIPNS0_9StatementEEEPKNS0_12AstRawStringEiRKNS0_22ParserFormalParametersENS0_12FunctionKindENS0_15FunctionLiteral12FunctionTypeEPb+0x2d7)[0x10015c7]
    out/Release/node(_ZN2v88internal6Parser13ParseFunctionEPKNS0_12AstRawStringEiNS0_12FunctionKindENS0_15FunctionLiteral12FunctionTypeEPNS0_16DeclarationScopeEPiSA_PbSA_SB_+0x2da)[0x100538a]
    out/Release/node(_ZN2v88internal6Parser20ParseFunctionLiteralEPKNS0_12AstRawStringENS0_7Scanner8LocationENS0_20FunctionNameValidityENS0_12FunctionKindEiNS0_15FunctionLiteral12FunctionTypeENS0_12LanguageModeEPb+0x39e)[0x1005d2e]
    out/Release/node(_ZN2v88internal6Parser15DoParseFunctionEPNS0_9ParseInfoEPKNS0_12AstRawStringE+0x4d4)[0x1008d84]
    out/Release/node(_ZN2v88internal6Parser13ParseFunctionEPNS0_7IsolateEPNS0_9ParseInfoENS0_6HandleINS0_18SharedFunctionInfoEEE+0x254)[0x10093f4]
    out/Release/node(_ZN2v88internal7parsing13ParseFunctionEPNS0_9ParseInfoENS0_6HandleINS0_18SharedFunctionInfoEEEPNS0_7IsolateE+0x1a6)[0x10177a6]
    out/Release/node(_ZN2v88internal8Compiler7CompileENS0_6HandleINS0_18SharedFunctionInfoEEENS1_18ClearExceptionFlagE+0x2ed)[0xdb9acd]
    out/Release/node(_ZN2v88internal8Compiler7CompileENS0_6HandleINS0_10JSFunctionEEENS1_18ClearExceptionFlagE+0x1da)[0xdba15a]
    out/Release/node(_ZN2v88internal19Runtime_CompileLazyEiPPNS0_6ObjectEPNS0_7IsolateE+0x8a)[0x10aa03a]
    [0x36d938d8465d]
    ======= Memory map: ========
    00400000-01fd8000 r-xp 00000000 ca:01 1464808                            /home/iojs/build/workspace/node-test-commit-linux/nodes/centos7-64/out/Release/node
    021d7000-021d8000 r--p 01bd7000 ca:01 1464808                            /home/iojs/build/workspace/node-test-commit-linux/nodes/centos7-64/out/Release/node
    021d8000-021f2000 rw-p 01bd8000 ca:01 1464808                            /home/iojs/build/workspace/node-test-commit-linux/nodes/centos7-64/out/Release/node
    021f2000-0220a000 rw-p 00000000 00:00 0 
    02a53000-02b7b000 rw-p 00000000 00:00 0                                  [heap]
    160faf80000-160fb000000 rw-p 00000000 00:00 0 
    3edd8680000-3edd8700000 rw-p 00000000 00:00 0 
    3f4b0400000-3f4b0480000 rw-p 00000000 00:00 0 
    419ec900000-419ec94d000 rw-p 00000000 00:00 0 
    9be8ea00000-9be8ea05000 rw-p 00000000 00:00 0 
    13e792b00000-13e792b80000 rw-p 00000000 00:00 0 
    15e061e80000-15e061f00000 rw-p 00000000 00:00 0 
    182a8d780000-182a8d800000 rw-p 00000000 00:00 0 
    1e3c74800000-1e3c74880000 rw-p 00000000 00:00 0 
    2463e4280000-2463e4300000 rw-p 00000000 00:00 0 
    29114f180000-29114f200000 rw-p 00000000 00:00 0 
    2c0f76200000-2c0f76280000 rw-p 00000000 00:00 0 
    2e1488300000-2e1488380000 rw-p 00000000 00:00 0 
    345e46f5d000-345e46f60000 ---p 00000000 00:00 0 
    345e46f60000-345e46f68000 rw-p 00000000 00:00 0 
    345e46f68000-345e46f69000 ---p 00000000 00:00 0 
    35c072300000-35c072380000 rw-p 00000000 00:00 0 
    366feee80000-366feef00000 rw-p 00000000 00:00 0 
    36d938bec000-36d938c00000 ---p 00000000 00:00 0 
    36d938c00000-36d938c03000 rw-p 00000000 00:00 0 
    36d938c03000-36d938c04000 ---p 00000000 00:00 0 
    36d938c04000-36d938c05000 rwxp 00000000 00:00 0 
    36d938c05000-36d938c80000 ---p 00000000 00:00 0 
    36d938c80000-36d938c83000 rw-p 00000000 00:00 0 
    36d938c83000-36d938c84000 ---p 00000000 00:00 0 
    36d938c84000-36d938c85000 rwxp 00000000 00:00 0 
    36d938c85000-36d938d00000 ---p 00000000 00:00 0 
    36d938d00000-36d938d03000 rw-p 00000000 00:00 0 
    36d938d03000-36d938d04000 ---p 00000000 00:00 0 
    36d938d04000-36d938d05000 rwxp 00000000 00:00 0 
    36d938d05000-36d938d80000 ---p 00000000 00:00 0 
    36d938d80000-36d938d83000 rw-p 00000000 00:00 0 
    36d938d83000-36d938d84000 ---p 00000000 00:00 0 
    36d938d84000-36d938dff000 rwxp 00000000 00:00 0 
    36d938dff000-36d938e00000 ---p 00000000 00:00 0 
    36d938e00000-36d938e03000 rw-p 00000000 00:00 0 
    36d938e03000-36d938e04000 ---p 00000000 00:00 0 
    36d938e04000-36d938e7f000 rwxp 00000000 00:00 0 
    36d938e7f000-36d938e80000 ---p 00000000 00:00 0 
    36d938e80000-36d938e83000 rw-p 00000000 00:00 0 
    36d938e83000-36d938e84000 ---p 00000000 00:00 0 
    36d938e84000-36d938eff000 rwxp 00000000 00:00 0 
    36d938eff000-36d938f00000 ---p 00000000 00:00 0 
    36d938f00000-36d938f03000 rw-p 00000000 00:00 0 
    36d938f03000-36d938f04000 ---p 00000000 00:00 0 
    36d938f04000-36d938f7f000 rwxp 00000000 00:00 0 
    36d938f7f000-36d958bec000 ---p 00000000 00:00 0 
    388ffda80000-388ffdb00000 rw-p 00000000 00:00 0 
    391d64c00000-391d64c80000 rw-p 00000000 00:00 0 
    3cdfd4480000-3cdfd4500000 rw-p 00000000 00:00 0 
    7f1bb0000000-7f1bb0021000 rw-p 00000000 00:00 0 
    7f1bb0021000-7f1bb4000000 ---p 00000000 00:00 0 
    7f1bb8000000-7f1bb8021000 rw-p 00000000 00:00 0 
    7f1bb8021000-7f1bbc000000 ---p 00000000 00:00 0 
    7f1bbc000000-7f1bbc021000 rw-p 00000000 00:00 0 
    7f1bbc021000-7f1bc0000000 ---p 00000000 00:00 0 
    7f1bc0000000-7f1bc0021000 rw-p 00000000 00:00 0 
    7f1bc0021000-7f1bc4000000 ---p 00000000 00:00 0 
    7f1bc406f000-7f1bc4070000 ---p 00000000 00:00 0 
    7f1bc4070000-7f1bc4870000 rw-p 00000000 00:00 0 
    7f1bc4870000-7f1bc4871000 ---p 00000000 00:00 0 
    7f1bc4871000-7f1bc5071000 rw-p 00000000 00:00 0 
    7f1bc5071000-7f1bc5072000 ---p 00000000 00:00 0 
    7f1bc5072000-7f1bc5872000 rw-p 00000000 00:00 0 
    7f1bc5872000-7f1bc5873000 ---p 00000000 00:00 0 
    7f1bc5873000-7f1bc6073000 rw-p 00000000 00:00 0 
    7f1bc6073000-7f1bc6074000 ---p 00000000 00:00 0 
    7f1bc6074000-7f1bc6874000 rw-p 00000000 00:00 0 
    7f1bc6874000-7f1bc6875000 ---p 00000000 00:00 0 
    7f1bc6875000-7f1bc7075000 rw-p 00000000 00:00 0 
    7f1bc7075000-7f1bc7076000 ---p 00000000 00:00 0 
    7f1bc7076000-7f1bc7876000 rw-p 00000000 00:00 0 
    7f1bc7876000-7f1bc7877000 ---p 00000000 00:00 0 
    7f1bc7877000-7f1bc8077000 rw-p 00000000 00:00 0 
    7f1bc8077000-7f1bc822f000 r-xp 00000000 ca:01 656565                     /usr/lib64/libc-2.17.so
    7f1bc822f000-7f1bc842f000 ---p 001b8000 ca:01 656565                     /usr/lib64/libc-2.17.so
    7f1bc842f000-7f1bc8433000 r--p 001b8000 ca:01 656565                     /usr/lib64/libc-2.17.so
    7f1bc8433000-7f1bc8435000 rw-p 001bc000 ca:01 656565                     /usr/lib64/libc-2.17.so
    7f1bc8435000-7f1bc843a000 rw-p 00000000 00:00 0 
    7f1bc843a000-7f1bc8451000 r-xp 00000000 ca:01 656591                     /usr/lib64/libpthread-2.17.so
    7f1bc8451000-7f1bc8650000 ---p 00017000 ca:01 656591                     /usr/lib64/libpthread-2.17.so
    7f1bc8650000-7f1bc8651000 r--p 00016000 ca:01 656591                     /usr/lib64/libpthread-2.17.so
    7f1bc8651000-7f1bc8652000 rw-p 00017000 ca:01 656591                     /usr/lib64/libpthread-2.17.so
    7f1bc8652000-7f1bc8656000 rw-p 00000000 00:00 0 
    7f1bc8656000-7f1bc866b000 r-xp 00000000 ca:01 660769                     /usr/lib64/libgcc_s-4.8.5-20150702.so.1
    7f1bc866b000-7f1bc886a000 ---p 00015000 ca:01 660769                     /usr/lib64/libgcc_s-4.8.5-20150702.so.1
    7f1bc886a000-7f1bc886b000 r--p 00014000 ca:01 660769                     /usr/lib64/libgcc_s-4.8.5-20150702.so.1
    7f1bc886b000-7f1bc886c000 rw-p 00015000 ca:01 660769                     /usr/lib64/libgcc_s-4.8.5-20150702.so.1
    7f1bc886c000-7f1bc896d000 r-xp 00000000 ca:01 656573                     /usr/lib64/libm-2.17.so
    7f1bc896d000-7f1bc8b6c000 ---p 00101000 ca:01 656573                     /usr/lib64/libm-2.17.so
    7f1bc8b6c000-7f1bc8b6d000 r--p 00100000 ca:01 656573                     /usr/lib64/libm-2.17.so
    7f1bc8b6d000-7f1bc8b6e000 rw-p 00101000 ca:01 656573                     /usr/lib64/libm-2.17.so
    7f1bc8b6e000-7f1bc8c57000 r-xp 00000000 ca:01 656857                     /usr/lib64/libstdc++.so.6.0.19
    7f1bc8c57000-7f1bc8e57000 ---p 000e9000 ca:01 656857                     /usr/lib64/libstdc++.so.6.0.19
    7f1bc8e57000-7f1bc8e5f000 r--p 000e9000 ca:01 656857                     /usr/lib64/libstdc++.so.6.0.19
    7f1bc8e5f000-7f1bc8e61000 rw-p 000f1000 ca:01 656857                     /usr/lib64/libstdc++.so.6.0.19
    7f1bc8e61000-7f1bc8e76000 rw-p 00000000 00:00 0 
    7f1bc8e76000-7f1bc8e7d000 r-xp 00000000 ca:01 656595                     /usr/lib64/librt-2.17.so
    7f1bc8e7d000-7f1bc907c000 ---p 00007000 ca:01 656595                     /usr/lib64/librt-2.17.so
    7f1bc907c000-7f1bc907d000 r--p 00006000 ca:01 656595                     /usr/lib64/librt-2.17.so
    7f1bc907d000-7f1bc907e000 rw-p 00007000 ca:01 656595                     /usr/lib64/librt-2.17.so
    7f1bc907e000-7f1bc9080000 r-xp 00000000 ca:01 656571                     /usr/lib64/libdl-2.17.so
    7f1bc9080000-7f1bc9280000 ---p 00002000 ca:01 656571                     /usr/lib64/libdl-2.17.so
    7f1bc9280000-7f1bc9281000 r--p 00002000 ca:01 656571                     /usr/lib64/libdl-2.17.so
    7f1bc9281000-7f1bc9282000 rw-p 00003000 ca:01 656571                     /usr/lib64/libdl-2.17.so
    7f1bc9282000-7f1bc92a3000 r-xp 00000000 ca:01 656554                     /usr/lib64/ld-2.17.so
    7f1bc9493000-7f1bc949a000 rw-p 00000000 00:00 0 
    7f1bc949d000-7f1bc949e000 rw-p 00000000 00:00 0 
    7f1bc949e000-7f1bc949f000 ---p 00000000 00:00 0 
    7f1bc949f000-7f1bc94a3000 rw-p 00000000 00:00 0 
    7f1bc94a3000-7f1bc94a4000 r--p 00021000 ca:01 656554                     /usr/lib64/ld-2.17.so
    7f1bc94a4000-7f1bc94a5000 rw-p 00022000 ca:01 656554                     /usr/lib64/ld-2.17.so
    7f1bc94a5000-7f1bc94a6000 rw-p 00000000 00:00 0 
    7ffc1ca1b000-7ffc1ca3c000 rw-p 00000000 00:00 0                          [stack]
    7ffc1ca5b000-7ffc1ca5d000 r-xp 00000000 00:00 0                          [vdso]
    ffffffffff600000-ffffffffff601000 r-xp 00000000 00:00 0                  [vsyscall]
  ...

@Trott
Copy link
Member Author

Trott commented Dec 31, 2017

Pinging the async-hooks and zlib folks who also know C++: @addaleax @trevnorris Any idea what the crash above could be all about? It seems very surprising to me that this change (which is only to JS files in benchmark and test) could result in that crash. But it's been consistent across three CI runs.

@addaleax
Copy link
Member

@Trott I can’t seem to reproduce that locally, and this looks like a generic memory corruption bug so the output doesn’t really help with pinpointing the source :/

So the best approach might be for somebody to SSH into the machines and run the failing tests under valgrind or sth like that?

maclover7
maclover7 reviewed Dec 31, 2017
View reviewed changes
Copy link
Contributor

@maclover7 maclover7 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

-0 on this. I think it's good to try and split out test/common.js, but I think having multiple requires (separate modules) might confuse new readers to the tests... not sure if the eventual plan is to migrate other chunks of functionality to separate files, but it looks a little odd to have tmpdir be doing its own thing (not that 66 lines isn't worth being split out into its own module).

@Trott
Copy link
Member Author

Trott commented Dec 31, 2017

not sure if the eventual plan is to migrate other chunks of functionality to separate files,

That is indeed the plan. Or, uh, my plan at least. It's already happened with benchmark, dns, countdown, fixtures, and a few others.

My hope is that eventually common will be only things that are required by nearly all tests (like the global leak checking) or things that are ubiquitous (like common.mustCall()). But not everyone may be on board with that approach.

@Trott Trott force-pushed the tmpdir branch 6 times, most recently from fd94e48 to caf966c Compare January 6, 2018 02:17
@Trott
Copy link
Member Author

Trott commented Feb 1, 2018

https://ci.nodejs.org/job/node-test-pull-request/12871/

@Trott
Copy link
Member Author

Trott commented Feb 1, 2018

Linux failure seems build related. Re-running: https://ci.nodejs.org/job/node-test-commit-linux/16011/

@Trott
Copy link
Member Author

Trott commented Feb 1, 2018 •
edited
Loading

Failure on Linux is indeed unrelated but I had to log into the box to terminate some stray processes to fix it.

One more time with CI: https://ci.nodejs.org/job/node-test-commit-linux/16012/

And one more CI after that too: https://ci.nodejs.org/job/node-test-commit-linux/16013/

And yet more: https://ci.nodejs.org/job/node-test-commit-linux/16014/

And finally with the host that was failing repeatedly: https://ci.nodejs.org/job/node-test-commit-linux/16015/

And yet again with that host because it build-failed that last time.
(╯°□°)╯︵ ┻━┻
https://ci.nodejs.org/job/node-test-commit-linux/16016/

@Trott
Copy link
Member Author

Trott commented Feb 1, 2018

Got a green run on Linux finally. Landing...

@Trott
Copy link
Member Author

Trott commented Feb 1, 2018

Landed in bf6ce47

@Trott Trott closed this Feb 1, 2018
Trott added a commit to Trott/io.js that referenced this pull request Feb 1, 2018
@Trott
test: move tmpdir to submodule of common
bf6ce47 
Move tmpdir functionality to its own module (common/tmpdir).

PR-URL: nodejs#17856
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
MylesBorins pushed a commit that referenced this pull request Feb 20, 2018
@Trott @MylesBorins
test: move tmpdir to submodule of common
dba6652 
Move tmpdir functionality to its own module (common/tmpdir).

PR-URL: #17856
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
MylesBorins pushed a commit that referenced this pull request Feb 21, 2018
@Trott @MylesBorins
test: move tmpdir to submodule of common
ce07810 
Move tmpdir functionality to its own module (common/tmpdir).

PR-URL: #17856
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
MylesBorins pushed a commit that referenced this pull request Feb 21, 2018
@Trott @MylesBorins
test: move tmpdir to submodule of common
eb25252 
Move tmpdir functionality to its own module (common/tmpdir).

PR-URL: #17856
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
@MylesBorins MylesBorins mentioned this pull request Feb 21, 2018
Merged
@MylesBorins
Copy link
Contributor

Should this be backported to v8.x-staging or v6.x-staging? If yes please follow the guide and raise a backport PR, if not let me know or add the dont-land-on label.

Trott added a commit to Trott/io.js that referenced this pull request Mar 20, 2018
@Trott
test: move tmpdir to submodule of common
371a004 
Move tmpdir functionality to its own module (common/tmpdir).

PR-URL: nodejs#17856
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
@Trott Trott mentioned this pull request Mar 20, 2018
Closed
2 tasks
@Trott
Copy link
Member Author

Trott commented Mar 20, 2018

@MylesBorins v8.x backport in #19488

MylesBorins pushed a commit that referenced this pull request Mar 21, 2018
@Trott @MylesBorins
test: move tmpdir to submodule of common
f91064f 
Move tmpdir functionality to its own module (common/tmpdir).

Backport-PR-URL: #19488
PR-URL: #17856
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
MylesBorins pushed a commit that referenced this pull request Mar 28, 2018
@Trott @MylesBorins
test: move tmpdir to submodule of common
f687bb1 
Move tmpdir functionality to its own module (common/tmpdir).

Backport-PR-URL: #19488
PR-URL: #17856
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
MylesBorins pushed a commit that referenced this pull request Mar 30, 2018
@Trott @MylesBorins
test: move tmpdir to submodule of common
a1b0d5d 
Move tmpdir functionality to its own module (common/tmpdir).

Backport-PR-URL: #19488
PR-URL: #17856
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
@MylesBorins MylesBorins mentioned this pull request May 2, 2018
Merged
MayaLekova pushed a commit to MayaLekova/node that referenced this pull request May 8, 2018
@Trott @MayaLekova
test: move tmpdir to submodule of common
24b2139 
Move tmpdir functionality to its own module (common/tmpdir).

PR-URL: nodejs#17856
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
@Trott Trott deleted the tmpdir branch January 13, 2022 22:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@maclover7 maclover7 maclover7 left review comments

@gibfahn gibfahn gibfahn approved these changes

@BridgeAR BridgeAR BridgeAR approved these changes

@joyeecheung joyeecheung joyeecheung approved these changes

@jasnell jasnell jasnell approved these changes

Assignees
No one assigned
Labels
addons Issues and PRs related to native addons. async_hooks Issues and PRs related to the async hooks subsystem. benchmark Issues and PRs related to the benchmark subsystem. http Issues or PRs related to the http subsystem. module Issues and PRs related to the module subsystem. test Issues and PRs related to the tests.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
10 participants
@Trott @gibfahn @addaleax @fhinkel @MylesBorins @jasnell @maclover7 @joyeecheung @BridgeAR @nodejs-github-bot