test: refactor of zlib-from-gzip-with-trailing-garbage - #10674
Conversation
Sorry, something went wrong.
There was a problem hiding this comment.
it would likely be worthwhile validating the details of the error reported
Sorry, something went wrong.
There was a problem hiding this comment.
I would also recommend restoring result as an argument and adding an assertion for it. So all together, maybe something like this?:
assert(err instanceof Error);
assert.strictEqual(err.code, 'Z_DATA_ERROR');
assert.strictEqual(err.message, 'unknown compression method');
assert.strictEqual(result, undefined);
Sorry, something went wrong.
There was a problem hiding this comment.
New commit pushed up with these suggestions included. Thanks to the both of you for the help. Long time listener, first time committer. 😄
Sorry, something went wrong.
jasnell
left a comment
There was a problem hiding this comment.
LGTM with one suggestion
Sorry, something went wrong.
There was a problem hiding this comment.
Same suggestions here as above. Restore result, add an assertion to confirm it is undefined, and do some more rigorous checking on err.
Sorry, something went wrong.
* use assert.strictEqual instead of assert.equal * add RegExp in second argument of assert.throws * removed unused arguments
* restore result args and validate response * validate error message and code
Sorry, something went wrong.
|
@jasnell I see one test failed... any suggestion on how to fix this? |
Sorry, something went wrong.
|
@leftynaut test/arm is marked as failed but it actually passed, you can see this if you click on details. CI is 100% green 🎉 . |
Sorry, something went wrong.
Bug here: nodejs/github-bot#108 |
Sorry, something went wrong.
* use assert.strictEqual instead of assert.equal * add RegExp in second argument of assert.throws * validate error message and code PR-URL: nodejs#10674 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
|
Landed in 39c4af5. |
Sorry, something went wrong.
* use assert.strictEqual instead of assert.equal * add RegExp in second argument of assert.throws * validate error message and code PR-URL: nodejs#10674 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
* use assert.strictEqual instead of assert.equal * add RegExp in second argument of assert.throws * validate error message and code PR-URL: nodejs#10674 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
* use assert.strictEqual instead of assert.equal * add RegExp in second argument of assert.throws * validate error message and code PR-URL: nodejs#10674 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
* use assert.strictEqual instead of assert.equal * add RegExp in second argument of assert.throws * validate error message and code PR-URL: nodejs#10674 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
* use assert.strictEqual instead of assert.equal * add RegExp in second argument of assert.throws * validate error message and code PR-URL: #10674 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
* use assert.strictEqual instead of assert.equal * add RegExp in second argument of assert.throws * validate error message and code PR-URL: #10674 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Checklist