Skip to content

Conversation

@mpeddada1
Copy link
Collaborator

No description provided.

@product-auto-label product-auto-label bot added the api: bigtable Issues related to the Bigtable API. label Nov 10, 2025
@mpeddada1 mpeddada1 marked this pull request as ready for review November 10, 2025 20:08
@mpeddada1 mpeddada1 requested a review from a team as a code owner November 10, 2025 20:08
@mpeddada1 mpeddada1 requested a review from scotthart November 10, 2025 20:08
diegomarquezp
diegomarquezp previously approved these changes Nov 10, 2025
@codecov
Copy link

codecov bot commented Nov 10, 2025

Codecov Report

❌ Patch coverage is 4.16667% with 46 lines in your changes missing coverage. Please review.
✅ Project coverage is 93.03%. Comparing base (fa02b1b) to head (d6b7e19).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
...ogle/cloud/bigtable/tests/data_integration_test.cc 4.16% 46 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #15737      +/-   ##
==========================================
- Coverage   93.05%   93.03%   -0.03%     
==========================================
  Files        2445     2445              
  Lines      225986   226034      +48     
==========================================
- Hits       210297   210284      -13     
- Misses      15689    15750      +61     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

std::map<std::string, std::pair<std::string, std::string>> expected_values;
expected_values[row_key1] = {value11, value12};
expected_values[row_key2] = {value21, value22};
for (auto const& row_status : rows) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What I want to verify with this test is using StreamOf to read the rows. So let's change the for loop to look something like:

using RowType = std::tuple<std::string, std::string, std::string>;
std::vector<RowType> rows;
for (auto& row : StreamOf<RowType>(row_stream)) {
  ASSERT_STATUS_OK(row);
  rows.push_back(std::move(row));
}

EXPECT_THAT(rows, UnorderedElementsAre(RowType(), RowType("multi-column-query-row-1", "v11", "v12),
                                                                  RowType("multi-column-query-row-2", "v21", "v22")));

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The rest of these ASSERT/EXPECTS can be skipped after adding the above.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it! This really helps in cleaning up the assertions too, thanks.

@mpeddada1 mpeddada1 enabled auto-merge (squash) November 10, 2025 21:47
@mpeddada1 mpeddada1 merged commit 601c5b8 into googleapis:main Nov 10, 2025
69 of 71 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api: bigtable Issues related to the Bigtable API.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants