11679 BigQuery-Denormalized Destination: improve code coverage #17827

suhomud · 2022-10-11T10:25:28Z

What

Improve code coverage for BigQuery-Denormalized Destination unit test
Edited
The initial task was to improve the code coverage for unit tests however with existing structure of the unit test for BigQuery-Denormalized Destination it was hard to achieve. In most cases our unit tests for connectors are really similar to integration test.
NOTE:

There is no logic change for the connector but only refactoring for code duplication and clean up for unused code
The integration tests are not changed

How

The idea of this refactoring is to have a separate unit test class for each implementation class which needs to be tested. If unit tests duplicates implementation structure we have some benefits like access to protected methods and support, extension and readability of the unit tests.

Change list explanation

Unused method here and here were removed
Code duplication here and here. This functionality is moved to FormatterUtil class
TestBigQueryDenormalizedRecordFormatter and TestGcsBigQueryDenormalizedRecordFormatter classes used only to make protected void addAirbyteColumns(final ObjectNode data, final AirbyteRecordMessage recordMessage) method as public. After refactoring protected method can be accessed from unit test
The previous test BigQueryDenormalizedUtilsTest was not changed but splited to DefaultBigQueryDenormalizedRecordFormatterTest and GcsBigQueryDenormalizedRecordFormatterTest
new unit tests structure:

github-actions · 2022-10-11T10:26:17Z

NOTE ⚠️ Changes in this PR affect the following connectors. Make sure to run corresponding integration tests:

destination-bigquery-denormalized

suhomud · 2022-10-11T10:26:55Z

/test connector=connectors/destination-bigquery-denormalized

🕑 connectors/destination-bigquery-denormalized https://github.com/airbytehq/airbyte/actions/runs/3226070055
✅ connectors/destination-bigquery-denormalized https://github.com/airbytehq/airbyte/actions/runs/3226070055
Python tests coverage:

Name                                                              Stmts   Miss  Cover
-------------------------------------------------------------------------------------
normalization/transform_config/__init__.py                            2      0   100%
normalization/transform_catalog/reserved_keywords.py                 14      0   100%
normalization/transform_catalog/__init__.py                           2      0   100%
normalization/destination_type.py                                    14      0   100%
normalization/__init__.py                                             4      0   100%
normalization/transform_catalog/destination_name_transformer.py     166      8    95%
normalization/transform_catalog/table_name_registry.py              174     34    80%
normalization/transform_config/transform.py                         191     49    74%
normalization/transform_catalog/utils.py                             51     14    73%
normalization/transform_catalog/dbt_macro.py                         22      7    68%
normalization/transform_catalog/catalog_processor.py                147     80    46%
normalization/transform_catalog/transform.py                         61     38    38%
normalization/transform_catalog/stream_processor.py                 589    394    33%
-------------------------------------------------------------------------------------
TOTAL                                                              1437    624    57%

Build Passed

Test summary info:

All Passed

edgao · 2022-10-12T16:05:18Z

is there a recommended reading order for the files in this PR? Also, It looks like some of this is refactoring / moving code around, could you summarize those changes?

(would be great to put this info in the PR description)

github-actions · 2022-10-13T03:43:55Z

NOTE ⚠️ Changes in this PR affect the following connectors. Make sure to run corresponding integration tests:

destination-bigquery-denormalized

suhomud · 2022-10-13T03:53:10Z

is there a recommended reading order for the files in this PR? Also, It looks like some of this is refactoring / moving code around, could you summarize those changes?

(would be great to put this info in the PR description)

@edgao it's definitely should be explained. I added PR description please have a look

edgao

thanks for the description, was super helpful for reading through this!

I added some comments - overall the structure makes a lot of sense, so I just focused on general test implementations. Didn't read through the actual test cases in a lot of depth (LMK if you'd find that valuable); I was more looking for ways to improve readability + future maintainability.

...src/main/java/io/airbyte/integrations/destination/bigquery/formatter/util/FormatterUtil.java

...egrations/destination/bigquery/formatter/DefaultBigQueryDenormalizedRecordFormatterTest.java

...t/java/io/airbyte/integrations/destination/bigquery/BigQueryDenormalizedDestinationTest.java

...egrations/destination/bigquery/formatter/DefaultBigQueryDenormalizedRecordFormatterTest.java

.../integrations/destination/bigquery/formatter/GcsBigQueryDenormalizedRecordFormatterTest.java

...yte/integrations/destination/bigquery/formatter/arrayformater/DefaultArrayFormatterTest.java

...test/java/io/airbyte/integrations/destination/bigquery/formatter/util/FormatterUtilTest.java

github-actions · 2022-10-17T13:03:21Z

NOTE ⚠️ Changes in this PR affect the following connectors. Make sure to run corresponding integration tests:

destination-bigquery-denormalized

github-actions · 2022-10-17T13:22:28Z

NOTE ⚠️ Changes in this PR affect the following connectors. Make sure to run corresponding integration tests:

destination-bigquery-denormalized

...t/java/io/airbyte/integrations/destination/bigquery/BigQueryDenormalizedDestinationTest.java

edgao

left a few inline comments - I think this is really close. Couple final questions and then we should be good to merge 🚛

github-actions · 2022-10-18T14:56:32Z

NOTE ⚠️ Changes in this PR affect the following connectors. Make sure to run corresponding integration tests:

destination-bigquery-denormalized

github-actions · 2022-10-18T15:18:04Z

NOTE ⚠️ Changes in this PR affect the following connectors. Make sure to run corresponding integration tests:

destination-bigquery-denormalized

edgao

thanks for dealing with all the back-and-forth here! LGTM 🎉

* master: (304 commits) Bump helm chart version reference to 0.40.27 (#18152) Bump helm chart version reference to 0.40.26 (#18094) Update deployment.yaml (#18151) Publishes Postgres, MySQL, MSSQL source with changes from #18041 (#18086) Fix minor DBT Cloud Errors. (#18147) Sentry Integration : Stop reporting all non system-error error types. (#18133) Docs: Fix backoff stategy docs (#18143) 🐛 Destination GCS: Fix error logs to log 'Gcs' rather than 'AWS' (#17901) Add openAPI spec for Connector Builder Server (#17535) Alex/mvp UI for dbt cloud integration (#18095) increased timeout for sat tests (#18128) Bmoric/remove dep connector worker (#17977) `recordsRead` should be a long (#18123) doc_update_oath_issue_gsc (#17967) 🎉 Source Zendesk Chat: engagements data fix infinity looping + gradlew format (#18121) 🐛 Source Zendesk Chat: engagements data fix infinity looping (#17745) Custom APM Tracing (#17947) 11679 BigQuery-Denormalized Destination: improve code coverage (#17827) increased timeout for sat tests (#18114) docs: clarify language (#18090) ...

…tehq#17827) * 11679 BigQuery-Denormalized Destination improve code coverage

11679 BigQuery-Denormalized Destination improve code coverage

e295f48

suhomud requested review from grishick and etsybaev October 11, 2022 10:25

github-actions bot added the area/connectors Connector related issues label Oct 11, 2022

octavia-squidington-iv added the connectors/destination/bigquery-denormalized label Oct 11, 2022

suhomud linked an issue Oct 11, 2022 that may be closed by this pull request

BigQuery-Denormalized Destination: improve code coverage #11679

Closed

1 task

grishick requested review from edgao and ryankfu October 11, 2022 15:56

etsybaev approved these changes Oct 11, 2022

View reviewed changes

11679 Clean up commented code

551a902

edgao reviewed Oct 13, 2022

View reviewed changes

11679 PR review changes

869b876

11679 PR review changes

d435899

suhomud requested a review from edgao October 17, 2022 19:01

edgao reviewed Oct 17, 2022

View reviewed changes

...t/java/io/airbyte/integrations/destination/bigquery/BigQueryDenormalizedDestinationTest.java Outdated Show resolved Hide resolved

...t/java/io/airbyte/integrations/destination/bigquery/BigQueryDenormalizedDestinationTest.java Outdated Show resolved Hide resolved

edgao reviewed Oct 17, 2022

View reviewed changes

11679 Refactoring of Gcs and Default formatter tests

1ff0371

11679 Refactoring of Gcs and Default formatter tests

8f44bde

edgao approved these changes Oct 18, 2022

View reviewed changes

suhomud merged commit 8c3162a into master Oct 18, 2022

suhomud deleted the suhomud/11679_bigquery_denormalized_destination_improve_unit_test branch October 18, 2022 16:36

octavia-squidington-iii mentioned this pull request Oct 20, 2022

Bump Airbyte version from 0.40.15 to 0.40.16 #18268

Merged

jhammarstedt pushed a commit to jhammarstedt/airbyte that referenced this pull request Oct 31, 2022

11679 BigQuery-Denormalized Destination: improve code coverage (airby…

2292ff7

…tehq#17827) * 11679 BigQuery-Denormalized Destination improve code coverage

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

11679 BigQuery-Denormalized Destination: improve code coverage #17827

11679 BigQuery-Denormalized Destination: improve code coverage #17827

suhomud commented Oct 11, 2022 •

edited

Loading

github-actions bot commented Oct 11, 2022

suhomud commented Oct 11, 2022 •

edited by github-actions bot

Loading

edgao commented Oct 12, 2022

github-actions bot commented Oct 13, 2022

suhomud commented Oct 13, 2022 •

edited

Loading

edgao left a comment

github-actions bot commented Oct 17, 2022

github-actions bot commented Oct 17, 2022

edgao left a comment

github-actions bot commented Oct 18, 2022

github-actions bot commented Oct 18, 2022

edgao left a comment

11679 BigQuery-Denormalized Destination: improve code coverage #17827

11679 BigQuery-Denormalized Destination: improve code coverage #17827

Conversation

suhomud commented Oct 11, 2022 • edited Loading

What

How

Change list explanation

github-actions bot commented Oct 11, 2022

suhomud commented Oct 11, 2022 • edited by github-actions bot Loading

Build Passed

edgao commented Oct 12, 2022

github-actions bot commented Oct 13, 2022

suhomud commented Oct 13, 2022 • edited Loading

edgao left a comment

Choose a reason for hiding this comment

github-actions bot commented Oct 17, 2022

github-actions bot commented Oct 17, 2022

edgao left a comment

Choose a reason for hiding this comment

github-actions bot commented Oct 18, 2022

github-actions bot commented Oct 18, 2022

edgao left a comment

Choose a reason for hiding this comment

suhomud commented Oct 11, 2022 •

edited

Loading

suhomud commented Oct 11, 2022 •

edited by github-actions bot

Loading

suhomud commented Oct 13, 2022 •

edited

Loading