Skip to content
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

Support sentinel-based ranges in default stringify #2004

Merged
merged 4 commits into from
Aug 18, 2020

Conversation

seanmiddleditch
Copy link
Contributor

Fix for issue #2003

  • Enhances rangeToString to work with a separate iterator and sentinel type
  • Adds a test to ensure this compiles

@horenmar
Copy link
Member

Tests in default run cannot fail, or it will fail CI. Instead, you should use ::Catch::Detail::stringify in the test and check that the return value matches expected stringification.

@seanmiddleditch
Copy link
Contributor Author

seanmiddleditch commented Aug 14, 2020

Ah, gotcha. Easy enough to fix, thanks.

@seanmiddleditch
Copy link
Contributor Author

@horenmar - I'm not familiar with the approval test stuff here; should I be checking in the files that are modified locally on build?

@horenmar
Copy link
Member

On a second look, this part seems to be missing from the documentation after the rewrite. 🤔

Approval tests diff known good output with current output. If there are changes, check that the diff makes sense, run scripts/approve.py and commit the updated baselines.

@codecov
Copy link

codecov bot commented Aug 16, 2020

Codecov Report

Merging #2004 into master will not change coverage.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #2004   +/-   ##
=======================================
  Coverage   88.76%   88.76%           
=======================================
  Files         138      138           
  Lines        5648     5648           
=======================================
  Hits         5013     5013           
  Misses        635      635           

@horenmar
Copy link
Member

Looks good now, thanks.

@horenmar horenmar merged commit 284672c into catchorg:master Aug 18, 2020
@seanmiddleditch seanmiddleditch deleted the sentinels branch August 18, 2020 20:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants