test(storage): init retry conformance test scaffolding #4711
Conversation
BrennaEpp
commented
Aug 31, 2021
BrennaEpp
commented
- adds conformance test scaffolding
- updates conformance test proto
- adds retry test json with scenario 1 tests
product-auto-label
bot
added
the
api: storage
|
All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the ℹ️ Googlers: Go here for more info. |
google-cla
bot
added
the
cla: no
|
All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the ℹ️ Googlers: Go here for more info. |
BrennaEpp
force-pushed
the
conf-scaffolding
branch
from
a1e083e
to
ec4ea20
Compare
google-cla
bot
added
cla: yes
| "PATH_STYLE": 0, | ||
| "VIRTUAL_HOSTED_STYLE": 1, | ||
| "BUCKET_BOUND_HOSTNAME": 2, | ||
| func (x UrlStyle) Enum() *UrlStyle { |
There was a problem hiding this comment.
Curious about these updates in the generated code-- do the signed URL tests still work as-is?
There was a problem hiding this comment.
yes, looks like it! these updates are from the conformance tests repo
storage/conformance_test.go
Outdated
| t.Run(testName, func(t *testing.T) { | ||
|
|
||
| // Create the retry subtest | ||
| subtest := &retrySubtest{T: t, name: testName} |
There was a problem hiding this comment.
"subtest" seems like the wrong word here-- this creates/manages a retry test in the emulator, correct? Any thoughts on a better name?
There was a problem hiding this comment.
I named it that as each t.Run() calls each function it runs a subtest and this struct is used throughout this functio, but yeah there is probably a better name for it.
Maybe something like emulatorRetryTest?
storage/conformance_test.go
Outdated
| rt.id = testRes.TestID | ||
|
|
||
| // Create wrapped client which will send emulator instructions | ||
| rt.host.Path = "" |
There was a problem hiding this comment.
Looks good! Curious is the main purpose of creating a wrappedClient in here to populate the retrySubtest struct?
There was a problem hiding this comment.
The wrappedClient adds the retry test header to each request, prints the request if the test fails and also adds the DevstorageFullControlScope to the https client transport. Correct, we do create it here to populate the struct
BrennaEpp
force-pushed
the
conf-scaffolding
branch
from
ba9ae24
to
c157162
Compare
storage/conformance_test.go
Outdated
| // Create necessary test resources in the emulator | ||
| fs := &resources{} | ||
| for _, resource := range method.Resources { | ||
| if err := fs.populate(ctx, client, resource); err != nil { |
There was a problem hiding this comment.
Should populating the resources be a method on the subtest as well? Not required but it feels slightly cleaner I guess.
