-
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
test: use conventional argument order in assertion #40591
test: use conventional argument order in assertion #40591
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Commit Queue failed- Loading data for nodejs/node/pull/40591 ✔ Done loading data for nodejs/node/pull/40591 ----------------------------------- PR info ------------------------------------ Title test: use conventional argument order in assertion (#40591) ⚠ Could not retrieve the email or name of the PR author's from user's GitHub profile! Branch tniessen:test-buffer-fill-actual-expected-order -> nodejs:master Labels test, author ready Commits 1 - test: use conventional argument order in assertion Committers 1 - Tobias Nießen PR-URL: https://github.com/nodejs/node/pull/40591 Reviewed-By: Colin Ihrig Reviewed-By: Voltrex Reviewed-By: Rich Trott Reviewed-By: Michaël Zasso Reviewed-By: Luigi Pinca Reviewed-By: Trivikram Kamat Reviewed-By: Ruben Bridgewater Reviewed-By: James M Snell ------------------------------ Generated metadata ------------------------------ PR-URL: https://github.com/nodejs/node/pull/40591 Reviewed-By: Colin Ihrig Reviewed-By: Voltrex Reviewed-By: Rich Trott Reviewed-By: Michaël Zasso Reviewed-By: Luigi Pinca Reviewed-By: Trivikram Kamat Reviewed-By: Ruben Bridgewater Reviewed-By: James M Snell -------------------------------------------------------------------------------- ℹ This PR was created on Sun, 24 Oct 2021 21:58:40 GMT ✔ Approvals: 8 ✔ - Colin Ihrig (@cjihrig) (TSC): https://github.com/nodejs/node/pull/40591#pullrequestreview-787557451 ✔ - Voltrex (@VoltrexMaster): https://github.com/nodejs/node/pull/40591#pullrequestreview-787572367 ✔ - Rich Trott (@Trott) (TSC): https://github.com/nodejs/node/pull/40591#pullrequestreview-787614687 ✔ - Michaël Zasso (@targos) (TSC): https://github.com/nodejs/node/pull/40591#pullrequestreview-787647207 ✔ - Luigi Pinca (@lpinca): https://github.com/nodejs/node/pull/40591#pullrequestreview-787657637 ✔ - Trivikram Kamat (@trivikr): https://github.com/nodejs/node/pull/40591#pullrequestreview-789555558 ✔ - Ruben Bridgewater (@BridgeAR) (TSC): https://github.com/nodejs/node/pull/40591#pullrequestreview-791762513 ✔ - James M Snell (@jasnell) (TSC): https://github.com/nodejs/node/pull/40591#pullrequestreview-791932157 ✔ Last GitHub Actions successful ℹ Last Full PR CI on 2021-10-28T14:13:33Z: https://ci.nodejs.org/job/node-test-pull-request/40528/ - Querying data for job/node-test-pull-request/40528/ ✔ Build data downloaded - Querying failures of job/node-test-commit/49509/ ✔ Data downloaded ✖ 2 failure(s) on the last Jenkins CI run -------------------------------------------------------------------------------- ✔ Aborted `git node land` session in /home/runner/work/node/node/.ncuhttps://github.com/nodejs/node/actions/runs/1395092374 |
PR-URL: #40591 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Voltrex <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Trivikram Kamat <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: James M Snell <[email protected]>
Landed in f7209ee |
@VoltrexMaster Please in the future do not land PRs with failing CI. In this particular case it's not worth reverting the land. |
Sorry, sometimes we land PRs without them fully passing the CI after many tries, not entirely sure if there was any problem landing this particular PR in this case, but will look forward to them fully passing it in the future. 😅 |
We should not be. The collaborator guide specifically calls out:
If the CI's are constantly failing this needs to be raised as issues, investigated and addressed (at worst if it's failing tests they can marked flaky or a known issue). |
PR-URL: #40591 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Voltrex <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Trivikram Kamat <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: James M Snell <[email protected]>
PR-URL: #40591 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Voltrex <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Trivikram Kamat <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: James M Snell <[email protected]>
By convention (and per the names of the arguments), the expected value should be the second argument.