Skip to content

Add test utility for wrapping directories in FilterDirectory layer#143563

Merged
ChrisHegarty merged 17 commits intoelastic:mainfrom
ChrisHegarty:wrapped_test_dir
Mar 26, 2026
Merged

Add test utility for wrapping directories in FilterDirectory layer#143563
ChrisHegarty merged 17 commits intoelastic:mainfrom
ChrisHegarty:wrapped_test_dir

Conversation

@ChrisHegarty
Copy link
Copy Markdown
Contributor

@ChrisHegarty ChrisHegarty commented Mar 4, 2026

Elasticsearch wraps directories in multiple FilterDirectory layers in production (e.g. StoreDirectory -> ByteSizeCachingDirectory -> MMapDirectory), but unit tests typically pass bare directory instances. This mismatch allowed the ClassCastException fixed in #143531 to go undetected by unit tests.

This PR adds ESTestCase.maybeWrapDirectoryInFilterDirectory, a randomized test utility that wraps a Directory in 0-3 FilterDirectory layers. It can be used by any test where production code receives a Directory that may be wrapped. And we can expand up it later with specific ES wrapper types, if necessary, but minimally FilterDirectory is enough to catch most of these cases.

MemorySegmentUtilsTests is updated to use this utility in all tests that pass a Directory to getContiguousMemorySegment / getContiguousPackedMemorySegment. Additionally, new tests are also added for getContiguousPackedMemorySegment, which previously had no direct test coverage.

relates #143531

@ChrisHegarty ChrisHegarty added >test Issues or PRs that are addressing/adding tests :Search Relevance/Vectors Vector search Team:Search Relevance Meta label for the Search Relevance team in Elasticsearch labels Mar 4, 2026
@elasticsearchmachine
Copy link
Copy Markdown
Collaborator

Pinging @elastic/es-search-relevance (Team:Search Relevance)

Copy link
Copy Markdown
Contributor

@ldematte ldematte left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

Copy link
Copy Markdown
Contributor

@mayya-sharipova mayya-sharipova left a comment

Choose a reason for hiding this comment

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

Very nice, makes sense.
We can merge this after #143531 is merged.

@ChrisHegarty ChrisHegarty added the auto-backport Automatically create backport pull requests when merged label Mar 24, 2026
@ChrisHegarty ChrisHegarty added the test-gpu Run tests using a GPU label Mar 25, 2026
@ChrisHegarty ChrisHegarty merged commit f98aefe into elastic:main Mar 26, 2026
37 checks passed
@elasticsearchmachine
Copy link
Copy Markdown
Collaborator

💔 Backport failed

Status Branch Result
9.3 Commit could not be cherrypicked due to conflicts

You can use sqren/backport to manually backport by running backport --upstream elastic/elasticsearch --pr 143563

szybia added a commit to szybia/elasticsearch that referenced this pull request Mar 26, 2026
* upstream/main: (146 commits)
  Revert "[Native] Gradle-related tweaks to improve handling of the simdvec native library (elastic#144539)"
  Fix ArrayIndexOutOfBoundsException in fetch phase with partial results (elastic#144385)
  ESQL: Correctly manage NULL data type for SUM (elastic#144942)
  [ESQL] Fixes GroupedTopNBenchmark not executing (elastic#144944)
  Fix reader context leak when query response serialization fails (elastic#144708)
  Validate individual offset values in BULK_OFFSETS bounds checks (elastic#144643)
  Merge main21 source set into main in simdvec (elastic#144921)
  [TEST] Unmute TsidExtractingIdFieldMapperTests (elastic#144848)
  [Native] Gradle-related tweaks to improve handling of the simdvec native library (elastic#144539)
  Fix `ThreadedActionListenerTests#testRejectionHandling` (elastic#144795)
  Add new DLM Frozen Tier Transition execution plugin and service (elastic#144595)
  Prometheus: execute query_range via parsed EsqlStatement plan (elastic#144416)
  Investigate `testBulkIndexingRequestSplitting` failure (elastic#144766)
  Add test utility for wrapping directories in FilterDirectory layer (elastic#143563)
  Fix ES|QL decay tests with negative scale (elastic#144657)
  Fix circuit breaker leak in percolator query construction (elastic#144827)
  Use XPerFieldDocValuesFormat in AbstractTSDBSyntheticIdCodec (elastic#144744)
  [DOCS] Document how reindex work in CPS (elastic#144016)
  Fix Int4 vector library tests failing on Java 21 (elastic#144830)
  [DiskBBQ] Fix index sorting on flush (elastic#144938)
  ...
seanzatzdev pushed a commit to seanzatzdev/elasticsearch that referenced this pull request Mar 26, 2026
…lastic#143563)

Elasticsearch wraps directories in multiple FilterDirectory layers in production (e.g. StoreDirectory -> ByteSizeCachingDirectory -> MMapDirectory), but unit tests typically pass bare directory instances. This mismatch allowed the ClassCastException fixed in elastic#143531 to go undetected by unit tests.

This PR adds ESTestCase.maybeWrapDirectoryInFilterDirectory, a randomized test utility that wraps a Directory in 0-3 FilterDirectory layers. It can be used by any test where production code receives a Directory that may be wrapped. And we can expand up it later with specific ES wrapper types, if necessary, but minimally FilterDirectory is enough to catch most of these cases.

MemorySegmentUtilsTests is updated to use this utility in all tests that pass a Directory to getContiguousMemorySegment / getContiguousPackedMemorySegment. Additionally, new tests are also added for getContiguousPackedMemorySegment, which previously had no direct test coverage.

relates elastic#143531
seanzatzdev pushed a commit to seanzatzdev/elasticsearch that referenced this pull request Mar 27, 2026
…lastic#143563)

Elasticsearch wraps directories in multiple FilterDirectory layers in production (e.g. StoreDirectory -> ByteSizeCachingDirectory -> MMapDirectory), but unit tests typically pass bare directory instances. This mismatch allowed the ClassCastException fixed in elastic#143531 to go undetected by unit tests.

This PR adds ESTestCase.maybeWrapDirectoryInFilterDirectory, a randomized test utility that wraps a Directory in 0-3 FilterDirectory layers. It can be used by any test where production code receives a Directory that may be wrapped. And we can expand up it later with specific ES wrapper types, if necessary, but minimally FilterDirectory is enough to catch most of these cases.

MemorySegmentUtilsTests is updated to use this utility in all tests that pass a Directory to getContiguousMemorySegment / getContiguousPackedMemorySegment. Additionally, new tests are also added for getContiguousPackedMemorySegment, which previously had no direct test coverage.

relates elastic#143531
mamazzol pushed a commit to mamazzol/elasticsearch that referenced this pull request Mar 30, 2026
…lastic#143563)

Elasticsearch wraps directories in multiple FilterDirectory layers in production (e.g. StoreDirectory -> ByteSizeCachingDirectory -> MMapDirectory), but unit tests typically pass bare directory instances. This mismatch allowed the ClassCastException fixed in elastic#143531 to go undetected by unit tests.

This PR adds ESTestCase.maybeWrapDirectoryInFilterDirectory, a randomized test utility that wraps a Directory in 0-3 FilterDirectory layers. It can be used by any test where production code receives a Directory that may be wrapped. And we can expand up it later with specific ES wrapper types, if necessary, but minimally FilterDirectory is enough to catch most of these cases.

MemorySegmentUtilsTests is updated to use this utility in all tests that pass a Directory to getContiguousMemorySegment / getContiguousPackedMemorySegment. Additionally, new tests are also added for getContiguousPackedMemorySegment, which previously had no direct test coverage.

relates elastic#143531
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

auto-backport Automatically create backport pull requests when merged backport pending :Search Relevance/Vectors Vector search Team:Search Relevance Meta label for the Search Relevance team in Elasticsearch >test Issues or PRs that are addressing/adding tests test-gpu Run tests using a GPU v9.3.3 v9.4.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants