test: BigDecmialByteStringConverter E2E Test by JacobStocklass · Pull Request #979 · googleapis/java-bigquerystorage

JacobStocklass · 2021-03-31T20:20:48Z

Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:

Make sure to open an issue as a bug/issue before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea
Ensure the tests and linter pass
Code coverage does not decrease (if any source code was changed)
Appropriate docs were updated (if necessary)

Fixes #<issue_number_goes_here> ☕️

to support these changes. Fix all affected tests

codecov · 2021-03-31T20:53:54Z

Codecov Report

Merging #979 (bee66d3) into master (235ce1d) will increase coverage by 0.04%.
The diff coverage is 100.00%.

@@             Coverage Diff              @@
##             master     #979      +/-   ##
============================================
+ Coverage     80.99%   81.03%   +0.04%     
- Complexity     1043     1046       +3     
============================================
  Files            78       78              
  Lines          5657     5669      +12     
  Branches        433      436       +3     
============================================
+ Hits           4582     4594      +12     
  Misses          889      889              
  Partials        186      186

Impacted Files	Coverage Δ	Complexity Δ
...torage/v1beta2/BQTableSchemaToProtoDescriptor.java	`98.48% <100.00%> (ø)`	`8.00 <0.00> (ø)`
...d/bigquery/storage/v1beta2/JsonToProtoMessage.java	`97.12% <100.00%> (+0.27%)`	`47.00 <0.00> (+3.00)`
...google/cloud/bigquery/storage/v1alpha2/Waiter.java	`55.95% <0.00%> (ø)`	`12.00% <0.00%> (ø%)`
...e/cloud/bigquery/storage/v1alpha2/WriterCache.java	`94.25% <0.00%> (ø)`	`14.00% <0.00%> (ø%)`
.../cloud/bigquery/storage/v1alpha2/DirectWriter.java	`77.55% <0.00%> (ø)`	`6.00% <0.00%> (ø%)`
.../cloud/bigquery/storage/v1alpha2/StreamWriter.java	`80.39% <0.00%> (ø)`	`34.00% <0.00%> (ø%)`
...oud/bigquery/storage/v1alpha2/JsonWriterCache.java	`91.07% <0.00%> (ø)`	`12.00% <0.00%> (ø%)`
...ud/bigquery/storage/v1alpha2/JsonStreamWriter.java	`77.19% <0.00%> (ø)`	`12.00% <0.00%> (ø%)`
.../bigquery/storage/v1alpha2/JsonToProtoMessage.java	`96.96% <0.00%> (ø)`	`47.00% <0.00%> (ø%)`
...bigquery/storage/v1alpha2/SchemaCompatibility.java	`91.30% <0.00%> (ø)`	`99.00% <0.00%> (ø%)`
... and 3 more

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 235ce1d...bee66d3. Read the comment docs.

...querystorage/src/main/java/com/google/cloud/bigquery/storage/v1beta2/JsonToProtoMessage.java

.../com/google/cloud/bigquery/storage/v1beta2/it/ITBigQueryBigDecimalByteStringEncoderTest.java

...querystorage/src/main/java/com/google/cloud/bigquery/storage/v1beta2/JsonToProtoMessage.java

… Resolving other comments

...querystorage/src/main/java/com/google/cloud/bigquery/storage/v1beta2/JsonToProtoMessage.java

JacobStocklass · 2021-04-02T23:36:48Z

@yirutang PTAL

yirutang

Looks good, some comments here.

...test/java/com/google/cloud/bigquery/storage/v1alpha2/BQTableSchemaToProtoDescriptorTest.java

...querystorage/src/main/java/com/google/cloud/bigquery/storage/v1beta2/JsonToProtoMessage.java

JacobStocklass · 2021-04-05T17:28:12Z

@yirutang PTAL

yirutang · 2021-04-05T18:35:05Z

...ystorage/src/test/java/com/google/cloud/bigquery/storage/v1beta2/JsonToProtoMessageTest.java

          .put(RepeatedObject.getDescriptor(), "object")
          .build();

+  private static ImmutableMap<Descriptor, Message[]> BytesRepeatedCorrectProto =


Could you add this case directly to AllTypesToDebugMessageTest?

I was messing with that for a while but I think your comment below actually kind of solves the issue I was having with it. I just added in the positive test to the AllRepeatedTypesToDebugmessageTest and it works now. I will see if the negative test can also be added in now.

...ystorage/src/test/java/com/google/cloud/bigquery/storage/v1beta2/JsonToProtoMessageTest.java

yirutang · 2021-04-05T18:40:38Z

...ystorage/src/test/java/com/google/cloud/bigquery/storage/v1beta2/JsonToProtoMessageTest.java

+              "Error: root.test_repeated[0] could not be converted to byte[].", e.getMessage());
+        }
+      }
+      if (entry.getKey() == RepeatedBytes.getDescriptor()) {


does this test have 2 cases? I only see one entry in the ByteRepeatedTypesToDebugMessageTest. It is quite hard to read. Could you just add the positive case to the AllTypesToDebugMessageTest and then focus on the negative test here?

I realized that there was a better way to add in both of the positive and negative tests to the existing test. It should be covered now and much easier to understand what changes I made

🤖 I have created a release *beep* *boop* --- ### Updating meta-information for bleeding-edge SNAPSHOT release. --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).

JacobStocklass added 2 commits March 31, 2021 20:15

Add E2E test for BigDecimalByteStringConverter. Fix JsonToProtoMessage

cb3c85f

to support these changes. Fix all affected tests

Cleaning up logs and unnecessary code

24235c0

JacobStocklass requested review from a team and tswast March 31, 2021 20:20

product-auto-label bot added the api: bigquerystorage Issues related to the googleapis/java-bigquerystorage API. label Mar 31, 2021

google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label Mar 31, 2021

JacobStocklass requested review from gnanda and yirutang March 31, 2021 20:21

JacobStocklass added 2 commits March 31, 2021 20:41

Encode numerics in WriteManualClientTest

a6e02af

Lint

dba09f9

yirutang reviewed Mar 31, 2021

View reviewed changes

JacobStocklass added 3 commits March 31, 2021 22:20

Catch errors with JSONArray in JsonToProtoMessage, cover with a test.…

7c93196

… Resolving other comments

Lint

453b4fd

Removing comment

1aee8ed

yirutang reviewed Apr 1, 2021

View reviewed changes

JacobStocklass added 2 commits April 2, 2021 21:40

Add unit test for new Byte changes

9ada94b

Lint

19385c0

yirutang reviewed Apr 2, 2021

View reviewed changes

JacobStocklass added 3 commits April 5, 2021 16:47

Adding unit tests for repeated BYTES field changes

5defe5c

Lint

5e081fa

Fixing test error

ec16438

yirutang reviewed Apr 5, 2021

View reviewed changes

JacobStocklass added 2 commits April 5, 2021 21:10

Moved new tests into existing test for better clarity and readability

cfbfa3a

Cleaning up

bee66d3

yirutang approved these changes Apr 5, 2021

View reviewed changes

stephaniewang526 approved these changes Apr 6, 2021

View reviewed changes

stephaniewang526 merged commit 5c8c4e7 into googleapis:master Apr 6, 2021

Conversation

JacobStocklass commented Mar 31, 2021

Uh oh!

codecov bot commented Mar 31, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

JacobStocklass commented Apr 2, 2021

Uh oh!

yirutang left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

JacobStocklass commented Apr 5, 2021

Uh oh!

yirutang Apr 5, 2021

Choose a reason for hiding this comment

Uh oh!

JacobStocklass Apr 5, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

yirutang Apr 5, 2021

Choose a reason for hiding this comment

Uh oh!

JacobStocklass Apr 5, 2021

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

codecov bot commented Mar 31, 2021 •

edited

Loading