Skip to content

Fix MutableColumnStatisticsTest#18018

Merged
xiangfu0 merged 1 commit intoapache:masterfrom
Jackie-Jiang:fix_stats_test
Mar 29, 2026
Merged

Fix MutableColumnStatisticsTest#18018
xiangfu0 merged 1 commit intoapache:masterfrom
Jackie-Jiang:fix_stats_test

Conversation

@Jackie-Jiang
Copy link
Copy Markdown
Contributor

No description provided.

@Jackie-Jiang Jackie-Jiang added the testing Related to tests or test infrastructure label Mar 29, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Fixes MutableColumnStatisticsTest to align with the current MutableColumnStatistics implementation, which computes variable-width element lengths via Dictionary#getValueSize().

Changes:

  • Update the test to mock Dictionary#getValueSize(int) instead of getStringValue(int).
  • Add Guava Utf8 usage for computing UTF-8 byte length in the mock answer.

Comment on lines +56 to +57
when(dictionary.getValueSize(anyInt())).thenAnswer(
invocation -> Utf8.encodedLength(elements[(int) invocation.getArgument(0)]));
Copy link

Copilot AI Mar 29, 2026

Choose a reason for hiding this comment

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

The mocked dictionary.getValueSize() uses Utf8.encodedLength(...), but the expected minElementLength/maxElementLength are computed with randomString.getBytes(UTF_8).length. These can diverge (and Utf8.encodedLength can throw for malformed surrogate pairs), making the test potentially flaky. Consider computing expected lengths using the same Utf8.encodedLength(...) logic (or restrict generated strings to ASCII/alphanumeric) so the test matches the production StringDictionary#getValueSize behavior.

Copilot uses AI. Check for mistakes.
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Mar 29, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 63.29%. Comparing base (a664c82) to head (eba8553).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@             Coverage Diff              @@
##             master   #18018      +/-   ##
============================================
+ Coverage     55.53%   63.29%   +7.75%     
- Complexity      752     1543     +791     
============================================
  Files          2505     3200     +695     
  Lines        143118   194169   +51051     
  Branches      22967    29915    +6948     
============================================
+ Hits          79481   122899   +43418     
- Misses        56897    61611    +4714     
- Partials       6740     9659    +2919     
Flag Coverage Δ
custom-integration1 100.00% <ø> (ø)
integration 100.00% <ø> (ø)
integration1 100.00% <ø> (ø)
integration2 0.00% <ø> (ø)
java-11 63.25% <ø> (+7.75%) ⬆️
java-21 63.27% <ø> (+7.77%) ⬆️
temurin 63.29% <ø> (+7.75%) ⬆️
unittests 63.29% <ø> (+7.75%) ⬆️
unittests1 55.53% <ø> (+<0.01%) ⬆️
unittests2 34.20% <ø> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

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

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@xiangfu0 xiangfu0 merged commit c23b8fd into apache:master Mar 29, 2026
35 of 36 checks passed
@Jackie-Jiang Jackie-Jiang deleted the fix_stats_test branch March 29, 2026 05:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

testing Related to tests or test infrastructure

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants