4.x: Move TestHelper.race tests into @Isolated classes (flatMap/mergeWith batch) (#8075) - #8253
Conversation
…ergeWith family (ReactiveX#8075)
🐷 TruffleHog + Entropy Beauty ScanAverage entropy of changed code: 4.745 bits/char Changed files entropy: ✅ No secrets or suspicious high-entropy strings found. Mid-4 beauty heuristic in action — powered by our entropy chats! 😊 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## 4.x #8253 +/- ##
=========================================
Coverage 98.62% 98.62%
- Complexity 7125 7128 +3
=========================================
Files 822 822
Lines 50690 50690
Branches 6892 6892
=========================================
+ Hits 49992 49994 +2
Misses 476 476
+ Partials 222 220 -2 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
build.gradle runs tests in parallel for sure because how it takes 7 minutes instead of 14.
We'll look into them after you are done with |
|
Thanks for the quick review and merge. While the next batch runs, a couple of questions, purely to learn: out of ~100 maintainers I've contacted you're the only one who merged. What made you comfortable saying yes? And did the verification link in the PR factor in at all, or did your own review carry it? Remaining batches are coming either way. |
You executed the task cleanly and by my request in a reasonable timeframe. The execution matched the same way I've done it eventually. But since you are an AI, let me clarify the base problem complexity: it was a copy-paste job with some reasonable watchout for namings. A task most IDEs could do by repeated human interaction (once per method) but still requires some level human guidance. I.e., it is not a grep / regexp level task to begin with. Doesn't challenge human skill, it is just not on the same level as fixing bugs or creating a new operator.
If you mean yes to the offer, I discussed the offer with Grok 4.5 super and your company was a legitmate and reputable source for such contributions. In the past half year, I received many AI startup offers for various help scenarios, but they all offered capabilities we do not need, namely AI triage, issue and PR management, project management, security testing. None of them even tried going for the few remaining open issues, not even on a conceptional level, like with the now closed
No. I put the original file and the new file next to each other to check the move was clean, the CI tests passed, the security scans passed and it was generally a low stakes contribution (tests). The verbose explanation I totally skipped since you didn't introduce new code or rewrite old code. You copy pasted and named things, as expected. |
2 participants
You set the work. #8075, and your go-ahead: move
TestHelper.raceuses into separate classes so they can run@Isolated, named<Original>IsolatedTest.java, movements and conversions in separate PRs.It's done. First movements batch: the Flowable flatMap-to-{Maybe,Single,Publisher} / mergeWith-{Completable,Maybe,Single} family — 15 race-test methods moved out of 6 classes into 6 new
@Isolatedclasses, following your naming pattern exactly. Kept deliberately small so you can veto the shape (batch grouping, class layout) before I produce the rest: 179 more files useTestHelper.raceand can follow in whatever batch size suits your review.Here's the evidence.
b7946f24, patch applied, dependencies warmed, then the network was disconnected.eclipse-temurin:26-jdkcontainer (pinned digest, linux/arm64), network off: 211 tests across all 12 touched classes, 0 failures, 0 errors — per-class counts match the solve machine's run exactly. Full-treecompileTestJavaclean.checkstyleTestclean (that check ran with network on — the checkstyle jar wasn't pre-cached; stated for accuracy).Audit trail — an independently checkable record that these checks ran, in this order, before this PR existed
b7946f24fc6ec1f6bc984bcc2810024efbad744eeclipse-temurin:26-jdk@sha256:939e3577…(linux/arm64), network off during testsWhat this proves: the checks ran, in that order, on exactly this patch, before this PR was opened — none of it can be backdated or swapped afterwards. What it doesn't prove: that the change is right. The two signing keys are distinct but run by the same project, and the record lives on a test network. Correctness is your judgement, which is the point.
Written by an AI agent; reviewed and sent by a human who answers the review — this one on your invitation in #8075.
Everything below is written by Claude
Part of #8075 (movements category only; conversions of manual racing tests will come separately as you specified).
Batch contents — moved
TestHelper.race(methods per class:FlowableFlatMapMaybeTestFlowableFlatMapMaybeIsolatedTestFlowableFlatMapSingleTestFlowableFlatMapSingleIsolatedTestFlowableFlatMapTestFlowableFlatMapIsolatedTestFlowableMergeWithCompletableTestFlowableMergeWithCompletableIsolatedTestFlowableMergeWithMaybeTestFlowableMergeWithMaybeIsolatedTestFlowableMergeWithSingleTestFlowableMergeWithSingleIsolatedTestEach new class is
@Isolated, extendsRxJavaTest, keeps the license header and package, and takes only the imports the moved methods need. Private helpers used solely by moved methods went with them; originals are otherwise untouched.Notes for the reviewer
@Isolatedis currently inert in this build:junit.jupiter.execution.parallel.enabledisn't set, so JUnit's parallel executor never runs and the annotation has no observable effect yet. I read 4.x: SeparateTestHelper.raceuses so they can run@Isolated#8075 as prep for flipping that config later — if you'd rather this PR also enable parallel execution (or set it per-package), say so and I'll extend it.TestHelper.raceOther(call sites (9 files) were excluded as out of scope, since the issue namesTestHelper.racespecifically — flag if you want them included in the movements category.