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

build: add pummel tests to ci runs #34289

Merged
merged 5 commits into from
Apr 10, 2021
Merged

build: add pummel tests to ci runs #34289

merged 5 commits into from
Apr 10, 2021

Conversation

Trott
Copy link
Member

@Trott Trott commented Jul 10, 2020

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

@nodejs-github-bot nodejs-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
Member Author

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.

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
Member Author

Trott commented Apr 10, 2021

Landed in 6a1986d...2853b76

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

targos commented Apr 20, 2021

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
Member Author

Trott commented Apr 20, 2021

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]>
@danielleadams danielleadams 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 free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Issues and PRs related to build files or the CI. test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants