refactor: Extract test loop boilerplate and add type helpers#16766
Open
han-yan01 wants to merge 1 commit intofacebookincubator:mainfrom
Open
refactor: Extract test loop boilerplate and add type helpers#16766han-yan01 wants to merge 1 commit intofacebookincubator:mainfrom
han-yan01 wants to merge 1 commit intofacebookincubator:mainfrom
Conversation
meta-cla
bot
added
the
CLA Signed
|
@han-yan01 has exported this pull request. If you are a Meta employee, you can view the originating Diff in D96376610. |
✅ Deploy Preview for meta-velox canceled.
|
…elpers (facebookincubator#16766) Summary: **Phase 1:** Reduced test LOC by extracting createIndexBoundEncodeTestCases + for-loop pattern into runIndexBoundTests() helper. Replaced 149 occurrences with single-line calls. Also fixed critical bug at line 158: was testing kDescNullsFirst twice instead of kDescNullsFirst + kDescNullsLast, meaning DESC_NULLS_LAST sort order was never tested. **Phase 2:** Added type-specific row creation helpers (makeIntRow, makeBigIntRow, makeSmallIntRow, makeTinyIntRow, makeStringRow) with overloads for single-column, two-column, and nullable variants. Applied these systematically across all encodeIndexBoundsWith* tests using automated refactoring script. **Phase 3:** Removed 997 redundant sort order comments (e.g., "// ASC_NULLS_FIRST lower") from runIndexBoundTests() calls. The parameter names (ascNullsFirstExpectedLowerBound, etc.) are self-documenting, making the inline comments pure noise. Applied automated regex-based removal followed by clang-format reflow, which collapsed multi-line expressions for an additional 241-line reduction. **Net result:** Reduced KeyEncoderTest.cpp from 12,165 lines to 10,192 lines (1,973 lines removed, 16.2% reduction) while maintaining 100% test coverage. Differential Revision: D96376610
meta-codesync
bot
changed the title
han-yan01
force-pushed
the
export-D96376610
branch
from
1a508a2 to
82d35a7
Compare
han-yan01
changed the title
han-yan01
changed the title
xiaoxmeng
approved these changes
Mar 14, 2026
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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:
Reduced test LOC by extracting createIndexBoundEncodeTestCases + for-loop pattern
into runIndexBoundTests() helper. Replaced 149 occurrences with single-line calls.
Also fixed critical bug at line 158: was testing kDescNullsFirst
twice instead of kDescNullsFirst + kDescNullsLast, meaning
DESC_NULLS_LAST sort order was never tested.
Added type-specific row creation helpers (makeIntRow, makeBigIntRow,
makeSmallIntRow, makeTinyIntRow, makeStringRow) with overloads for single-column,
two-column, and nullable variants. Applied these systematically across all
encodeIndexBoundsWith* tests using automated refactoring script.
Removed 997 redundant sort order comments (e.g., "// ASC_NULLS_FIRST lower")
from runIndexBoundTests() calls. The parameter names (ascNullsFirstExpectedLowerBound, etc.)
are self-documenting, making the inline comments pure noise. Applied automated regex-based
removal followed by clang-format reflow, which collapsed multi-line expressions.
Differential Revision: D96376610