-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Storage: Update conformance tests #5901
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
Storage: Update conformance tests #5901
Conversation
* Refactor V4SigningTest to use JUnits Parameterized Test runner.
This allows running each of the tests as an individual test thereby
providing success/ignore/failure information per test.
* Refactor V4SigningTest to use the generated Java Protobuf classes
from the google-cloud-conformance-test module. The format read from is
still json.
* Update to use the latest revision of the conformance-tests data.
* One of the updated tests `Headers should be trimmed` does not
currently pass and has been marked as ignore until there is a
resolution to b/136171758 relating to the specification of how
whitespace is to be handled when signing URLs.
* I choose to move forward with the update, to reduce the drift
between the test definitions in conformance-tests where other
automation is being introduced to hopefully reduce the maintenance
burden of conformance tests.
Codecov Report
@@ Coverage Diff @@
## master #5901 +/- ##
============================================
- Coverage 46.79% 46.79% -0.01%
- Complexity 25648 25663 +15
============================================
Files 2456 2456
Lines 267593 267593
Branches 30564 30559 -5
============================================
- Hits 125229 125222 -7
- Misses 133105 133112 +7
Partials 9259 9259
Continue to review full report at Codecov.
|
3 similar comments
Codecov Report
@@ Coverage Diff @@
## master #5901 +/- ##
============================================
- Coverage 46.79% 46.79% -0.01%
- Complexity 25648 25663 +15
============================================
Files 2456 2456
Lines 267593 267593
Branches 30564 30559 -5
============================================
- Hits 125229 125222 -7
- Misses 133105 133112 +7
Partials 9259 9259
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## master #5901 +/- ##
============================================
- Coverage 46.79% 46.79% -0.01%
- Complexity 25648 25663 +15
============================================
Files 2456 2456
Lines 267593 267593
Branches 30564 30559 -5
============================================
- Hits 125229 125222 -7
- Misses 133105 133112 +7
Partials 9259 9259
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## master #5901 +/- ##
============================================
- Coverage 46.79% 46.79% -0.01%
- Complexity 25648 25663 +15
============================================
Files 2456 2456
Lines 267593 267593
Branches 30564 30559 -5
============================================
- Hits 125229 125222 -7
- Misses 133105 133112 +7
Partials 9259 9259
Continue to review full report at Codecov.
|
| public V4SigningTest( | ||
| SigningV4Test testData, | ||
| ServiceAccountCredentials serviceAccountCredentials, | ||
| @SuppressWarnings("unused") String description) { |
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.
It would be helpful to log out the name of the test being ran. Otherwise remove description given it's Test code and shouldn't impact the library.
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.
The test name is written into the test results for the test suite, and is referenced by the @Parameters(name = "{2}") here: https://github.com/googleapis/google-cloud-java/pull/5901/files#diff-265f00ab3c7dd6cdaf7e8a7392158e52R132
In the JUnit.xml picked up by the build server it is rendered like this:
<testcase name="test[Simple GET]" classname="com.google.cloud.storage.V4SigningTest(sponge_log)" time="0.04"/>
<testcase name="test[Simple PUT]" classname="com.google.cloud.storage.V4SigningTest(sponge_log)" time="0.015"/>
<testcase name="test[POST for resumable uploads]" classname="com.google.cloud.storage.V4SigningTest(sponge_log)" time="0.006"/>
<testcase name="test[Vary expiration and timestamp]" classname="com.google.cloud.storage.V4SigningTest(sponge_log)" time="0.005"/>
<testcase name="test[Vary bucket and object]" classname="com.google.cloud.storage.V4SigningTest(sponge_log)" time="0.005"/>
<testcase name="test[Simple headers]" classname="com.google.cloud.storage.V4SigningTest(sponge_log)" time="0.006"/>
<testcase name="test[Headers should be trimmed]" classname="com.google.cloud.storage.V4SigningTest(sponge_log)" time="0">
<skipped message="Test skipped until b/136171758 is resolved: got: "test[Headers should be trimmed]", expected: is not "test[Headers should be trimmed]""/>
</testcase>
<testcase name="test[Header value with multiple inline values]" classname="com.google.cloud.storage.V4SigningTest(sponge_log)" time="0.004"/>
<testcase name="test[Customer-supplied encryption key]" classname="com.google.cloud.storage.V4SigningTest(sponge_log)" time="0.005"/>
<testcase name="test[List Objects]" classname="com.google.cloud.storage.V4SigningTest(sponge_log)" time="0.004"/>In IntelliJ it renders like this:

Unfortunately, when using the Parameterized runner the number of properties passed to the constructor has to match exactly with the number of values provided. And in the @Parameters only allows a value that will have .toString called on it, it doesn't allow us to do something like @Parameters(name = "{1}.getDescription())").
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.
Thanks for clarifying.
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.
LGTM, just have that one nit.
|
Thanks for the review @frankyn! |
* Update BigTable conformance tests (#5901) * Refactor ReadRowsMergingAcceptanceTest to use the generated Java Protobuf classes from the google-cloud-conformance-test module. The format read from is still json. * Update to use the latest revision of the conformance-tests data.
This allows running each of the tests as an individual test thereby
providing success/ignore/failure information per test.
from the google-cloud-conformance-test module. The format read from is
still json.
Headers should be trimmeddoes notcurrently pass and has been marked as ignore until there is a
resolution to b/136171758 relating to the specification of how
whitespace is to be handled when signing URLs.
between the test definitions in conformance-tests where other
automation is being introduced to hopefully reduce the maintenance
burden of conformance tests.