Updated topic tests logging configuration#640
Merged
alex268 merged 3 commits intoydb-platform:masterfrom Apr 22, 2026
Merged
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #640 +/- ##
============================================
- Coverage 70.86% 70.85% -0.01%
- Complexity 3310 3313 +3
============================================
Files 374 374
Lines 15699 15699
Branches 1650 1650
============================================
- Hits 11125 11124 -1
- Misses 3929 3930 +1
Partials 645 645 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
pnv1
approved these changes
Apr 22, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
Updates Topic module test logging to be less noisy and more consistent by centralizing temporary log suppression and adjusting log levels.
Changes:
- Update test
log4j2.xmlpattern/layout and tweak logger levels (e.g.,tech.ydbtoinfo). - Introduce
@HideLoggers+HideLoggersRuleto silence specific noisy loggers during selected JUnit4 tests. - Reduce verbosity in writer implementation logs (
debug→trace) and change write-session close logging.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| topic/src/test/resources/log4j2.xml | Adjusts test logging format and default logger levels to reduce noise. |
| topic/src/test/java/tech/ydb/topic/write/impl/WriterQueueTest.java | Replaces ad-hoc log silencing with @HideLoggers + rule. |
| topic/src/test/java/tech/ydb/topic/utils/HideLoggersRule.java | Adds JUnit4 TestRule to temporarily turn off specified loggers per test method. |
| topic/src/test/java/tech/ydb/topic/utils/HideLoggers.java | Adds method-level annotation to declare loggers to hide for a test. |
| topic/src/test/java/tech/ydb/topic/impl/TopicRetryableStreamTest.java | Uses the new logger-hiding rule for a noisy test. |
| topic/src/test/java/tech/ydb/topic/impl/SerialExecutorTest.java | Uses the new logger-hiding rule for tests that intentionally trigger warnings/errors. |
| topic/src/test/java/tech/ydb/topic/YdbTopicsIntegrationTest.java | Fixes package declaration to match directory structure. |
| topic/src/test/java/tech/ydb/topic/YdbTopicsCodecIntegrationTest.java | Fixes package + migrates log silencing to @HideLoggers/rule. |
| topic/src/test/java/tech/ydb/topic/TopicReadersIntegrationTest.java | Fixes package + migrates log silencing to @HideLoggers/rule. |
| topic/src/main/java/tech/ydb/topic/write/impl/WriterQueue.java | Lowers per-message “prepare sent message” logging to trace. |
| topic/src/main/java/tech/ydb/topic/write/impl/WriteSession.java | Lowers ack logging to trace and changes close logging to info. |
Comments suppressed due to low confidence (4)
topic/src/test/java/tech/ydb/topic/TopicReadersIntegrationTest.java:5
TopicReadersIntegrationTestnow importstech.ydb.topic.YdbTopicsIntegrationTestbefore thejava.*/org.*imports. This violates the project's CheckstyleImportOrderconfiguration (java/javax first, then third-party, thentech.ydb), and the import itself is redundant because the class is in the same package after the package rename. Consider removing this import and usingTopicReadersIntegrationTest.class(or fully qualifying only where needed) so imports can follow the enforced order.
topic/src/test/java/tech/ydb/topic/YdbTopicsCodecIntegrationTest.java:42- The
tech.ydb.topic.utils.HideLoggersimport is placed aftertech.ydb.topic.write.*imports, which is out of order for the project's CheckstyleImportOrder(imports must be sorted within thetech.ydbgroup). Please reorder thetech.ydb.*imports sotech.ydb.topic.utils.*comes beforetech.ydb.topic.write.*.
topic/src/test/java/tech/ydb/topic/YdbTopicsIntegrationTest.java:3 - After changing the package to
tech.ydb.topic, any imports from the same package (e.g.,import tech.ydb.topic.TopicClient;) become redundant and may fail Checkstyle'sRedundantImportrule if test sources are included in the Checkstyle run. Please remove same-package imports and rely on simple type names instead.
topic/src/test/java/tech/ydb/topic/YdbTopicsCodecIntegrationTest.java:4 - After changing the package to
tech.ydb.topic, imports from that same package (e.g.,TopicClient) become redundant. If Checkstyle is run on test sources,RedundantImportwill fail; please remove same-package imports and rely on simple names.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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
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.
No description provided.