sinonjs / sinon

Test spies, stubs and mocks for JavaScript.
https://sinonjs.org/
Other
9.63k stars 769 forks source link

fix: assertion log limit #2485

Closed sgoossens closed 10 months ago

sgoossens commented 1 year ago

Purpose (TL;DR) - mandatory

Allow for a log limit setting on the assertion object, that when true, will truncate the error.message that is returned via a failing assertion. The limit for the log message will be set to a default if none was provided

https://github.com/sinonjs/sinon/issues/2484

Background (Problem in detail) - optional

When a stubbed method has a large property on it, it is logged in the failing assertion causing runoff in the terminal and making it hard to see what assertion actually failed.

By allowing for a log limitation setting and a default log limit, error logs can be truncated.

How to verify - mandatory

  1. use this app and point the sinon instance to this version
  2. see the logs are reduced when the setting is on

Checklist for author

fatso83 commented 1 year ago

Would you be interested in help getting it over the finish line? Have some time these days.

sgoossens commented 1 year ago

Would you be interested in help getting it over the finish line? Have some time these days.

Sure that would be great. I lost track of this but think it would be a great add. I can also put some time in to addressing your feedback as well

codecov[bot] commented 10 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Comparison is base (8dbfd02) 96.03% compared to head (7701afc) 96.05%. Report is 4 commits behind head on main.

:exclamation: Current head 7701afc differs from pull request most recent head ef5c456. Consider uploading reports for the commit ef5c456 to get more accurate results

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #2485 +/- ## ========================================== + Coverage 96.03% 96.05% +0.02% ========================================== Files 40 41 +1 Lines 1915 1928 +13 ========================================== + Hits 1839 1852 +13 Misses 76 76 ``` | [Flag](https://app.codecov.io/gh/sinonjs/sinon/pull/2485/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=sinonjs) | Coverage Δ | | |---|---|---| | [unit](https://app.codecov.io/gh/sinonjs/sinon/pull/2485/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=sinonjs) | `96.05% <100.00%> (+0.02%)` | :arrow_up: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=sinonjs#carryforward-flags-in-the-pull-request-comment) to find out more. | [Files](https://app.codecov.io/gh/sinonjs/sinon/pull/2485?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=sinonjs) | Coverage Δ | | |---|---|---| | [lib/create-sinon-api.js](https://app.codecov.io/gh/sinonjs/sinon/pull/2485?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=sinonjs#diff-bGliL2NyZWF0ZS1zaW5vbi1hcGkuanM=) | `100.00% <100.00%> (ø)` | | | [lib/sinon.js](https://app.codecov.io/gh/sinonjs/sinon/pull/2485?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=sinonjs#diff-bGliL3Npbm9uLmpz) | `100.00% <100.00%> (ø)` | | | [lib/sinon/assert.js](https://app.codecov.io/gh/sinonjs/sinon/pull/2485?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=sinonjs#diff-bGliL3Npbm9uL2Fzc2VydC5qcw==) | `100.00% <100.00%> (ø)` | | | [lib/sinon/colorizer.js](https://app.codecov.io/gh/sinonjs/sinon/pull/2485?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=sinonjs#diff-bGliL3Npbm9uL2NvbG9yaXplci5qcw==) | `100.00% <ø> (ø)` | | | [lib/sinon/create-sandbox.js](https://app.codecov.io/gh/sinonjs/sinon/pull/2485?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=sinonjs#diff-bGliL3Npbm9uL2NyZWF0ZS1zYW5kYm94Lmpz) | `100.00% <100.00%> (ø)` | | | [lib/sinon/sandbox.js](https://app.codecov.io/gh/sinonjs/sinon/pull/2485?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=sinonjs#diff-bGliL3Npbm9uL3NhbmRib3guanM=) | `97.90% <100.00%> (+<0.01%)` | :arrow_up: | | [lib/sinon/spy-formatters.js](https://app.codecov.io/gh/sinonjs/sinon/pull/2485?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=sinonjs#diff-bGliL3Npbm9uL3NweS1mb3JtYXR0ZXJzLmpz) | `98.55% <100.00%> (ø)` | |

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

fatso83 commented 10 months ago

OK, so I added test coverage, fixed up some logical errors, exposed the options on the sandbox creation, etc.

This is ready for merge from my perspective, but it does not change the default, as that would be a breaking change, but we can do that with version 18 . It still allows creating sandboxes with log limits, which is probably what you want right now.

The way I stubbed out

fatso83 commented 10 months ago

@sgoossens You ok with this? Anything to add/change?