test_runner: preserve original property descriptor when mutating objects#49433
Conversation
|
Review requested:
|
768af5c to
3fda352
Compare
ljharb
left a comment
There was a problem hiding this comment.
would be good to add a test or two checking enumerability, since that's likely the only thing different with this change
Uh, good catch! Gonna do it in a few |
Signed-off-by: Erick Wendel <erick.workspace@gmail.com>
Signed-off-by: Erick Wendel <erick.workspace@gmail.com>
93a9e75 to
64f872a
Compare
done! |
Commit Queue failed- Loading data for nodejs/node/pull/49433 ✔ Done loading data for nodejs/node/pull/49433 ----------------------------------- PR info ------------------------------------ Title test_runner: preserve original property descriptor when mutating objects (#49433) Author Erick Wendel (@ErickWendel) Branch ErickWendel:test_runner/mock-preserve-original-property-descriptor -> nodejs:main Labels author ready, needs-ci, test_runner Commits 3 - test_runner: preserve original property descriptor - test_runner: add test for checking the property descriptor state - test_runner: refactoring functions Committers 1 - Erick Wendel PR-URL: https://github.com/nodejs/node/pull/49433 Reviewed-By: Benjamin Gruenbaum Reviewed-By: Raz Luvaton Reviewed-By: Chemi Atlow Reviewed-By: Moshe Atlow ------------------------------ Generated metadata ------------------------------ PR-URL: https://github.com/nodejs/node/pull/49433 Reviewed-By: Benjamin Gruenbaum Reviewed-By: Raz Luvaton Reviewed-By: Chemi Atlow Reviewed-By: Moshe Atlow -------------------------------------------------------------------------------- ℹ This PR was created on Fri, 01 Sep 2023 17:13:36 GMT ✔ Approvals: 4 ✔ - Benjamin Gruenbaum (@benjamingr) (TSC): https://github.com/nodejs/node/pull/49433#pullrequestreview-1608049890 ✔ - Raz Luvaton (@rluvaton): https://github.com/nodejs/node/pull/49433#pullrequestreview-1608346449 ✔ - Chemi Atlow (@atlowChemi): https://github.com/nodejs/node/pull/49433#pullrequestreview-1608366919 ✔ - Moshe Atlow (@MoLow) (TSC): https://github.com/nodejs/node/pull/49433#pullrequestreview-1608411583 ✔ Last GitHub CI successful ℹ Last Full PR CI on 2023-09-02T19:22:27Z: https://ci.nodejs.org/job/node-test-pull-request/53694/ - Querying data for job/node-test-pull-request/53694/ ✔ Last Jenkins CI successful -------------------------------------------------------------------------------- ✔ No git cherry-pick in progress ✔ No git am in progress ✔ No git rebase in progress -------------------------------------------------------------------------------- - Bringing origin/main up to date... From https://github.com/nodejs/node * branch main -> FETCH_HEAD ✔ origin/main is now up-to-date - Downloading patch for 49433 From https://github.com/nodejs/node * branch refs/pull/49433/merge -> FETCH_HEAD ✔ Fetched commits as 0add7a8f0c92..64f872a28f86 -------------------------------------------------------------------------------- [main 9ce045d826] test_runner: preserve original property descriptor Author: Erick Wendel Date: Fri Sep 1 14:07:12 2023 -0300 1 file changed, 137 insertions(+), 35 deletions(-) [main 9a7621b25c] test_runner: add test for checking the property descriptor state Author: Erick Wendel Date: Fri Sep 1 15:55:26 2023 -0300 1 file changed, 49 insertions(+) [main 2de5750354] test_runner: refactoring functions Author: Erick Wendel Date: Fri Sep 1 15:59:39 2023 -0300 2 files changed, 162 insertions(+), 141 deletions(-) ✔ Patches applied There are 3 commits in the PR. Attempting autorebase. Rebasing (2/6)https://github.com/nodejs/node/actions/runs/6065810018 |
|
Landed in 8e82cfc |
PR-URL: #49433 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Raz Luvaton <rluvaton@gmail.com> Reviewed-By: Chemi Atlow <chemi@atlow.co.il> Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
PR-URL: nodejs#49433 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Raz Luvaton <rluvaton@gmail.com> Reviewed-By: Chemi Atlow <chemi@atlow.co.il> Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
PR-URL: #49433 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Raz Luvaton <rluvaton@gmail.com> Reviewed-By: Chemi Atlow <chemi@atlow.co.il> Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
PR-URL: nodejs/node#49433 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Raz Luvaton <rluvaton@gmail.com> Reviewed-By: Chemi Atlow <chemi@atlow.co.il> Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
PR-URL: nodejs/node#49433 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Raz Luvaton <rluvaton@gmail.com> Reviewed-By: Chemi Atlow <chemi@atlow.co.il> Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
In this comment, #49397 (comment) @ljharb suggested preserving the behavior and attributes of properties as they were before the modification
cc @nodejs/test_runner