-
Notifications
You must be signed in to change notification settings - Fork 976
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
tests: Bulk IT Tests #2005
base: main
Are you sure you want to change the base?
tests: Bulk IT Tests #2005
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2005 +/- ##
============================================
+ Coverage 45.40% 52.93% +7.52%
+ Complexity 3672 1367 -2305
============================================
Files 842 378 -464
Lines 49970 20661 -29309
Branches 5261 2090 -3171
============================================
- Hits 22690 10936 -11754
+ Misses 25608 9045 -16563
+ Partials 1672 680 -992
|
* changes to schema. Changes include Index changes, Primary key transformations and Generated | ||
* columns migration. | ||
*/ | ||
@Category({TemplateIntegrationTest.class, SkipDirectRunnerTest.class}) |
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.
nit: If its meant to test primary key transformations and generated columns and index change specifically, can we modify the name to capture that, maybe advancedSchemaChangesIT or something like that.
with comments mentioning more details like this tests:
- Different indexes
- Different primary key columns
- Generated and Default value columns at Source and Not on Spanner migrated correctly.
THis helps track what sort of cases are covered.
spannerResourceManager.readTableRecords( | ||
"employee_attribute", "employee_id", "attribute_name", "value"); | ||
|
||
SpannerAsserts.assertThatStructs(employeeAttribute).hasRows(4); // Supports composite keys |
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.
Any reason we are not doing full row matches like the first assert?
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.
+1 add full row asserts wherever possible.
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
spannerResourceManager.readTableRecords( | ||
"employee_attribute", "employee_id", "attribute_name", "value"); | ||
|
||
SpannerAsserts.assertThatStructs(employeeAttribute).hasRows(4); // Supports composite keys |
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.
+1 add full row asserts wherever possible.
No description provided.