FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

test: fix test-memory-usage.js for IBMi by Trott · Pull Request #36758 · nodejs/node · GitHub

/ node Public

test: fix test-memory-usage.js for IBMi - #36758

Merged
Trott merged 1 commit into
nodejs:masterfrom
Trott:rss-test
Jan 5, 2021
Merged

test: fix test-memory-usage.js for IBMi#36758
Trott merged 1 commit into
nodejs:masterfrom
Trott:rss-test

Conversation

Trott commented Jan 3, 2021

Copy link
Copy Markdown
Member

Newly added process.memoryUsage.rss() will presumably return 0 on IBMi
the same way process.memoryUsage().rss does. Allow IBMi to skip the new
assertion.

The test was using a mix of assert() and assert.ok(). This change
makes it consistently use assert.ok().

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines

Trott added the ibm i Issues and PRs related to the IBM i platform. label Jan 3, 2021
nodejs-github-bot added the test Issues and PRs related to the tests. label Jan 3, 2021

Trott commented Jan 3, 2021

Copy link
Copy Markdown
Member Author

IBMi CI on master (should fail): https://ci.nodejs.org/job/node-test-commit-ibmi/210/

IBMi CI this PR (should pass): https://ci.nodejs.org/job/node-test-commit-ibmi/212/

Trott added the request-ci Add this label to start a Jenkins CI on a PR. label Jan 3, 2021
github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jan 3, 2021

Copy link
Copy Markdown
Collaborator

Copy link
Copy Markdown
Member

cc @nodejs/platform-ibmi

mhdawson left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

LGTM

Newly added process.memoryUsage.rss() will presumably return 0 on IBMi
the same way process.memoryUsage().rss does. Allow IBMi to skip the new
assertion.

The test was using a mix of `assert()` and `assert.ok()`. This change
makes it consistently use `assert.ok()`.

PR-URL: nodejs#36758
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
Trott merged commit 6a5d628 into nodejs:master Jan 5, 2021

Trott commented Jan 5, 2021

Copy link
Copy Markdown
Member Author

Landed in 6a5d628

Trott deleted the rss-test branch January 5, 2021 14:48
danielleadams pushed a commit that referenced this pull request Jan 12, 2021
Newly added process.memoryUsage.rss() will presumably return 0 on IBMi
the same way process.memoryUsage().rss does. Allow IBMi to skip the new
assertion.

The test was using a mix of `assert()` and `assert.ok()`. This change
makes it consistently use `assert.ok()`.

PR-URL: #36758
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
danielleadams mentioned this pull request Jan 12, 2021
targos pushed a commit that referenced this pull request Aug 8, 2021
Newly added process.memoryUsage.rss() will presumably return 0 on IBMi
the same way process.memoryUsage().rss does. Allow IBMi to skip the new
assertion.

The test was using a mix of `assert()` and `assert.ok()`. This change
makes it consistently use `assert.ok()`.

PR-URL: #36758
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
BethGriggs pushed a commit that referenced this pull request Aug 12, 2021
Newly added process.memoryUsage.rss() will presumably return 0 on IBMi
the same way process.memoryUsage().rss does. Allow IBMi to skip the new
assertion.

The test was using a mix of `assert()` and `assert.ok()`. This change
makes it consistently use `assert.ok()`.

PR-URL: #36758
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
MylesBorins pushed a commit that referenced this pull request Aug 31, 2021
Newly added process.memoryUsage.rss() will presumably return 0 on IBMi
the same way process.memoryUsage().rss does. Allow IBMi to skip the new
assertion.

The test was using a mix of `assert()` and `assert.ok()`. This change
makes it consistently use `assert.ok()`.

PR-URL: #36758
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ibm i Issues and PRs related to the IBM i platform. test Issues and PRs related to the tests.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants


Back | FazBrowse Home | New Git URL