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

Fix corrupt certificate test #205

Closed
wants to merge 1 commit into from

Conversation

rturner3
Copy link
Collaborator

The error message returned by the x509 package in the test case changed with a newer version of Go. Update the test to look for the new error message.

The error message returned by the `x509` package in the test case
changed with a newer version of Go. Update the test to look for the new
error message.

Signed-off-by: Ryan Turner <[email protected]>
azdagron
azdagron previously approved these changes Dec 12, 2022
@azdagron azdagron dismissed their stale review December 12, 2022 21:16

missed something.

@azdagron
Copy link
Member

So, as it stands, this library is set to a go1.13 minimum and that is the version we run our tests with. Independent of the minimum version we support, we should probably be running our tests against each version of go.

In either case, unless we pull up our minimum supported version, this test should still continue to function with go1.13. This might require us to use build tags to change up which error string we test here.

@rturner3
Copy link
Collaborator Author

So, as it stands, this library is set to a go1.13 minimum and that is the version we run our tests with. Independent of the minimum version we support, we should probably be running our tests against each version of go.

In either case, unless we pull up our minimum supported version, this test should still continue to function with go1.13. This might require us to use build tags to change up which error string we test here.

That makes sense. I agree that there is some missing infrastructure before we can really take a change like this. I've filed #206 to track that effort.

I will go ahead and close out this PR for now. I've referenced it in the issue I created so this doesn't get lost.

@rturner3 rturner3 closed this Dec 13, 2022
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.

2 participants