Skip to content

test: unit tests for the least covered core packages (metrics, ack, misc helpers) - #219

Open
devin-ai-integration[bot] wants to merge 2 commits into
mainfrom
devin/1784980309-core-coverage-tests
Open

test: unit tests for the least covered core packages (metrics, ack, misc helpers)#219
devin-ai-integration[bot] wants to merge 2 commits into
mainfrom
devin/1784980309-core-coverage-tests

Conversation

@devin-ai-integration

Copy link
Copy Markdown

Description

JaCoCo run over the full netty-socketio-core suite (all 113 test classes, Docker/Testcontainers included) showed these packages/classes as the weakest, ignoring the Redis stream/reliable stores which have no test class matching the surefire **/*Test.java include (DistributedRedissonClusterSuite):

target before after
metrics package 2.8% 100% (MicrometerSocketIOMetrics 99.3%)
ack package 24.8% ~95% (AckManager 91%, AckSchedulerKey/AckEntry 100%)
BroadcastAckCallback / AckCallback / VoidAckCallback / MultiTypeAckCallback 0% 100% (BroadcastAckCallback 95.7%)
MultiTypeArgs 14% 100%
JsonSupportWrapper 31.5% 100%
HandshakeData 40% 100%
WrongUrlHandler 10.8% 100%
store.event.PublishConfig / PublishMode / EventStoreType 0% 100%
listener.ExceptionListenerAdapter / DefaultExceptionListener 30% / partial 100%

No production code was touched; JaCoCo was only wired in locally for measurement and is not part of this diff.

Type of Change

  • Test improvements

Changes Made

  • ack/AckManagerTest — ack index allocation (registerAck returns initAckIndex-relative indexes), single vs. MultiTypeAckCallback dispatch on onAck, timeout scheduling/cancellation and onDisconnect timing out pending callbacks, using a recording CancelableScheduler stub that can fire scheduled callbacks manually
  • ack/AckSchedulerKeyTestequals/hashCode across differing type, session and index, including inequality with the plain SchedulerKey
  • metrics/MicrometerSocketIOMetricsTest — counters/timers/gauges against a SimpleMeterRegistry, empty namespace mapped to default, non-positive durations not recorded, room members gauge clamped at 0, factory + noop implementation
  • metrics/NamespaceMetersTest — meter registration and the periodic (5s) publish window of the unknown-event-name HLL estimate
  • AckCallbackTest — callback hierarchy plus MultiTypeArgs out-of-bounds-safe accessors and the BroadcastAckCallback loopFinished()/counter interplay that gates onAllSuccess
  • JsonSupportWrapperTest, HandshakeDataTest, handler/WrongUrlHandlerTest (via EmbeddedChannel: 400 + release + close), store/event/PublishConfigTest, listener/ExceptionListenerTest

Testing

  • All existing tests pass
  • New tests added for new functionality
  • Tests pass locally with mvn test

76 new tests, all green locally with checkstyle/PMD/license checks enabled.

Link to Devin session: https://app.devin.ai/sessions/727bd5be6dd5437caf53f6ff46651322
Requested by: @sanjomo

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
@sanjomo sanjomo self-assigned this Jul 25, 2026
@devin-ai-integration

Copy link
Copy Markdown
Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment, CI, and merge conflict monitoring

@Test
@DisplayName("Should delegate value writing")
void shouldDelegateWriteValue() throws IOException {
ByteBufOutputStream out = new ByteBufOutputStream(buffer);

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no bugs or issues to report.

Open in Devin Review

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