Storage: Update conformance tests by BenWhitehead · Pull Request #5901 · googleapis/google-cloud-java

BenWhitehead · 2019-07-26T20:04:36Z

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.

* 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 · 2019-07-27T03:25:44Z

Codecov Report

Merging #5901 into master will decrease coverage by <.01%.
The diff coverage is n/a.

@@ 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

Impacted Files	Coverage Δ	Complexity Δ
.../java/com/google/cloud/testing/CommandWrapper.java	`87.87% <0%> (-9.1%)`	`13% <0%> (ø)`
.../cloud/datastore/testing/LocalDatastoreHelper.java	`80.59% <0%> (-4.48%)`	`17% <0%> (ø)`
...ain/java/com/google/cloud/pubsub/v1/Publisher.java	`89.54% <0%> (-0.35%)`	`40% <0%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b212217...004cfcb. Read the comment docs.

codecov · 2019-07-27T03:25:44Z

Codecov Report

Merging #5901 into master will decrease coverage by <.01%.
The diff coverage is n/a.

@@ 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

Impacted Files	Coverage Δ	Complexity Δ
.../java/com/google/cloud/testing/CommandWrapper.java	`87.87% <0%> (-9.1%)`	`13% <0%> (ø)`
.../cloud/datastore/testing/LocalDatastoreHelper.java	`80.59% <0%> (-4.48%)`	`17% <0%> (ø)`
...ain/java/com/google/cloud/pubsub/v1/Publisher.java	`89.54% <0%> (-0.35%)`	`40% <0%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b212217...004cfcb. Read the comment docs.

codecov · 2019-07-27T03:25:45Z

Codecov Report

Merging #5901 into master will decrease coverage by <.01%.
The diff coverage is n/a.

@@ 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

Impacted Files	Coverage Δ	Complexity Δ
.../java/com/google/cloud/testing/CommandWrapper.java	`87.87% <0%> (-9.1%)`	`13% <0%> (ø)`
.../cloud/datastore/testing/LocalDatastoreHelper.java	`80.59% <0%> (-4.48%)`	`17% <0%> (ø)`
...ain/java/com/google/cloud/pubsub/v1/Publisher.java	`89.54% <0%> (-0.35%)`	`40% <0%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b212217...004cfcb. Read the comment docs.

codecov · 2019-07-27T03:25:45Z

Codecov Report

Merging #5901 into master will decrease coverage by <.01%.
The diff coverage is n/a.

@@ 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

Impacted Files	Coverage Δ	Complexity Δ
.../java/com/google/cloud/testing/CommandWrapper.java	`87.87% <0%> (-9.1%)`	`13% <0%> (ø)`
.../cloud/datastore/testing/LocalDatastoreHelper.java	`80.59% <0%> (-4.48%)`	`17% <0%> (ø)`
...ain/java/com/google/cloud/pubsub/v1/Publisher.java	`89.54% <0%> (-0.35%)`	`40% <0%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b212217...004cfcb. Read the comment docs.

frankyn · 2019-07-31T22:46:53Z

+ public V4SigningTest(
+ SigningV4Test testData,
+ ServiceAccountCredentials serviceAccountCredentials,
+ @SuppressWarnings("unused") String description) {


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.

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:
<testcasename="test[Simple GET]"classname="com.google.cloud.storage.V4SigningTest(sponge_log)"time="0.04"/> <testcasename="test[Simple PUT]"classname="com.google.cloud.storage.V4SigningTest(sponge_log)"time="0.015"/> <testcasename="test[POST for resumable uploads]"classname="com.google.cloud.storage.V4SigningTest(sponge_log)"time="0.006"/> <testcasename="test[Vary expiration and timestamp]"classname="com.google.cloud.storage.V4SigningTest(sponge_log)"time="0.005"/> <testcasename="test[Vary bucket and object]"classname="com.google.cloud.storage.V4SigningTest(sponge_log)"time="0.005"/> <testcasename="test[Simple headers]"classname="com.google.cloud.storage.V4SigningTest(sponge_log)"time="0.006"/> <testcasename="test[Headers should be trimmed]"classname="com.google.cloud.storage.V4SigningTest(sponge_log)"time="0"> <skippedmessage="Test skipped until b/136171758 is resolved: got: "test[Headers should be trimmed]", expected: is not "test[Headers should be trimmed]""/> </testcase> <testcasename="test[Header value with multiple inline values]"classname="com.google.cloud.storage.V4SigningTest(sponge_log)"time="0.004"/> <testcasename="test[Customer-supplied encryption key]"classname="com.google.cloud.storage.V4SigningTest(sponge_log)"time="0.005"/> <testcasename="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())").

Thanks for clarifying.

frankyn

LGTM, just have that one nit.

BenWhitehead · 2019-08-01T17:55:08Z

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.

* Update BigTable conformance tests (googleapis#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.

BenWhitehead added api: storage Issues related to the Cloud Storage API. type: cleanup An internal cleanup or hygiene concern. labels Jul 26, 2019

BenWhitehead requested review from JesseLovelace and frankyn July 26, 2019 20:04

BenWhitehead self-assigned this Jul 26, 2019

googlebot added the cla: yes This human has signed the Contributor License Agreement. label Jul 26, 2019

chingor13 changed the title ~~Update Storage conformance tests~~Storage: Update conformance testsJul 30, 2019

frankyn reviewed Jul 31, 2019
View reviewed changes

frankyn approved these changes Aug 1, 2019
View reviewed changes

BenWhitehead merged commit d8d9742 into googleapis:masterAug 1, 2019

BenWhitehead deleted the storage-conformance-test-update branch August 1, 2019 18:42

chingor13 mentioned this pull request Aug 7, 2019
Release google-cloud-java v0.104.0 #6048
Merged

kolea2 mentioned this pull request Aug 28, 2019
Release google-cloud-java v0.107.0 #6188
Merged

Navigation Menu

Uh oh!

Storage: Update conformance tests - #5901

Storage: Update conformance tests#5901
BenWhitehead merged 1 commit into
googleapis:masterfrom
BenWhitehead:storage-conformance-test-update

BenWhitehead commented Jul 26, 2019•
edited
Loading

Uh oh!

codecovBot commented Jul 27, 2019

Uh oh!

codecovBot commented Jul 27, 2019

Uh oh!

codecovBot commented Jul 27, 2019

Uh oh!

codecovBot commented Jul 27, 2019•
edited
Loading

Uh oh!

frankynJul 31, 2019

Uh oh!

BenWhiteheadAug 1, 2019

Uh oh!

frankynAug 1, 2019

Uh oh!

frankyn left a comment

Uh oh!

BenWhitehead commented Aug 1, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

BenWhitehead commented Jul 26, 2019• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecovBot commented Jul 27, 2019

Codecov Report

Uh oh!

codecovBot commented Jul 27, 2019

Codecov Report

Uh oh!

codecovBot commented Jul 27, 2019

Codecov Report

Uh oh!

codecovBot commented Jul 27, 2019• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

frankynJul 31, 2019

Choose a reason for hiding this comment

Uh oh!

BenWhiteheadAug 1, 2019

Choose a reason for hiding this comment

Uh oh!

frankynAug 1, 2019

Choose a reason for hiding this comment

Uh oh!

frankyn left a comment

Choose a reason for hiding this comment

Uh oh!

BenWhitehead commented Aug 1, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

BenWhitehead commented Jul 26, 2019•
edited
Loading

codecovBot commented Jul 27, 2019•
edited
Loading