Skip to content

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

@TrottTrott 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

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

Trott commented Jan 3, 2021

Copy link
Copy Markdown
MemberAuthor

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/

@TrottTrott added the request-ci Add this label to start a Jenkins CI on a PR. label Jan 3, 2021
@github-actionsgithub-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jan 3, 2021
@nodejs-github-bot

nodejs-github-bot commented Jan 3, 2021

Copy link
Copy Markdown
Collaborator

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

@richardlau

richardlau commented Jan 3, 2021

Copy link
Copy Markdown
Member

cc @nodejs/platform-ibmi

mhdawson
mhdawson approved these changes Jan 4, 2021
View reviewed changes

@mhdawsonmhdawson 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.

LGTM

@Trott
test: fix test-memory-usage.js for IBMi
6a5d628
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
Trott merged commit 6a5d628 into nodejs:masterJan 5, 2021
@Trott

Trott commented Jan 5, 2021

Copy link
Copy Markdown
MemberAuthor

Landed in 6a5d628

@Trott
Trott deleted the rss-test branch January 5, 2021 14:48
danielleadams pushed a commit that referenced this pull request Jan 12, 2021
@Trott@danielleadams
test: fix test-memory-usage.js for IBMi
d48e00e
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>
@danielleadamsdanielleadams mentioned this pull request Jan 12, 2021
Merged
targos pushed a commit that referenced this pull request Aug 8, 2021
@Trott@targos
test: fix test-memory-usage.js for IBMi
3913c70
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
@Trott@BethGriggs
test: fix test-memory-usage.js for IBMi
726aed0
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
@Trott@MylesBorins
test: fix test-memory-usage.js for IBMi
815d59a
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>
@targostargos mentioned this pull request Sep 4, 2021
Merged
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@watildewatildewatilde approved these changes
@richardlaurichardlaurichardlau approved these changes
@gireeshpunathilgireeshpunathilgireeshpunathil approved these changes
+1 more reviewer
@mhdawsonmhdawsonmhdawson approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

ibm iIssues and PRs related to the IBM i platform.testIssues and PRs related to the tests.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@Trott@nodejs-github-bot@richardlau@watilde@gireeshpunathil@mhdawson@targos