feat: Add unit tests for GCP support #162
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
WalkthroughThis pull request involves package restructuring and minor code modifications across several GCP integration test files. The changes primarily focus on adjusting package declarations, removing specific imports, and introducing a new test class for GCS format testing. The modifications suggest a reorganization of the project's package structure and test coverage for Google Cloud Storage format handling. Changes
Suggested reviewers
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
varant-zlai
approved these changes
Dec 26, 2024
tchow-zlai
force-pushed
the
tchow/bq-support-9
branch
from
cb65e63 to
8cec3e7
Compare
tchow-zlai
force-pushed
the
tchow/bq-support-10
branch
from
29465e7 to
3caa9f9
Compare
tchow-zlai
force-pushed
the
tchow/bq-support-9
branch
2 times, most recently
from
8963d86 to
98b6e58
Compare
tchow-zlai
force-pushed
the
tchow/bq-support-10
branch
2 times, most recently
from
0f85bbb to
3502bab
Compare
tchow-zlai
force-pushed
the
tchow/bq-support-9
branch
2 times, most recently
from
c30c4ab to
f6573cf
Compare
tchow-zlai
force-pushed
the
tchow/bq-support-10
branch
from
3502bab to
e800c39
Compare
tchow-zlai
force-pushed
the
tchow/bq-support-9
branch
from
f6573cf to
14f7ef9
Compare
tchow-zlai
force-pushed
the
tchow/bq-support-10
branch
2 times, most recently
from
dd86306 to
3b7bb43
Compare
tchow-zlai
force-pushed
the
tchow/bq-support-9
branch
from
d0deb93 to
9390cba
Compare
tchow-zlai
force-pushed
the
tchow/bq-support-10
branch
from
3b7bb43 to
150f0b8
Compare
tchow-zlai
force-pushed
the
tchow/bq-support-9
branch
from
9390cba to
8f9777e
Compare
tchow-zlai
force-pushed
the
tchow/bq-support-10
branch
from
150f0b8 to
9f8afaa
Compare
tchow-zlai
force-pushed
the
tchow/bq-support-9
branch
from
8f9777e to
563195b
Compare
tchow-zlai
force-pushed
the
tchow/bq-support-10
branch
2 times, most recently
from
5da0b1c to
454d9a8
Compare
tchow-zlai
force-pushed
the
tchow/bq-support-10
branch
2 times, most recently
from
616e47c to
2fe5f01
Compare
piyush-zlai
approved these changes
Jan 2, 2025
tchow-zlai
force-pushed
the
tchow/bq-support-10
branch
from
2fe5f01 to
e6fb9e6
Compare
tchow-zlai
force-pushed
the
tchow/bq-support-9
branch
from
7af07f1 to
ce3448d
Compare
tchow-zlai
force-pushed
the
tchow/bq-support-10
branch
from
e6fb9e6 to
37e326a
Compare
david-zlai
approved these changes
Jan 3, 2025
Comment on lines
+18
to
+21
| lazy val spark: SparkSession = SparkSessionBuilder.build( | ||
| "BigQuerySparkTest", | ||
| local = true | ||
| ) |
Contributor
There was a problem hiding this comment.
wondered if we should put this in a setup and then add a teardown to stop the spark session?
Collaborator
Author
There was a problem hiding this comment.
that would be a good way to do it, but I think it may not work well with how things are set up in Chronon - we actually use a singleton spark session everywhere for unit tests. If the tests run in parallel, shutting down the spark session here may cause other in flight unit tests to crash unexpectedly.
-e Co-authored-by: Thomas Chow <[email protected]> -e Co-authored-by: Thomas Chow <[email protected]> -e Co-authored-by: Thomas Chow <[email protected]> -e Co-authored-by: Thomas Chow <[email protected]> -e Co-authored-by: Thomas Chow <[email protected]> Co-authored-by: Thomas Chow <[email protected]>
-e Co-authored-by: Thomas Chow <[email protected]> -e Co-authored-by: Thomas Chow <[email protected]> -e Co-authored-by: Thomas Chow <[email protected]> -e Co-authored-by: Thomas Chow <[email protected]> -e Co-authored-by: Thomas Chow <[email protected]> Co-authored-by: Thomas Chow <[email protected]>
-e Co-authored-by: Thomas Chow <[email protected]> -e Co-authored-by: Thomas Chow <[email protected]> -e Co-authored-by: Thomas Chow <[email protected]> -e Co-authored-by: Thomas Chow <[email protected]> -e Co-authored-by: Thomas Chow <[email protected]> Co-authored-by: Thomas Chow <[email protected]>
-e Co-authored-by: Thomas Chow <[email protected]> -e Co-authored-by: Thomas Chow <[email protected]> -e Co-authored-by: Thomas Chow <[email protected]> -e Co-authored-by: Thomas Chow <[email protected]> -e Co-authored-by: Thomas Chow <[email protected]> Co-authored-by: Thomas Chow <[email protected]>
tchow-zlai
force-pushed
the
tchow/bq-support-10
branch
from
37e326a to
9b39ab2
Compare
Co-authored-by: Thomas Chow <[email protected]>
Co-authored-by: Thomas Chow <[email protected]>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
cloud_gcp/src/test/scala/ai/chronon/integrations/cloud_gcp/GCSFormatTest.scala (1)
3-3: Prefer ScalaTest assertions
You're mixing JUnitassertEqualswith ScalaTestAnyFunSuite. Using ScalaTest's built-ins (e.g.,assert) yields consistency.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro (Legacy)
📒 Files selected for processing (4)
cloud_gcp/src/main/scala/ai/chronon/integrations/cloud_gcp/BigQueryFormat.scala(1 hunks)cloud_gcp/src/test/scala/ai/chronon/integrations/cloud_gcp/BigQueryCatalogTest.scala(1 hunks)cloud_gcp/src/test/scala/ai/chronon/integrations/cloud_gcp/DataprocSubmitterTest.scala(1 hunks)cloud_gcp/src/test/scala/ai/chronon/integrations/cloud_gcp/GCSFormatTest.scala(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- cloud_gcp/src/main/scala/ai/chronon/integrations/cloud_gcp/BigQueryFormat.scala
- cloud_gcp/src/test/scala/ai/chronon/integrations/cloud_gcp/DataprocSubmitterTest.scala
🔇 Additional comments (5)
cloud_gcp/src/test/scala/ai/chronon/integrations/cloud_gcp/BigQueryCatalogTest.scala (1)
1-1: Verify references. The package rename might break them if they’re no longer in scope.cloud_gcp/src/test/scala/ai/chronon/integrations/cloud_gcp/GCSFormatTest.scala (4)
16-21: Revisit shared Spark session
Using a shared singleton spark session can cause concurrency issues in parallel test runs (previously discussed).
23-41: Comprehensive partition test
This effectively tests partition logic. Good coverage.
43-61: Empty partition test
Nice check for absence of partitions.
63-91: Date partition test
Good approach. Watch out for time zone differences.
Co-authored-by: Thomas Chow <[email protected]>
This was referenced Jan 4, 2025
4 tasks
4 tasks
This was referenced Apr 7, 2025
kumar-zlai
pushed a commit
that referenced
this pull request
Apr 25, 2025
## Summary - Adding unit tests for #147 - https://app.asana.com/0/1208949807589885/1208960391734329/f ## Checklist - [x] Added Unit Tests - [ ] Covered by existing CI - [ ] Integration tested - [ ] Documentation update <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Added a new test class to validate GCS format partitioning functionality. - **Refactor** - Updated package structure for test files. - Removed specific imports in test files. - **Chores** - Added an import for a BigQuery table in the format handling. <!-- end of auto-generated comment: release notes by coderabbit.ai --> <!-- av pr metadata This information is embedded by the av CLI when creating PRs to track the status of stacks when using Aviator. Please do not delete or edit this section of the PR. ``` {"parent":"main","parentHead":"","trunk":"main"} ``` --> --------- Co-authored-by: Thomas Chow <[email protected]>
kumar-zlai
pushed a commit
that referenced
this pull request
Apr 29, 2025
## Summary - Adding unit tests for #147 - https://app.asana.com/0/1208949807589885/1208960391734329/f ## Checklist - [x] Added Unit Tests - [ ] Covered by existing CI - [ ] Integration tested - [ ] Documentation update <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Added a new test class to validate GCS format partitioning functionality. - **Refactor** - Updated package structure for test files. - Removed specific imports in test files. - **Chores** - Added an import for a BigQuery table in the format handling. <!-- end of auto-generated comment: release notes by coderabbit.ai --> <!-- av pr metadata This information is embedded by the av CLI when creating PRs to track the status of stacks when using Aviator. Please do not delete or edit this section of the PR. ``` {"parent":"main","parentHead":"","trunk":"main"} ``` --> --------- Co-authored-by: Thomas Chow <[email protected]>
chewy-zlai
pushed a commit
that referenced
this pull request
May 15, 2025
## Summary - Adding unit tests for #147 - https://app.asana.com/0/1208949807589885/1208960391734329/f ## Checklist - [x] Added Unit Tests - [ ] Covered by existing CI - [ ] Integration tested - [ ] Documentation update <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Added a new test class to validate GCS format partitioning functionality. - **Refactor** - Updated package structure for test files. - Removed specific imports in test files. - **Chores** - Added an import for a BigQuery table in the format handling. <!-- end of auto-generated comment: release notes by coderabbit.ai --> <!-- av pr metadata This information is embedded by the av CLI when creating PRs to track the status of stacks when using Aviator. Please do not delete or edit this section of the PR. ``` {"parent":"main","parentHead":"","trunk":"main"} ``` --> --------- Co-authored-by: Thomas Chow <[email protected]>
chewy-zlai
pushed a commit
that referenced
this pull request
May 16, 2025
## Summary - Adding unit tests for #147 - https://app.asana.com/0/1208949807589885/1208960391734329/f ## Cheour clientslist - [x] Added Unit Tests - [ ] Covered by existing CI - [ ] Integration tested - [ ] Documentation update <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Added a new test class to validate GCS format partitioning functionality. - **Refactor** - Updated paour clientsage structure for test files. - Removed specific imports in test files. - **Chores** - Added an import for a BigQuery table in the format handling. <!-- end of auto-generated comment: release notes by coderabbit.ai --> <!-- av pr metadata This information is embedded by the av CLI when creating PRs to traour clients the status of staour clientss when using Aviator. Please do not delete or edit this section of the PR. ``` {"parent":"main","parentHead":"","trunk":"main"} ``` --> --------- Co-authored-by: Thomas Chow <[email protected]>
4 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Checklist
Summary by CodeRabbit
New Features
Refactor
Chores