Skip to content

Conversation

@Trott
Copy link
Member

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines

@nodejs-github-botnodejs-github-bot added build Issues and PRs related to build files or the CI. windows Issues and PRs related to the Windows platform. labels Jul 10, 2020
@nodejs-github-bot

This comment has been minimized.

@gengjiawen
Copy link
Member

ASAN check failed with, since it works in normal test, we may need to skip it in asan, like in

if(process.config.variables.asan)
common.skip('ASAN messes with memory measurements');

Fail log:

AssertionError [ERR_ASSERTION]: The expression evaluated to a falsy value: assert.ok(maxMem < 64 * 1024 * 1024) at process.<anonymous> (/home/runner/work/node/node/test/pummel/test-vm-memleak.js:61:10) at process.emit (events.js:326:22){ generatedMessage: true, code: 'ERR_ASSERTION', actual: false, expected: true, operator: '=='}Command: out/Release/node --max_old_space_size=32 /home/runner/work/node/node/test/pummel/test-vm-memleak.js

@Trott

This comment has been minimized.

@Trott
Copy link
MemberAuthor

Trott commented Jul 12, 2020

ASAN check failed with, since it works in normal test, we may need to skip it in asan, like in

Is this something you're seeing in CI or testing locally?

Same test failed on AIX. https://ci.nodejs.org/job/node-test-commit-aix/31507/nodes=aix71-ppc64/testReport/junit/(root)/test/pummel_test_vm_memleak/

@nodejs-github-bot

This comment has been minimized.

@nodejs-github-bot

This comment has been minimized.

@nodejs-github-bot

This comment has been minimized.

@nodejs-github-bot

This comment has been minimized.

@nodejs-github-bot

This comment has been minimized.

@nodejs-github-bot

This comment has been minimized.

@gengjiawen
Copy link
Member

on macOS, not sure flaky

Also assert.js:103 throw new AssertionError(obj); ^AssertionError [ERR_ASSERTION]: Expected values to be strictly equal:false !== true at Timeout._onTimeout (/Users/runner/work/node/node/test/pummel/test-timers.js:55:10) at listOnTimeout (internal/timers.js:555:17) at processTimers (internal/timers.js:498:7){ generatedMessage: true, code: 'ERR_ASSERTION', actual: false, expected: true, operator: 'strictEqual'}Command: out/Release/node /Users/runner/work/node/node/test/pummel/test-timers.js

@nodejs-github-bot

This comment has been minimized.

@nodejs-github-bot

This comment has been minimized.

@Trott

This comment has been minimized.

@nodejs-github-bot

This comment has been minimized.

@nodejs-github-bot

This comment has been minimized.

@nodejs-github-bot

This comment has been minimized.

@nodejs-github-bot

This comment has been minimized.

@nodejs-github-bot

This comment has been minimized.

@nodejs-github-bot

This comment has been minimized.

@nodejs-github-bot

This comment has been minimized.

Trott added 2 commits April 10, 2021 15:18
The pummel tests result in the Windows coverage runs in CI to exhaust memory, so we need to bump up the heap size. PR-URL: nodejs#34289 Reviewed-By: Richard Lau <[email protected]>
@Trott
Copy link
MemberAuthor

Landed in 6a1986d...2853b76

@TrottTrott merged commit 2853b76 into nodejs:masterApr 10, 2021
@TrottTrott deleted the pummel-in-ci branch April 10, 2021 22:19
@targos
Copy link
Member

These tests take quite a long time on the Raspberry Pis (and sometimes fail with a timeout). Could we disable pummel tests on node-test-binary-arm-12+ ?

@Trott
Copy link
MemberAuthor

These tests take quite a long time on the Raspberry Pis (and sometimes fail with a timeout). Could we disable pummel tests on node-test-binary-arm-12+ ?

Looking at Pi 2 results in https://ci.nodejs.org/job/node-test-binary-arm-12+/10400/, the longest running tests (and how long it took them to run in seconds) are:

  1. pummel/test-crypto-dh-hash-modp18 (700.975)
  2. pummel/test-policy-integrity (696.201)
  3. pummel/test-crypto-dh-hash (451.29)
  4. pummel/test-dh-regr (311.406)
  5. pummel/test-next-tick-infinite-calls (260.476)
  6. parallel/test-webcrypto-derivebits-pbkdf2 (170.465)
  7. parallel/test-heapsnapshot-near-heap-limit (136.232)
  8. sequential/test-net-bytes-per-incoming-chunk-overhead (82.495)
  9. pummel/test-fs-watch-system-limit (75.903)
  10. parallel/test-heapsnapshot-near-heap-limit-bounded (72.233)

I think it makes sense to:

  1. move those above that aren't already in pummel into pummel,
  2. move the pummel tests that only take a second or two to run to somewhere outside of pummel (although we'll need to check them for why they are in pummel in the first place--for example, if it's disk usage, that might warrant staying in pummel even if they only take a few seconds to run)
  3. set up pummel tests to skip on Raspberry Pi.

I'll get to work on this tonight and hopefully have a PR together quickly.

Trott added a commit to Trott/io.js that referenced this pull request Apr 25, 2021
Skipped the longest-running pummel tests on the Raspberry Pi devices in CI. Refs: nodejs#34289 (comment)
Trott added a commit to Trott/io.js that referenced this pull request Apr 25, 2021
Move slower tests to pummel and skip on Raspberry Pi devices in CI. Refs: nodejs#34289 (comment)
Trott added a commit to Trott/io.js that referenced this pull request Apr 27, 2021
Skipped the longest-running pummel tests on the Raspberry Pi devices in CI. Refs: nodejs#34289 (comment) PR-URL: nodejs#38394 Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Filip Skokan <[email protected]>
Trott added a commit to Trott/io.js that referenced this pull request Apr 27, 2021
Move slower tests to pummel and skip on Raspberry Pi devices in CI. Refs: nodejs#34289 (comment) PR-URL: nodejs#38395 Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Filip Skokan <[email protected]>
targos pushed a commit that referenced this pull request Apr 29, 2021
Skipped the longest-running pummel tests on the Raspberry Pi devices in CI. Refs: #34289 (comment) PR-URL: #38394 Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Filip Skokan <[email protected]>
targos pushed a commit that referenced this pull request Apr 29, 2021
Move slower tests to pummel and skip on Raspberry Pi devices in CI. Refs: #34289 (comment) PR-URL: #38395 Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Filip Skokan <[email protected]>
targos pushed a commit that referenced this pull request May 1, 2021
The check for an 800ms window makesw assumptions about a setTimeout() not running late etc. Remove it. Refs: #34289 PR-URL: #38060 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Juan José Arboleda <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]>
targos pushed a commit that referenced this pull request May 1, 2021
The test is too slow to run on the Raspberry Pi bots. (It takes over 500 seconds to run on Pi 3 bots and over 900 seconds on Pi 2 bots.) Skip it on armv6 and armv7. Refs: #34289 PR-URL: #38076 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: James M Snell <[email protected]>
targos pushed a commit that referenced this pull request May 1, 2021
The test is too slow to run on the Raspberry Pi bots. (It takes over 900 seconds to run on Pi 3 bots and even more than that on Pi 2 bots.) Skip it on armv6 and armv7. Refs: #34289 PR-URL: #34289 Reviewed-By: Richard Lau <[email protected]>
targos pushed a commit that referenced this pull request May 1, 2021
ASAN increases memory usage, which in turn messes up the memory leak test. Skip the test on ASAN. PR-URL: #34289 Reviewed-By: Richard Lau <[email protected]>
@danielleadamsdanielleadams mentioned this pull request May 3, 2021
danielleadams pushed a commit that referenced this pull request May 8, 2021
The test is too slow to run on the Raspberry Pi bots. (It takes over 900 seconds to run on Pi 3 bots and even more than that on Pi 2 bots.) Skip it on armv6 and armv7. Refs: #34289 PR-URL: #34289 Reviewed-By: Richard Lau <[email protected]>
danielleadams pushed a commit that referenced this pull request May 8, 2021
ASAN increases memory usage, which in turn messes up the memory leak test. Skip the test on ASAN. PR-URL: #34289 Reviewed-By: Richard Lau <[email protected]>
targos pushed a commit that referenced this pull request May 30, 2021
Skipped the longest-running pummel tests on the Raspberry Pi devices in CI. Refs: #34289 (comment) PR-URL: #38394 Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Filip Skokan <[email protected]>
targos pushed a commit that referenced this pull request Jun 5, 2021
Skipped the longest-running pummel tests on the Raspberry Pi devices in CI. Refs: #34289 (comment) PR-URL: #38394 Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Filip Skokan <[email protected]>
targos pushed a commit that referenced this pull request Jun 5, 2021
Skipped the longest-running pummel tests on the Raspberry Pi devices in CI. Refs: #34289 (comment) PR-URL: #38394 Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Filip Skokan <[email protected]>
targos pushed a commit that referenced this pull request Jun 11, 2021
Skipped the longest-running pummel tests on the Raspberry Pi devices in CI. Refs: #34289 (comment) PR-URL: #38394 Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Filip Skokan <[email protected]>
targos pushed a commit that referenced this pull request Sep 7, 2021
Force garbage collection so that the memory leak is more easily differentiated from ordinary not-garbage-collected memory. Refs: #34289 PR-URL: #38054 Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

buildIssues and PRs related to build files or the CI.testIssues and PRs related to the tests.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@Trott@nodejs-github-bot@gengjiawen@richardlau@targos