test_runner: support object property mocking by idango10 · Pull Request #58438 · nodejs/node · GitHub

This PR adds object property mocking to the test runner.

Example usage:

test('mocks a property value', (t) => {
  const obj = { foo: 42 };
  const prop = t.mock.property(obj, 'foo', 100);

  assert.strictEqual(obj.foo, 100);
  assert.strictEqual(prop.mock.accessCount(), 1);
  assert.strictEqual(prop.mock.accesses[0].type, 'get');
  assert.strictEqual(prop.mock.accesses[0].value, 100);

  obj.foo = 200;
  assert.strictEqual(prop.mock.accessCount(), 2);
  assert.strictEqual(prop.mock.accesses[1].type, 'set');
  assert.strictEqual(prop.mock.accesses[1].value, 200);

  prop.mock.restore();
  assert.strictEqual(obj.foo, 42);
});

Review requested:

@nodejs/test_runner

Codecov Report

Attention: Patch coverage is 99.38650% with 1 line in your changes missing coverage. Please review.

Project coverage is 90.22%. Comparing base (d96d57d) to head (e1d143f).
Report is 135 commits behind head on main.

Files with missing lines	Patch %	Lines
lib/internal/test_runner/mock/mock.js	99.38%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #58438      +/-   ##
==========================================
+ Coverage   90.20%   90.22%   +0.01%     
==========================================
  Files         635      635              
  Lines      187174   187774     +600     
  Branches    36751    36866     +115     
==========================================
+ Hits       168846   169421     +575     
- Misses      11120    11126       +6     
- Partials     7208     7227      +19

Files with missing lines	Coverage Δ
lib/internal/test_runner/mock/mock.js	99.28% <99.38%> (+0.02%)	⬆️

... and 76 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Thanks for working on this. I think this is a good start, but I'd like to see it more unified with the rest of the mocking framework (particularly mock.method() since the use case is so similar)

- `mock.value` -> `mock.property`. - `value` parameter is now optional. - add `MockValueContext` section to the docs. - align API with the rest of the mocking framework. - add more tests

Thanks for working on this. I think this is a good start, but I'd like to see it more unified with the rest of the mocking framework (particularly mock.method() since the use case is so similar)

Thanks for the thorough review! 😃
I think I've addressed all your comments and pushed the changes. Let me know if there's anything else you'd like me to tweak.

…provided and rename property mock tests to align with the ones in `test-runner-mocking.js`.

Hey, welcome @idango10 nice seeing you around here, I trust Colin and agree with his feedback.

This is looking pretty good to me. Two notes:

It doesn't appear that a mockImplementationOnce() equivalent is implemented.
Can we add a test for explicitly mocking a property as undefined. This will definitely come up eventually, and I don't think the current implementation can support it. It might take a little bit of work to distinguish between the "explicit undefined" and "use the previously existing value" use cases.

…d not passing value parameter at all and add test for undefined mock property value

This is looking pretty good to me. Two notes:

It doesn't appear that a mockImplementationOnce() equivalent is implemented.

Can we add a test for explicitly mocking a property as undefined. This will definitely come up eventually, and I don't think the current implementation can support it. It might take a little bit of work to distinguish between the "explicit undefined" and "use the previously existing value" use cases.

I'm wondering about how we should implement this. It's pretty problematic since accesses includes both get and set actions. For gets we have no problem, but set is a different case. If a set happens at the same access index, it will consume the "once" value and change the mocked property to the value we only intended to use once.
Great catch, it was indeed a bug. Fixed and added a test for it.

I'm wondering about how we should implement this. It's pretty problematic since accesses includes both get and set actions. For gets we have no problem, but set is a different case. If a set happens at the same access index, it will consume the "once" value and change the mocked property to the value we only intended to use once.

Personally, I would just let it apply to both sets and gets and document the caveat you mentioned here. We could make it only apply to gets, but that would be inconsistent with how the rest of the implementation works and we would still need to document that caveat.

…r single invocation behavior

I'm wondering about how we should implement this. It's pretty problematic since accesses includes both get and set actions. For gets we have no problem, but set is a different case. If a set happens at the same access index, it will consume the "once" value and change the mocked property to the value we only intended to use once.

Personally, I would just let it apply to both sets and gets and document the caveat you mentioned here. We could make it only apply to gets, but that would be inconsistent with how the rest of the implementation works and we would still need to document that caveat.

I added mockImplementationOnce() which applies to both sets and gets, as you suggested. Please let me know if there's anything else I should change 😃

Suggested change

      
              for `object[valueName]`. **Default:** The original property value.
          
              for `object[propertyName]`. **Default:** The original property value.

valueName -> propertyName

Hey @cjihrig, hope you're doing well! Sorry to ping you, but I wanted to check if you've had a chance to look at my latest changes. Please let me know if anything needs tweaking. Thanks a lot!

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

it seems like the API just takes a value - what about getters/setters, enumerability, writability?

The notable-change PRs with changes that should be highlighted in changelogs. label has been added by @atlowChemi.

Please suggest a text for the release notes if you'd like to include a more detailed summary, then proceed to update the PR description with the text or a link to the notable change suggested text comment. Otherwise, the commit will be placed in the Other Notable Changes section.

it seems like the API just takes a value - what about getters/setters, enumerability, writability?

Mocking a value for testing why would modification of property descriptors be desirable?

Landed in 905a722

@peteduel because enumerability and writability are all things that should be tested?

PR-URL: nodejs#58438 Fixes: nodejs#58322 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Chemi Atlow <chemi@atlow.co.il>

PR-URL: #58438 Fixes: #58322 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Chemi Atlow <chemi@atlow.co.il>

Notable changes: doc: * add islandryu to collaborators (Shima Ryuhei) #58714 fs: * (SEMVER-MINOR) allow correct handling of burst in fs-events with AsyncIterator (Philipp Dunkel) #58490 module: * (SEMVER-MINOR) remove experimental warning from type stripping (Marco Ippolito) #58643 test_runner: * (SEMVER-MINOR) support object property mocking (Idan Goshen) #58438 url: * (SEMVER-MINOR) add fileURLToPathBuffer API (James M Snell) #58700 PR-URL: #58727

Notable changes: doc: * add islandryu to collaborators (Shima Ryuhei) #58714 fs: * (SEMVER-MINOR) allow correct handling of burst in fs-events with AsyncIterator (Philipp Dunkel) #58490 module: * (SEMVER-MINOR) remove experimental warning from type stripping (Marco Ippolito) #58643 test_runner: * (SEMVER-MINOR) support object property mocking (Idan Goshen) #58438 url: * (SEMVER-MINOR) add fileURLToPathBuffer API (James M Snell) #58700 PR-URL: #58804

Notable changes: doc: * add islandryu to collaborators (Shima Ryuhei) #58714 fs: * (SEMVER-MINOR) allow correct handling of burst in fs-events with AsyncIterator (Philipp Dunkel) #58490 module: * (SEMVER-MINOR) remove experimental warning from type stripping (Marco Ippolito) #58643 test: * fix test-timeout-flag after revert of auto subtest wait (Pietro Marchini) #58282 test_runner: * Revert "test_runner: remove promises returned by t.test() (Romain Menke) #58282 * Revert "test_runner: remove promises returned by test() (Romain Menke) #58282 * Revert "test_runner: automatically wait for subtests to finish (Romain Menke) #58282 * (SEMVER-MINOR) support object property mocking (Idan Goshen) #58438 url: * (SEMVER-MINOR) add fileURLToPathBuffer API (James M Snell) #58700 PR-URL: #58813

Notable changes: doc: * add islandryu to collaborators (Shima Ryuhei) #58714 fs: * (SEMVER-MINOR) allow correct handling of burst in fs-events with AsyncIterator (Philipp Dunkel) #58490 module: * (SEMVER-MINOR) remove experimental warning from type stripping (Marco Ippolito) #58643 test: * fix test-timeout-flag after revert of auto subtest wait (Pietro Marchini) #58282 test_runner: * (SEMVER-MINOR) support object property mocking (Idan Goshen) #58438 url: * (SEMVER-MINOR) add fileURLToPathBuffer API (James M Snell) #58700 PR-URL: #58813

PR-URL: #58438 Fixes: #58322 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Chemi Atlow <chemi@atlow.co.il>

Notable changes: crypto: * update root certificates to NSS 3.114 (Node.js GitHub Bot) #59571 deps: * update archs files for openssl-3.5.1 (Node.js GitHub Bot) #59234 * upgrade openssl sources to openssl-3.5.1 (Node.js GitHub Bot) #59234 http: * (SEMVER-MINOR) add Agent.agentKeepAliveTimeoutBuffer option (Haram Jeong) #59315 http2: * (SEMVER-MINOR) add support for raw header arrays in h2Stream.respond() (Tim Perry) #59455 inspector: * add http2 tracking support (Darshan Sen) #59611 sea: * (SEMVER-MINOR) implement execArgvExtension (Joyee Cheung) #59560 * (SEMVER-MINOR) support execArgv in sea config (Joyee Cheung) #59314 stream: * (SEMVER-MINOR) add brotli support to CompressionStream and DecompressionStream (Matthew Aitken) #59464 test_runner: * (SEMVER-MINOR) support object property mocking (Idan Goshen) #58438 worker: * (SEMVER-MINOR) add cpu profile APIs for worker (theanarkh) #59428 PR-URL: #59973

Notable changes: crypto: * update root certificates to NSS 3.114 (Node.js GitHub Bot) #59571 deps: * fix OpenSSL security level at 1 (Richard Lau) #59859 * upgrade openssl sources to openssl-3.5.2 (Node.js GitHub Bot) #59371 doc: * stabilize --disable-sigusr1 (Rafael Gonzaga) #59707 * mark `path.matchesGlob` as stable (Aviv Keller) #59572 http: * (SEMVER-MINOR) add Agent.agentKeepAliveTimeoutBuffer option (Haram Jeong) #59315 http2: * (SEMVER-MINOR) add support for raw header arrays in h2Stream.respond() (Tim Perry) #59455 inspector: * add http2 tracking support (Darshan Sen) #59611 sea: * (SEMVER-MINOR) implement execArgvExtension (Joyee Cheung) #59560 * (SEMVER-MINOR) support execArgv in sea config (Joyee Cheung) #59314 stream: * (SEMVER-MINOR) add brotli support to CompressionStream and DecompressionStream (Matthew Aitken) #59464 test_runner: * (SEMVER-MINOR) support object property mocking (Idan Goshen) #58438 worker: * (SEMVER-MINOR) add cpu profile APIs for worker (theanarkh) #59428 PR-URL: #59973

test_runner: support object property mocking

b117151

nodejs-github-bot added needs-ci PRs that need a full CI run. test_runner Issues and PRs related to the test runner subsystem. labels May 23, 2025

idango10 added 2 commits May 23, 2025 20:40

fix: lint

691f4d5

fix: lint

bccd7e2

cjihrig reviewed May 23, 2025

View reviewed changes

idango10 added 2 commits May 24, 2025 05:21

cr fixes:

e3f66f0

- `mock.value` -> `mock.property`. - `value` parameter is now optional. - add `MockValueContext` section to the docs. - align API with the rest of the mocking framework. - add more tests

fix: lint

5dfa826

add test to ensure original value is spied when value param is not …

e02bd78

…provided and rename property mock tests to align with the ones in `test-runner-mocking.js`.

cjihrig reviewed May 25, 2025

View reviewed changes

fix: MockPropertyContext distinguish between passing undefined an…

aaef61a

…d not passing value parameter at all and add test for undefined mock property value

feat: add mockImplementationOnce method to MockPropertyContext fo…

1d02888

…r single invocation behavior

regseb reviewed May 29, 2025

View reviewed changes

fix: typo in mock.property value description

e1d143f

valueName -> propertyName

cjihrig approved these changes Jun 5, 2025

View reviewed changes

cjihrig added the request-ci Add this label to start a Jenkins CI on a PR. label Jun 5, 2025

github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jun 5, 2025

atlowChemi approved these changes Jun 5, 2025

View reviewed changes

atlowChemi added author ready PRs that have at least one approval, no outstanding review comments, and a CI started. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. labels Jun 5, 2025

atlowChemi added semver-minor PRs that contain new features and should be released in the next minor version. notable-change PRs with changes that should be highlighted in changelogs. labels Jun 5, 2025

benjamingr added the commit-queue Add this label to land a pull request using GitHub Actions. label Jun 9, 2025

nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Jun 9, 2025

nodejs-github-bot merged commit 905a722 into nodejs:main Jun 9, 2025

idango10 deleted the idango/mock-value branch June 9, 2025 17:58

seriousme pushed a commit to seriousme/node that referenced this pull request Jun 10, 2025

test_runner: support object property mocking

bde9dc5

PR-URL: nodejs#58438 Fixes: nodejs#58322 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Chemi Atlow <chemi@atlow.co.il>

targos pushed a commit that referenced this pull request Jun 16, 2025

test_runner: support object property mocking

713fbad

PR-URL: #58438 Fixes: #58322 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Chemi Atlow <chemi@atlow.co.il>

github-actions Bot mentioned this pull request Jun 16, 2025

2025-06-17, Version 24.3.0 (Current) #58727

Closed

github-actions Bot mentioned this pull request Jun 23, 2025

2025-06-24, Version 24.3.0 (Current) #58804

Closed

github-actions Bot mentioned this pull request Jun 24, 2025

2025-06-24, Version 24.3.0 (Current) #58813

Merged

richardlau pushed a commit that referenced this pull request Sep 20, 2025

test_runner: support object property mocking

62bb80c

PR-URL: #58438 Fixes: #58322 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Chemi Atlow <chemi@atlow.co.il>

github-actions Bot mentioned this pull request Sep 22, 2025

2025-09-24, Version 22.20.0 'Jod' (LTS) #59973

Merged

regseb mentioned this pull request Oct 29, 2025

mock.property(): 'process.env' does not accept an accessor(getter/setter) descriptor #60486

Closed

AlCalzone mentioned this pull request May 13, 2026

Consider further Node.js version bumps zwave-js/zwave-js#8820

Closed

+              * `object` {Object} The object whose value is being mocked.
+              * `propertyName` {string|symbol} The identifier of the property on `object` to mock.
+              * `value` {any} An optional value used as the mock value
+                for `object[valueName]`. **Default:** The original property value.

	for `object[valueName]`. Default: The original property value.
	for `object[propertyName]`. Default: The original property value.

test_runner: support object property mocking by idango10 · Pull Request #58438 · nodejs/node · GitHub

Uh oh!

Uh oh!

test_runner: support object property mocking - #58438

test_runner: support object property mocking#58438nodejs-github-bot merged 9 commits intonodejs:mainnodejs/node:mainfrom idango10:idango/mock-valueCopy head branch name to clipboard

Conversation

idango10 commented May 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nodejs-github-bot commented May 23, 2025

Uh oh!

codecov Bot commented May 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

cjihrig left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

idango10 commented May 24, 2025

Uh oh!

benjamingr commented May 24, 2025

Uh oh!

cjihrig left a comment

Choose a reason for hiding this comment

Uh oh!

idango10 commented May 26, 2025

Uh oh!

cjihrig commented May 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

idango10 commented May 26, 2025

Uh oh!

regseb May 29, 2025

Choose a reason for hiding this comment

Uh oh!

idango10 commented Jun 5, 2025

Uh oh!

nodejs-github-bot commented Jun 5, 2025

Uh oh!

ljharb commented Jun 5, 2025

Uh oh!

github-actions Bot commented Jun 5, 2025

Uh oh!

peteduel commented Jun 6, 2025

Uh oh!

nodejs-github-bot commented Jun 9, 2025

Uh oh!

ljharb commented Jun 9, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

test_runner: support object property mocking#58438
nodejs-github-bot merged 9 commits into
nodejs:mainfrom
idango10:idango/mock-value

idango10 commented May 23, 2025 •
edited
Loading

codecov Bot commented May 23, 2025 •
edited
Loading

cjihrig commented May 26, 2025 •
edited
Loading