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

Accept both old and new cert error strings on MacOS in test assertions #1389

Merged
merged 2 commits into from
Jan 24, 2023

Conversation

cfryanr
Copy link
Member

@cfryanr cfryanr commented Jan 20, 2023

Some unit tests started to fail on MacOS when we started using Go 1.19.5. The Go toolchain on MacOS had a different error message for invalid certs starting in Go 1.18.1, and until it was fixed in Go 1.19.5.

Accept both old and new cert error strings on MacOS in test assertions. Adjust the tests so that they don't care which error message is returned when they are making assertions about error messages.

Also used this as an opportunity to refactor how some tests were making assertions about error strings. New test helpers make it easy for an error string to be expected as an exact string, as a string built using sprintf, as a regexp, or as a string built to include the platform-specific x509 error string. All of these test helpers can be used in a single wantErr field of a test table. They can be used for both unit tests and integration tests.

Release note:

NONE

Used this as an opportunity to refactor how some tests were
making assertions about error strings.

New test helpers make it easy for an error string to be expected as an
exact string, as a string built using sprintf, as a regexp, or as a
string built to include the platform-specific x509 error string.

All of these helpers can be used in a single `wantErr` field of a test
table. They can be used for both unit tests and integration tests.

Co-authored-by: Benjamin A. Petersen <ben@benjaminapetersen.me>
@codecov
Copy link

codecov bot commented Jan 20, 2023

Codecov Report

Merging #1389 (bd9d6fa) into main (5756c56) will decrease coverage by 0.14%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##             main    #1389      +/-   ##
==========================================
- Coverage   75.49%   75.36%   -0.14%     
==========================================
  Files         167      166       -1     
  Lines       14857    14884      +27     
==========================================
  Hits        11217    11217              
- Misses       3350     3377      +27     
  Partials      290      290              
Impacted Files Coverage Δ
internal/testutil/assertions.go 0.00% <0.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@joshuatcasey
Copy link
Member

Merging, check concourse-ci/integration-test-1.17 is actually green with concourse job https://hush-house.pivotal.io/teams/tanzu-user-auth/pipelines/pinniped-pull-requests/jobs/integration-test-1.17/builds/1285

@joshuatcasey joshuatcasey merged commit d2afdfa into main Jan 24, 2023
@joshuatcasey joshuatcasey deleted the error_assertions branch January 24, 2023 21:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants