-
Notifications
You must be signed in to change notification settings - Fork 29.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
assert: callTracker throw a specific error message when possible #43640
assert: callTracker throw a specific error message when possible #43640
Conversation
3d6fcbb
to
0b3b3cd
Compare
@nodejs/test_runner @nodejs/assert |
0b3b3cd
to
2449674
Compare
2449674
to
f878298
Compare
@MoLow FYI force-pushing to your branch makes reviewing harder as GitHub UI can't show the diff since last review, so reviewers basically have to re-review the whole PR. Not a very big deal, but I wanted to let you know in case you didn't know – and of course sometimes force-pushes are necessary, e.g. when there's a git conflict to fix. |
I do see the downside, however, I try to keep up to date with main since more than once that was the only way to get fixes to tests failing the CI. |
Commit Queue failed- Loading data for nodejs/node/pull/43640 ✔ Done loading data for nodejs/node/pull/43640 ----------------------------------- PR info ------------------------------------ Title assert: callTracker throw a specific error message when possible (#43640) Author Moshe Atlow (@MoLow) Branch MoLow:assert-specific-error-message -> nodejs:main Labels assert, author ready, needs-ci Commits 3 - assert: callTracker throw a specific error message when possible - CR - CR Committers 1 - Moshe Atlow PR-URL: https://github.com/nodejs/node/pull/43640 Reviewed-By: Antoine du Hamel Reviewed-By: Nitzan Uziely Reviewed-By: Benjamin Gruenbaum ------------------------------ Generated metadata ------------------------------ PR-URL: https://github.com/nodejs/node/pull/43640 Reviewed-By: Antoine du Hamel Reviewed-By: Nitzan Uziely Reviewed-By: Benjamin Gruenbaum -------------------------------------------------------------------------------- ℹ This PR was created on Fri, 01 Jul 2022 07:39:43 GMT ✔ Approvals: 3 ✔ - Antoine du Hamel (@aduh95) (TSC): https://github.com/nodejs/node/pull/43640#pullrequestreview-1028301103 ✔ - Nitzan Uziely (@linkgoron): https://github.com/nodejs/node/pull/43640#pullrequestreview-1031952209 ✔ - Benjamin Gruenbaum (@benjamingr): https://github.com/nodejs/node/pull/43640#pullrequestreview-1032124384 ✔ Last GitHub CI successful ℹ Last Full PR CI on 2022-07-08T07:51:08Z: https://ci.nodejs.org/job/node-test-pull-request/45149/ - Querying data for job/node-test-pull-request/45149/ ✔ 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 43640 From https://github.com/nodejs/node * branch refs/pull/43640/merge -> FETCH_HEAD ✔ Fetched commits as f3a92a0572b1..f87829815671 -------------------------------------------------------------------------------- [main 056d3eda99] assert: callTracker throw a specific error message when possible Author: Moshe Atlow Date: Fri Jul 1 10:32:10 2022 +0300 2 files changed, 32 insertions(+), 12 deletions(-) [main a84f04a6a7] CR Author: Moshe Atlow Date: Tue Jul 5 00:11:23 2022 +0300 2 files changed, 5 insertions(+), 6 deletions(-) [main 7baf7e12e7] CR Author: Moshe Atlow Date: Tue Jul 5 11:58:52 2022 +0300 1 file changed, 8 insertions(+), 2 deletions(-) ✔ Patches applied There are 3 commits in the PR. Attempting autorebase. Rebasing (2/6)https://github.com/nodejs/node/actions/runs/2635335874 |
Landed in 458c4fb |
PR-URL: #43640 Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Nitzan Uziely <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]>
PR-URL: #43640 Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Nitzan Uziely <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]>
PR-URL: #43640 Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Nitzan Uziely <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]>
PR-URL: nodejs/node#43640 Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Nitzan Uziely <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]>
currently
callTracker.verify()
throws a very generic error message when a tracked function isn't called the expected amount of times.this change propeses to throw a more specific message in case there was a single-tracked function that did not match the expected call counts