fix(tests): build the test tree from node ids, not from message order - #71
Open
roxblnfk wants to merge 1 commit into
Open
fix(tests): build the test tree from node ids, not from message order#71roxblnfk wants to merge 1 commit into
roxblnfk wants to merge 1 commit into
Conversation
Testo runs tests concurrently — fibers, an event loop — so two nodes are open at the same time and its service messages interleave. The name-based convertor nests whatever opened last inside whatever sits on its stack, so a run of a case with two `#[DataSet]` batches shows the second batch *inside* the first, and neither node ever closes. Data sets of different batches share a display name, which makes the result read as duplicated rows. Testo already puts `nodeId`/`parentNodeId` on every service message, so the tree is stated rather than inferred: `isIdBasedTestTree()` switches the platform to `GeneralIdBasedToSMTRunnerEventsConvertor`, which reads them and no longer cares in what order the messages arrive. Assisted-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
3 tasks
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.
Problem
Testo runs tests concurrently — fibers, an event loop — so two nodes are open at once and its service
messages interleave. The name-based events convertor takes the tree from the order messages arrive in:
it nests whatever opened last inside whatever sits on its stack.
Running a case with two
#[DataSet]batches (e.g. Testo's owntests/Sandbox/Self/AsyncTest.php) showsthe second batch inside the first, and neither node ever closes. Data sets of different batches carry
the same display name (
Dataset #0:0 [0]), so the result also reads as duplicated rows.Fix
Testo puts
nodeId/parentNodeIdon every service message, so the tree is stated rather than inferred.isIdBasedTestTree()switches the platform toGeneralIdBasedToSMTRunnerEventsConvertor(
SMTestRunnerConnectionUtilpicks it off that flag), which reads those attributes and stops caring aboutmessage order.
What Testo emits now:
Node ids come from the run: every node — suite, case, test, data set — has one, and every message about it
repeats it, so a finish or an output chunk lands on the right node no matter what else is in flight.
Heads-up before merging
This needs a Testo release that emits the ids. The convertor drops test messages without a
nodeId, sowith an older Testo the tree would come out empty. The emitting side is on Testo's
feat/test-identitybranch and is not in
1.xyet — please merge/release this only after that lands.There is no version gate here on purpose:
isIdBasedTestTree()is read when the console is created, andTestoVersionDetectoris currently wired only into the settings form, so the version is not known at thatpoint. Happy to add a gate if you want one — it needs the detected version threaded to the console properties.
Follow-up left out of this PR
ChannelOutputStorekeys output by test name:locationByNamemaps name -> locationHint, and the twobatches above share a name, so the second
testStartedoverwrites the first's entry and both tests' outputcollapses into one bucket. That is a separate defect, visible without concurrency too. The proper key is the
nodeId, but that also touches the lookup side (TestoChannelsUi,TestoChannelHistorygo throughkeyFor(proxy.name), andSMTestProxydoes not expose the node id), so it is your design call rather thansomething to guess at blind.
Checks
./gradlew compileKotlinand./gradlew testpass locally (JBR 25, Gradle 9.6).🤖 Generated with Claude Code