-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Vault test matrix #4698
Vault test matrix #4698
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All my questions can be resolved without a re-review. Looks great! Super excited to greatly expand our test matrix coverage!
if err := os.Chmod(h.binDir, 0700); err != nil { | ||
h.t.Fatalf("failed to chmod: %v", err) | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can all of this be replaced with https://golang.org/pkg/os/#MkdirAll ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
MkdirAll is useful for creating sub directories that don't exist. In this case the parent exists so we are just creating the subdirectory directly so mkdir is fine. The second chmod is b/c it wasn't properly chmoding unless I added the second call 🤷♂️
e2e/vault/vault_test.go
Outdated
|
||
for version, vaultBin := range vaultBinaries { | ||
t.Run(version, func(t *testing.T) { | ||
vbin := vaultBin |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doesn't this have to be the first line in the for loop -- outside the func?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep good catch!
@@ -195,7 +195,7 @@ | |||
{"path":"github.com/hashicorp/raft-boltdb","checksumSHA1":"QAxukkv54/iIvLfsUP6IK4R0m/A=","revision":"d1e82c1ec3f15ee991f7cc7ffd5b67ff6f5bbaee","revisionTime":"2015-02-01T20:08:39Z"}, | |||
{"path":"github.com/hashicorp/serf/coordinate","checksumSHA1":"0PeWsO2aI+2PgVYlYlDPKfzCLEQ=","revision":"80ab48778deee28e4ea2dc4ef1ebb2c5f4063996","revisionTime":"2018-05-07T23:19:28Z"}, | |||
{"path":"github.com/hashicorp/serf/serf","checksumSHA1":"QrT+nzyXsD/MmhTjjhcPdnALZ1I=","revision":"80ab48778deee28e4ea2dc4ef1ebb2c5f4063996","revisionTime":"2018-05-07T23:19:28Z"}, | |||
{"path":"github.com/hashicorp/vault/api","checksumSHA1":"+B4wuJNerIUKNAVzld7CmMaNW5A=","revision":"8575f8fedcf8f5a6eb2b4701cb527b99574b5286","revisionTime":"2018-09-06T17:45:45Z"}, | |||
{"path":"github.com/hashicorp/vault/api","checksumSHA1":"DP7dd8OErZVF0q+XfPo0RGkDcLk=","revision":"6e8d91a59c34bd9f323397c30be9651422295c65","revisionTime":"2018-09-19T17:09:49Z"}, | |||
{"path":"github.com/hashicorp/vault/helper/compressutil","checksumSHA1":"bSdPFOHaTwEvM4PIvn0PZfn75jM=","revision":"8575f8fedcf8f5a6eb2b4701cb527b99574b5286","revisionTime":"2018-09-06T17:45:45Z"}, | |||
{"path":"github.com/hashicorp/vault/helper/consts","checksumSHA1":"QNGGvSYtwk6VCkj4laZPjM2301E=","revision":"8575f8fedcf8f5a6eb2b4701cb527b99574b5286","revisionTime":"2018-09-06T17:45:45Z"}, | |||
{"path":"github.com/hashicorp/vault/helper/hclutil","checksumSHA1":"RlqPBLOexQ0jj6jomhiompWKaUg=","revision":"8575f8fedcf8f5a6eb2b4701cb527b99574b5286","revisionTime":"2018-09-06T17:45:45Z"}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we keep all of the vault packages at the same revision?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not really, the other packages didn't have relevant changes to pull in.
I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions. |
This PR introduces an E2E test to ensure that Nomad works with a matrix of Vault versions