Skip to content

test: cover tablet map eviction on schema change (DRIVER-1000) - #1086

Draft
nikagra wants to merge 1 commit into
scylladb:scylla-4.xfrom
nikagra:test/driver-1000-tablet-eviction
Draft

nikagra wants to merge 1 commit into
scylladb:scylla-4.xfrom
nikagra:test/driver-1000-tablet-eviction

Conversation

@nikagra

@nikagra nikagra commented Sep 15, 2026

Copy link
Copy Markdown

TabletMapSchemaChangeListener had no unit test at all. TabletMapSchemaChangesIT mocks the listener out, so it proves the driver calls onKeyspaceDropped/onTableDropped but never that the listener forwards anything — the removeByKeyspace/removeByTable calls it makes were untested, as were its four no-tablet-map early returns.

  • Covers all four listener callbacks, including onKeyspaceUpdated/onTableUpdated, which evict under the previous name — the cached tablets are keyed by it.
  • Covers the early return on a server without tablets, so the callbacks are no-ops rather than an NPE.
  • DefaultTabletMap: adds the second-sweep overlap eviction (a tablet overlapping only on its leading edge), removeByNode, and DefaultTablet's equals/hashCode/toString.
  • Pins one behaviour worth knowing: removeByTable matches on table name alone, so dropping ks1.foo also evicts ks2.foo. The cost is a refetch, not mis-routing — recorded in a test comment, not filed as a defect.
  • New cases use AssertJ, matching the rest of internal/core/metadata. The four existing methods in DefaultTabletMapTest mix JUnit with org.testng.Assert, whose assertEquals argument order is reversed; converting them is churn inside a coverage PR, so it is left alone deliberately.

Closes all 15 missed lines these two classes had in run 34559159215 (11 in DefaultTabletMap, 4 in the listener), plus the listener's 4 partial branches and DefaultTabletMap's L118/L135/L273. Reported as missed lines closed rather than a percentage, because the #1025 DSE-removal stack moves the aggregate ~3.7pp on its own.

Verified with mvn clean test -pl core: 3980 tests, 0 failures (3968 on the base branch + 12 new). Per-class coverage confirmed from jacoco.xml under -Pcoverage. No integration tests were run locally; the remaining parseTabletPayloadV1 partials are IT-covered in CI and were already partial at baseline.

Part of DRIVER-1000, one of five independent PRs off scylla-4.x — not a stack; this one shares no file with the others.

🤖 Generated with Claude Code

TabletMapSchemaChangeListener had no unit test: the integration test
mocks it, so it only proved the driver calls it, never that it forwards.
All four callbacks are covered here, including the two *Updated* ones
that evict under the previous name, and the no-tablet-map early return.

In DefaultTabletMap the untested parts were the second-sweep overlap
eviction, removeByNode, and DefaultTablet's value methods. Closes all 15
missed lines from run 34559159215.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Sep 15, 2026

Copy link
Copy Markdown

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

  • Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant