fix(inkless:ci): fix RequestQuotaTest and make CI test failures visible#692
Merged
Conversation
RequestQuotaTest iterates over all ApiKeys.brokerApis and builds a request for each, but requestBuilder had no case for the inkless-added ALTER_DISKLESS_SWITCH key, hitting the fallthrough and throwing IllegalArgumentException. This deterministically failed testUnauthorizedThrottle, testUnthrottledClient and testResponseThrottleTime.
The nightly parse step pointed junit.py at build/junit-xml/<java>, but copyTestXml writes to build/junit-xml/<module>/<java>. The glob matched nothing, so real test failures were reported as '0 tests' and the job failed only on the gradle exit code. Point at build/junit-xml so the recursive **/*.xml glob finds all module reports, matching inkless.yml.
PR #688 removed all test retries from inkless.yml, exposing the CI to known upstream flakiness (e.g. ReplicaManagerTest). Split the retry policy by test ownership: kafka-owned suites are retried (maxTestRetries=1, maxTestRetryFailures=3), while inkless-owned tests (:storage:inkless and *Inkless*/*Diskless*/io.aiven.inkless.*) run without retries so our own flakiness stays visible. The mixed :core:test invocation is split accordingly. Also add kafka.server.RequestQuotaTest to the kafka-owned :core:test group. It iterates over every broker ApiKey, so it guards that each inkless-added API (e.g. ALTER_DISKLESS_SWITCH) is wired into the request-building/quota paths, shifting that check left from nightly to per-PR CI.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes an Inkless-specific CI red state by updating a Kafka server unit test to handle the Inkless-added ALTER_DISKLESS_SWITCH API key and by correcting/adjusting CI workflows so JUnit failures are properly detected and reported, with retry behavior split by ownership.
Changes:
- Add an
ALTER_DISKLESS_SWITCHrequest builder case inRequestQuotaTestto prevent false failures when iteratingApiKeys.brokerApis. - Update nightly JUnit parsing to scan
build/junit-xml(so test failures are visible in the summary). - Adjust per-PR CI Gradle retry args to separate Inkless-owned (no retries) vs Kafka-owned (retries enabled) test suites, and include
kafka.server.RequestQuotaTestin per-PR runs.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| core/src/test/scala/unit/kafka/server/RequestQuotaTest.scala | Adds request builder support for the Inkless API key to stop unit test failures. |
| .github/workflows/inkless.yml | Splits retry configuration by suite ownership and adds RequestQuotaTest to per-PR CI selection. |
| .github/workflows/inkless-nightly.yml | Fixes JUnit report path so nightly summaries reflect actual failures. |
The test step chained gradle invocations with && but only wrapped the
first call in 'timeout ${TIMEOUT_MINUTES}m', so a hang in any later
suite ran untimed and never produced exit 124 (which gates the
thread-dump upload). Wrap the entire chain in a single
'timeout ... bash -c' so the budget covers all suites and a hang
anywhere is killed and reported. Applied to inkless.yml and
inkless-nightly.yml.
jeqo
marked this pull request as ready for review
July 9, 2026 13:33
tvainika
approved these changes
Jul 9, 2026
jeqo
added a commit
that referenced
this pull request
Jul 20, 2026
…le (#692) * fix(inkless:test): handle ALTER_DISKLESS_SWITCH in RequestQuotaTest RequestQuotaTest iterates over all ApiKeys.brokerApis and builds a request for each, but requestBuilder had no case for the inkless-added ALTER_DISKLESS_SWITCH key, hitting the fallthrough and throwing IllegalArgumentException. This deterministically failed testUnauthorizedThrottle, testUnthrottledClient and testResponseThrottleTime. * fix(inkless:ci): correct nightly JUnit report path The nightly parse step pointed junit.py at build/junit-xml/<java>, but copyTestXml writes to build/junit-xml/<module>/<java>. The glob matched nothing, so real test failures were reported as '0 tests' and the job failed only on the gradle exit code. Point at build/junit-xml so the recursive **/*.xml glob finds all module reports, matching inkless.yml. * fix(inkless:ci): retry kafka-owned tests, keep inkless tests unretried PR #688 removed all test retries from inkless.yml, exposing the CI to known upstream flakiness (e.g. ReplicaManagerTest). Split the retry policy by test ownership: kafka-owned suites are retried (maxTestRetries=1, maxTestRetryFailures=3), while inkless-owned tests (:storage:inkless and *Inkless*/*Diskless*/io.aiven.inkless.*) run without retries so our own flakiness stays visible. The mixed :core:test invocation is split accordingly. Also add kafka.server.RequestQuotaTest to the kafka-owned :core:test group. It iterates over every broker ApiKey, so it guards that each inkless-added API (e.g. ALTER_DISKLESS_SWITCH) is wired into the request-building/quota paths, shifting that check left from nightly to per-PR CI. * fix(inkless:ci): enforce the test timeout across the whole suite chain The test step chained gradle invocations with && but only wrapped the first call in 'timeout ${TIMEOUT_MINUTES}m', so a hang in any later suite ran untimed and never produced exit 124 (which gates the thread-dump upload). Wrap the entire chain in a single 'timeout ... bash -c' so the budget covers all suites and a hang anywhere is killed and reported. Applied to inkless.yml and inkless-nightly.yml.
jeqo
added a commit
that referenced
this pull request
Jul 20, 2026
…le (#692) * fix(inkless:test): handle ALTER_DISKLESS_SWITCH in RequestQuotaTest RequestQuotaTest iterates over all ApiKeys.brokerApis and builds a request for each, but requestBuilder had no case for the inkless-added ALTER_DISKLESS_SWITCH key, hitting the fallthrough and throwing IllegalArgumentException. This deterministically failed testUnauthorizedThrottle, testUnthrottledClient and testResponseThrottleTime. * fix(inkless:ci): correct nightly JUnit report path The nightly parse step pointed junit.py at build/junit-xml/<java>, but copyTestXml writes to build/junit-xml/<module>/<java>. The glob matched nothing, so real test failures were reported as '0 tests' and the job failed only on the gradle exit code. Point at build/junit-xml so the recursive **/*.xml glob finds all module reports, matching inkless.yml. * fix(inkless:ci): retry kafka-owned tests, keep inkless tests unretried PR #688 removed all test retries from inkless.yml, exposing the CI to known upstream flakiness (e.g. ReplicaManagerTest). Split the retry policy by test ownership: kafka-owned suites are retried (maxTestRetries=1, maxTestRetryFailures=3), while inkless-owned tests (:storage:inkless and *Inkless*/*Diskless*/io.aiven.inkless.*) run without retries so our own flakiness stays visible. The mixed :core:test invocation is split accordingly. Also add kafka.server.RequestQuotaTest to the kafka-owned :core:test group. It iterates over every broker ApiKey, so it guards that each inkless-added API (e.g. ALTER_DISKLESS_SWITCH) is wired into the request-building/quota paths, shifting that check left from nightly to per-PR CI. * fix(inkless:ci): enforce the test timeout across the whole suite chain The test step chained gradle invocations with && but only wrapped the first call in 'timeout ${TIMEOUT_MINUTES}m', so a hang in any later suite ran untimed and never produced exit 124 (which gates the thread-dump upload). Wrap the entire chain in a single 'timeout ... bash -c' so the budget covers all suites and a hang anywhere is killed and reported. Applied to inkless.yml and inkless-nightly.yml.
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.
Main CI was red with a misleading
Found 0 JUnit results/0 PASSED, 0 FAILEDsummary despite tests actually failing.Root cause:
:core:testhad 3 real failures inRequestQuotaTest, allIllegalArgumentException: Unsupported API key ALTER_DISKLESS_SWITCH— the test builds a request for everyApiKeys.brokerApisentry, butrequestBuilderhad no case for the inkless-added key (#678). The "0 tests" summary was a second bug: the nightly parse step pointedjunit.pyatbuild/junit-xml/<java>, but reports are written tobuild/junit-xml/<module>/<java>, so the glob matched nothing.Changes
fix(inkless:test): add theALTER_DISKLESS_SWITCHcase toRequestQuotaTest.requestBuilder.fix(inkless:ci): point the nightly parse step atbuild/junit-xmlso failures are actually reported (matchesinkless.yml).fix(inkless:ci): splitinkless.ymlretries by ownership — Kafka-owned suites retried (maxTestRetries=1), inkless-owned tests unretried so our own flakiness stays visible. Also addskafka.server.RequestQuotaTestto per-PR CI so future inkless ApiKeys are guarded there, not just nightly.fix(inkless:ci): wrap the whole test chain in a singletimeout ... bash -cin both workflows. Previously only the first gradle call was budgeted, so a hang in a later suite ran untimed and never emitted exit124(which gates the thread-dump upload).Testing
./gradlew :core:test --tests kafka.server.RequestQuotaTest— passes.