GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — test_runner: add getter and setter to MockTracker by fossamagna · Pull Request #45506 · nodejs/node

Conversation

@fossamagna

@fossamagna fossamagna commented Nov 18, 2022

Copy link
Copy Markdown
Contributor

This commit allows tests in test runner to use the getter and setter methods as "syntax sugar" for MockTracker.method with the options.getter or options.setter set to true in the options.

Refs: #45326 (comment)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
bricss reacted with eyes emoji
All reactions
  • 👀 1 reaction
@nodejs-github-bot nodejs-github-bot added dont-land-on-v14.x needs-ci PRs that need a full CI run. test_runner Issues and PRs related to the test runner subsystem. labels Nov 18, 2022
MoLow
MoLow reviewed Nov 18, 2022
View reviewed changes
Comment thread lib/internal/test_runner/mock.js Resolved

Uh oh!

There was an error while loading. Please reload this page.
MoLow
MoLow reviewed Nov 18, 2022
View reviewed changes
Comment thread lib/internal/test_runner/mock.js Resolved

Uh oh!

There was an error while loading. Please reload this page.
@fossamagna
fossamagna force-pushed the add-mock-getter-setter branch from 04a27b4 to 6dca9b9 Compare November 18, 2022 14:06
cjihrig
cjihrig reviewed Nov 18, 2022
View reviewed changes
Comment thread lib/internal/test_runner/mock.js Outdated
Comment on lines 212 to 213

@cjihrig cjihrig Nov 18, 2022

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Can you swap these two lines so that getter cannot be set to a different value. Same comment below for setter.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
fossamagna reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
Comment thread test/parallel/test-runner-mocking.js Outdated

@cjihrig cjihrig Nov 18, 2022

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Can you add tests that try to set getter and setter to false. Tests that throw when trying to set setter: true when calling getter() and vice versa would be nice to.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
fossamagna reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

@fossamagna fossamagna Nov 18, 2022

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

@cjihrig Thank you for your suggestions.
If getter cannot be set to a different value, I think that validation do not work for options in method().
Do we implement validation for options in getter() and setter() ?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@cjihrig cjihrig Nov 18, 2022

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

If getter cannot be set to a different value, I think that validation do not work for options in method().

There is validation, but if you set the getter option to false, it would pass validation, but the getter() method would not make sense.

Do we implement validation for options in getter() and setter() ?

I would be OK with validating the getter or setter option to ensure that they are not set to any value besides true and deferring all other validation to method().

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread doc/api/test.md Outdated

@cjihrig cjihrig Nov 18, 2022

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Suggested change
This function is syntax sugar for [`MockTracker.method`][] with `options.getter` is `true`.
This function is syntax sugar for [`MockTracker.method`][] with `options.getter` set to `true`.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread doc/api/test.md Outdated

@cjihrig cjihrig Nov 18, 2022

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Suggested change
This function is syntax sugar for [`MockTracker.method`][] with `options.setter` is `true`.
This function is syntax sugar for [`MockTracker.method`][] with `options.setter` set to `true`.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@fossamagna
fossamagna force-pushed the add-mock-getter-setter branch 2 times, most recently from f16927b to e240af4 Compare November 18, 2022 16:00
cjihrig
cjihrig reviewed Nov 18, 2022
View reviewed changes
Comment thread doc/api/test.md Outdated

@cjihrig cjihrig Nov 18, 2022

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Sorry, I just realized that this is not sorted. mock.setter() should come after mock.reset().

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@fossamagna fossamagna Nov 19, 2022

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

I will sort them in alphabetical order.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread lib/internal/test_runner/mock.js Outdated

@cjihrig cjihrig Nov 18, 2022

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Same comment for setter:

Suggested change
'options.getter', getter, "cannot set to 'false'"
'options.getter', getter, "cannot be false"

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread lib/internal/test_runner/mock.js Outdated
Comment on lines 216 to 220

@cjihrig cjihrig Nov 18, 2022

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

We can simplify this and let method() handle the validation (same for setter).

Suggested change
const { getter = true } = options;
validateBoolean(getter, 'options.getter');
if (!getter) {
if (options?.getter === false) {

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@anonrig anonrig Nov 18, 2022

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

I didn't understand the reason/logic behind allowing only getter === true while setting it to true as default, but if it's false throw an error? Why do we even check for options?.getter at all?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@fossamagna fossamagna Nov 19, 2022

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

It is incorrect to specify options.getter for getter(). If we don't check it, getter() will not throw an error even if we specify false for options.getter. I think that it is better to check options.getter, since getter() will not throw an error if options.getter is specified as false.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
anonrig
anonrig reviewed Nov 18, 2022
View reviewed changes
Comment thread lib/internal/test_runner/mock.js Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread lib/internal/test_runner/mock.js Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread lib/internal/test_runner/mock.js Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread lib/internal/test_runner/mock.js Outdated

@anonrig anonrig Nov 18, 2022

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Suggested change
if (!setter) {
if (setter === false) {

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread lib/internal/test_runner/mock.js Outdated
Comment on lines 216 to 220

@anonrig anonrig Nov 18, 2022

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

I didn't understand the reason/logic behind allowing only getter === true while setting it to true as default, but if it's false throw an error? Why do we even check for options?.getter at all?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@fossamagna
fossamagna force-pushed the add-mock-getter-setter branch 2 times, most recently from cde2736 to 8e13ae4 Compare November 19, 2022 15:04
MoLow
MoLow approved these changes Nov 19, 2022
View reviewed changes

@MoLow MoLow left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

LGTM

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
cjihrig
cjihrig reviewed Nov 20, 2022
View reviewed changes
Comment thread doc/api/test.md Outdated

@cjihrig cjihrig Nov 20, 2022

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Please remove this.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@aduh95 aduh95 Nov 20, 2022

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Suggested change
they are sorted in alphabetical order.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@fossamagna fossamagna Nov 20, 2022

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Sorry. I remove this.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread lib/internal/test_runner/mock.js Outdated

@cjihrig cjihrig Nov 20, 2022

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Suggested change
'options.getter', getter, "cannot be 'false'"
'options.getter', getter, "cannot be false"

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
aduh95 reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
Comment thread lib/internal/test_runner/mock.js Outdated

@cjihrig cjihrig Nov 20, 2022

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Suggested change
'options.setter', setter, "cannot be 'false'"
'options.setter', setter, "cannot be false"

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
aduh95 reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
aduh95
aduh95 reviewed Nov 20, 2022
View reviewed changes
Comment thread lib/internal/test_runner/mock.js Outdated

@aduh95 aduh95 Nov 20, 2022

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Suggested change
getter: true
getter,

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread lib/internal/test_runner/mock.js Outdated
Comment on lines 214 to 216

@aduh95 aduh95 Nov 20, 2022

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

We can skip the validation if we know it's an object already.

Suggested change
}
validateObject(options, 'options');
} else {
validateObject(options, 'options');
}

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
MoLow reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
Comment thread lib/internal/test_runner/mock.js Outdated
Comment on lines 243 to 245

@aduh95 aduh95 Nov 20, 2022

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Suggested change
}
validateObject(options, 'options');
} else {
validateObject(options, 'options');
}

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread lib/internal/test_runner/mock.js Outdated

@aduh95 aduh95 Nov 20, 2022

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Suggested change
setter: true
setter,

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread doc/api/test.md Outdated

@aduh95 aduh95 Nov 20, 2022

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Suggested change
they are sorted in alphabetical order.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
aduh95
aduh95 reviewed Nov 20, 2022
View reviewed changes
Comment thread lib/internal/test_runner/mock.js Outdated
Comment on lines 220 to 221

@aduh95 aduh95 Nov 20, 2022 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

The validation is already done in this.method (if you implement my suggestion in #45506 (comment)), I suggest removing it.

Suggested change
validateBoolean(getter, 'options.getter');

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread lib/internal/test_runner/mock.js Outdated
Comment on lines 249 to 250

@aduh95 aduh95 Nov 20, 2022

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Same here.

Suggested change
validateBoolean(setter, 'options.setter');

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@fossamagna
fossamagna force-pushed the add-mock-getter-setter branch 2 times, most recently from bae6aa6 to dd8d5d3 Compare November 20, 2022 14:21
aduh95
aduh95 reviewed Nov 20, 2022
View reviewed changes
Comment thread lib/internal/test_runner/mock.js Outdated

@aduh95 aduh95 Nov 20, 2022

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Suggested change
getter
getter,

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
fossamagna reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
Comment thread lib/internal/test_runner/mock.js Outdated

@aduh95 aduh95 Nov 20, 2022

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Suggested change
setter
setter,

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
fossamagna reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
@fossamagna
test_runner: add getter and setter to MockTracker
6da996a 
This commit allows tests in test runner to use the
`getter` and `setter` methods as "syntax sugar" for
`MockTracker.method` with the `options.getter` or
`options.setter` set to true in the options.

Refs: nodejs#45326 (comment)
@fossamagna
fossamagna force-pushed the add-mock-getter-setter branch from dd8d5d3 to 6da996a Compare November 20, 2022 23:59
cjihrig
cjihrig approved these changes Nov 21, 2022
View reviewed changes
@MoLow MoLow added the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Nov 21, 2022
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Nov 21, 2022
@nodejs-github-bot

nodejs-github-bot commented Nov 21, 2022

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/48069/
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@github-actions github-actions Bot mentioned this pull request Nov 22, 2022
CI Reliability 2022-11-22 nodejs/reliability#436
Open
15 tasks
@ljharb

ljharb commented Nov 22, 2022

Copy link
Copy Markdown
Member

cc @nodejs/testing
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
aduh95
aduh95 approved these changes Nov 22, 2022
View reviewed changes

@aduh95 aduh95 left a comment

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Ideally we would have tests to ensure that passing non-boolean values to getter or setter would trigger an error. Otherwise LGTM.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@github-actions github-actions Bot mentioned this pull request Nov 23, 2022
CI Reliability 2022-11-23 nodejs/reliability#437
Open
13 tasks
@nodejs-github-bot

nodejs-github-bot commented Nov 23, 2022

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/48109/
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@github-actions github-actions Bot mentioned this pull request Nov 24, 2022
CI Reliability 2022-11-24 nodejs/reliability#438
Open
16 tasks
@cjihrig cjihrig added commit-queue PRs queued for automated landing through the Commit Queue. commit-queue-rebase PRs the Commit Queue should land as multiple self-contained commits. and removed needs-ci PRs that need a full CI run. labels Nov 27, 2022
@nodejs-github-bot nodejs-github-bot removed the commit-queue PRs queued for automated landing through the Commit Queue. label Nov 27, 2022
@nodejs-github-bot

nodejs-github-bot commented Nov 27, 2022

Copy link
Copy Markdown
Collaborator

Landed in 147d810...afed1af
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@nodejs-github-bot nodejs-github-bot closed this Nov 27, 2022
nodejs-github-bot pushed a commit that referenced this pull request Nov 27, 2022
@fossamagna @nodejs-github-bot
test_runner: add getter and setter to MockTracker
afed1af 
This commit allows tests in test runner to use the
`getter` and `setter` methods as "syntax sugar" for
`MockTracker.method` with the `options.getter` or
`options.setter` set to true in the options.

Refs: #45326 (comment)
PR-URL: #45506
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
targos pushed a commit that referenced this pull request Dec 12, 2022
@fossamagna @targos
test_runner: add getter and setter to MockTracker
efc4456 
This commit allows tests in test runner to use the
`getter` and `setter` methods as "syntax sugar" for
`MockTracker.method` with the `options.getter` or
`options.setter` set to true in the options.

Refs: #45326 (comment)
PR-URL: #45506
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
@targos targos mentioned this pull request Dec 12, 2022
v19.3.0 release proposal #45831
Merged
@targos

targos commented Dec 14, 2022

Copy link
Copy Markdown
Member

Apparently the documentation for this landed in v16.x (maintenance LTS). I don't see any other references above so maybe the feature itself wasn't backported. /cc @richardlau
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@richardlau

richardlau commented Dec 14, 2022

Copy link
Copy Markdown
Member

Apparently the documentation for this landed in v16.x (maintenance LTS). I don't see any other references above so maybe the feature itself wasn't backported. /cc @richardlau

I'm confused. I don't see any references to mock(.getter/.setter) in https://nodejs.org/docs/v16.19.0/api/test.html.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@richardlau

richardlau commented Dec 14, 2022

Copy link
Copy Markdown
Member

Mocking itself is one of the commits in the open backport #45602 but that hasn't landed on v16.x-staging yet.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@targos

targos commented Dec 14, 2022

Copy link
Copy Markdown
Member

I'll dig in tomorrow, but while releasing v19.3.0, I had to update the added yaml field, which had an initial value for v16
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@targos

targos commented Dec 14, 2022

Copy link
Copy Markdown
Member

At least that's the case on the main branch
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@richardlau

richardlau commented Dec 14, 2022

Copy link
Copy Markdown
Member

Ah it's possible the release commit merged incorrectly.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@richardlau richardlau mentioned this pull request Dec 14, 2022
doc: fix doc metadata for Node.js 16.19.0 #45863
Merged
@richardlau

richardlau commented Dec 14, 2022

Copy link
Copy Markdown
Member

Yes, it was indeed. Another example of nodejs/Release#771 😞. Thanks for catching it.

Opened #45863 to correct.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
richardlau added a commit to richardlau/node-1 that referenced this pull request Dec 15, 2022
@richardlau
doc: fix doc metadata for Node.js 16.19.0
96174e2 
When cherry-picking the release commit for Node.js 16.19.0 to the `main`
branch git updated the metadata for the wrong item in `doc/api/test.md`.

Refs: nodejs#45506 (comment)
Refs: nodejs/Release#771
nodejs-github-bot pushed a commit that referenced this pull request Dec 16, 2022
@richardlau
doc: fix doc metadata for Node.js 16.19.0
97165fc 
When cherry-picking the release commit for Node.js 16.19.0 to the `main`
branch git updated the metadata for the wrong item in `doc/api/test.md`.

Refs: #45506 (comment)
Refs: nodejs/Release#771
PR-URL: #45863
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Tierney Cyren <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
@danielleadams danielleadams mentioned this pull request Dec 30, 2022
v18.13.0 release proposal #46025
Merged
danielleadams pushed a commit that referenced this pull request Dec 30, 2022
@fossamagna @danielleadams
test_runner: add getter and setter to MockTracker
e87d76f 
This commit allows tests in test runner to use the
`getter` and `setter` methods as "syntax sugar" for
`MockTracker.method` with the `options.getter` or
`options.setter` set to true in the options.

Refs: #45326 (comment)
PR-URL: #45506
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
danielleadams pushed a commit that referenced this pull request Jan 3, 2023
@fossamagna @danielleadams
test_runner: add getter and setter to MockTracker
a56560f 
This commit allows tests in test runner to use the
`getter` and `setter` methods as "syntax sugar" for
`MockTracker.method` with the `options.getter` or
`options.setter` set to true in the options.

Refs: #45326 (comment)
PR-URL: #45506
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
danielleadams pushed a commit that referenced this pull request Jan 5, 2023
@fossamagna @danielleadams
test_runner: add getter and setter to MockTracker
cdb479b 
This commit allows tests in test runner to use the
`getter` and `setter` methods as "syntax sugar" for
`MockTracker.method` with the `options.getter` or
`options.setter` set to true in the options.

Refs: #45326 (comment)
PR-URL: #45506
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
MoLow pushed a commit to MoLow/node-core-test that referenced this pull request Feb 2, 2023
@fossamagna @MoLow
feat: add getter and setter to MockTracker
b942f93 
This commit allows tests in test runner to use the
`getter` and `setter` methods as "syntax sugar" for
`MockTracker.method` with the `options.getter` or
`options.setter` set to true in the options.

Refs: nodejs/node#45326 (comment)
PR-URL: nodejs/node#45506
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
(cherry picked from commit afed1afa55962211b6b56c2068d520b4d8a08888)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@anonrig anonrig anonrig left review comments
@cjihrig cjihrig cjihrig approved these changes
@MoLow MoLow MoLow approved these changes
@aduh95 aduh95 aduh95 approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

commit-queue-rebase PRs the Commit Queue should land as multiple self-contained commits. test_runner Issues and PRs related to the test runner subsystem.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

9 participants

@fossamagna @nodejs-github-bot @ljharb @targos @richardlau @anonrig @cjihrig @MoLow @aduh95
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.