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

cli::tests: program: Clarify all unwrap_err(). #1166

Merged

Conversation

ilya-bobyr
Copy link

unwrap_err() does not contain a descriptive message for the case when it succeeds. It may help someone debugging the test, in particular they will see a short explanation without looking at the failed test.

Also, in a number of cases, unwrap_err() result was not checked, creating a possibility for false positives. As the generated error might be different from the one expected by the test author.


This PR includes #1165, I will rebase it after #1165 is merged.
Please, consider only the second commit.

@ilya-bobyr ilya-bobyr force-pushed the pr/cli-tests-clarify-unwrap_err branch from 682333f to b753b72 Compare May 3, 2024 05:41
@codecov-commenter
Copy link

codecov-commenter commented May 3, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 82.1%. Comparing base (57f92b7) to head (514bac5).
Report is 9 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff            @@
##           master    #1166     +/-   ##
=========================================
- Coverage    82.1%    82.1%   -0.1%     
=========================================
  Files         886      886             
  Lines      236394   236389      -5     
=========================================
- Hits       194205   194180     -25     
- Misses      42189    42209     +20     

`unwrap_err()` does not contain a descriptive message for the case when
it succeeds.  It may help someone debugging the test, in particular they
will see a short explanation without looking at the failed test.

Also, in a number of cases, `unwrap_err()` result was not checked,
creating a possibility for false positives.  As the generated error
might be different from the one expected by the test author.
@ilya-bobyr ilya-bobyr force-pushed the pr/cli-tests-clarify-unwrap_err branch from b753b72 to 514bac5 Compare May 3, 2024 19:00
@ilya-bobyr ilya-bobyr merged commit 18221c0 into anza-xyz:master May 3, 2024
38 checks passed
@ilya-bobyr ilya-bobyr deleted the pr/cli-tests-clarify-unwrap_err branch May 3, 2024 21:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants