build: bump golang 1.19.4 -> 1.19.5 #5557
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oh, Might have to wait a few hours for the DockerHub base image for golang to be updated to 1.19.5 |
|
rebased now. but there is another err in ci: shows [Go Test (darwin)] ci error .. Is there any hint from the pr in helm, helm Fixing x509 test on darwin |
|
Looks like a test we need to update. I'm not sure which of these changes in 1.19.5 is the culprit, maybe golang/go#57427? 🤔 |
|
I test it in my local mac pro. the code source is : function defined is : @srenatus |
|
Can't fully engage right now, sorry. But your error looks different from the one happening in CI. |
|
OK , I will continue to look it in my spare time.. |
|
It looks like in this run, the |
|
Oh.Sorry Maybe i have make someting wrong. . Its fine in local environment. |
|
The ci test succeed.... |
topdown/http_test.go
Outdated
| note string | ||
| rules []string | ||
| expected interface{} | ||
| //expectedError error |
There was a problem hiding this comment.
expectedError seems like it wasn't used anyways.
There was a problem hiding this comment.
yes, it is not used.
the go-lint will error if it is not used any place.
There was a problem hiding this comment.
Ok, let's remove it then.
There was a problem hiding this comment.
OK, rebase it later.
topdown/http_test.go
Outdated
| tests = append(tests, httpsStruct{note: "http.send", rules: []string{fmt.Sprintf( | ||
| `p = x { http.send({"method": "get", "url": "%s", "force_json_decode": true, "tls_insecure_skip_verify": true, "tls_use_system_certs": true,}, resp); x := clean_headers(resp) }`, ts.URL)}, expected: resultObj.String()}) | ||
|
|
||
| if runtime.GOOS == "darwin" { |
There was a problem hiding this comment.
code is keep the same with before.
but the "darwin" case special proces.
There was a problem hiding this comment.
Thanks. Can you please clarify why the special case is needed?
There was a problem hiding this comment.
crypto/x509: Verify on macOS and crypto/x509: Verify on macOS may the changed in 1.19.5.
There was a problem hiding this comment.
Let's adapt the code so that the previous test still works with macos and 1.19.5. It's OK to adjust the error message (like with fixupDarwinGo118), but this shouldn't be a "no error" vs "some error" case.
There was a problem hiding this comment.
Sorry, I'll have another look tomorrow! 👀
There was a problem hiding this comment.
So something that used to be an error is no longer an error, but only on macos+go1.19.5? That's quite odd. But yeah, let's do something about it -- I'm not too worried because I think few people run OPA as server on macos.
That said, the code here doesn't look right: if runtime.GOOS != "darwin", the fixupDarwinGo118 doesn't make sense in the other branch. 🤔
There was a problem hiding this comment.
@srenatus
thanks for spending time to invest it..
So , We skip the macos test?
Delete a test case in 'fixupDarwinGo118'?
Is that ok?
There was a problem hiding this comment.
Anything that makes the tests pass 😅
There was a problem hiding this comment.
Just skip the macos test fixupDarwinGo118
😅😅
Signed-off-by: yanggang <gang.yang@daocloud.io>
| // present. | ||
| tests = append(tests, httpsStruct{note: "http.send", rules: []string{fmt.Sprintf( | ||
| `p = x { http.send({"method": "get", "url": "%s", "force_json_decode": true, "tls_insecure_skip_verify": true, "tls_use_system_certs": true,}, resp); x := clean_headers(resp) }`, ts.URL)}, expected: resultObj.String()}) | ||
|
|
There was a problem hiding this comment.
Instead of removing the test case can't we just not use fixupDarwinGo118 ie.
&Error{Message: "x509: certificate signed by unknown authority"}Will this change cause the test to pass?
There was a problem hiding this comment.
But then this would not work with older versions.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Signed-off-by: yanggang gang.yang@daocloud.io
release go1.19.5)