feat(xcresult): resolve declarations from swift test --xunit-output - #1180
Draft
dfrankland wants to merge 4 commits into
Draft
feat(xcresult): resolve declarations from swift test --xunit-output#1180dfrankland wants to merge 4 commits into
swift test --xunit-output#1180dfrankland wants to merge 4 commits into
Conversation
`swift test --xunit-output` writes no file path for any test, and on Linux there
is no `.xcresult` to fall back to — so today a Swift test run there can never be
attributed to a file. The declaration index needs nothing from Xcode, so it can
answer this too.
The xunit XML carries more than it first appears. `classname` is the target
followed by the dot-qualified suite path, and only collapses to the bare target
for a top-level `@Test func`:
classname="MyCLITests" name="helloworld()"
classname="MyCLITests.AlphaSuite" name="shared()"
classname="MyCLITests.AlphaSuite.Inner" name="deep()"
classname="MyCLITests.BetaSuite" name="shared()"
That maps onto the existing `TestKey` almost unchanged: the innermost component
is the declaring type, exactly as the innermost component of an xcresult
`Suite/Inner/case()` identifier is, so only the separator differs. A top-level
function falls to the suiteless lookup already there for xcresult, and the first
component is the target, so the collision tie-break carries over for free.
Parameterised tests need no special handling either: swift-testing emits one
entry per function rather than one per argument, keeping argument labels
(`squares(n:)`), and that single entry is the declaration site we want.
XCTest needs none either. One run writes **two files** — swift-testing to
`<name>-swift-testing.xml` and XCTest to `<name>` — and the XCTest form is the
same `Module.Type` plus method, minus the `()`:
classname="MyCLITests.LegacyXCTests" name="testOldStyle"
so the same parse resolves it. That second file is only written when `--parallel`
is passed; without it the XCTest cases still run but are silently absent from the
output, which is worth knowing before trusting a project's xunit to be complete.
The fixture is a real package plus both files it actually produced. Its two
suites both declare `shared()` in different files, so a regression that ignored
the suite component would make one borrow the other's file — reverting the suite
component fails those tests rather than neither.
This is the resolver only. Nothing reads a JUnit file or writes a `file`
attribute back yet, and no flag is wired up.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
dfrankland
force-pushed
the
dylan/swift-test-declaration-locations
branch
from
August 31, 2026 19:58
51636d3 to
d28a012
Compare
Both inputs feed one `TestLocationIndex`, and nothing checked they agree. They were tested against different fixture packages, so a divergence in either adapter would have gone unnoticed until it reached a repository using both. Three layers of parity now: - `an_xcresult_identifier_and_a_junit_classname_key_alike` — the same test keys the same way from either input. The two build `suite` from different places, an identifier's second-to-last component against a classname's innermost, and arrive at `None` by different routes, so only this catches them drifting. - `an_xcresult_url_and_a_junit_classname_name_the_same_target` — the collision tie-break reads the target from `nodeIdentifierURL` on one side and `classname` on the other, and is only correct if they agree. - `parity::both_inputs_resolve_every_test_to_the_same_file` — one package captured both ways, via `swift test --xunit-output` and via `xcodebuild` into an `.xcresult`, asserting the full set of (test, file) pairs is identical. Writing it turned up two things worth having pinned. The real xcresult identifier for a Swift XCTest method is `testOldStyle()`, with parens, where Objective-C reports none — both shapes are now covered rather than the one I assumed. More importantly the two inputs disagree on that name: `xcodebuild` reports `testOldStyle()` and `swift test --xunit-output` reports `testOldStyle`. They resolve to the same file because keying normalises the parens away, but `name` feeds `gen_info_id_base`, so the same test arriving through the two inputs does not currently land on one identity. That is upstream of this crate and is not worked around here, only pinned by `the_two_inputs_spell_an_xctest_method_differently` so it cannot change unnoticed. Comparison is over sorted `(name, file)` pairs rather than a map keyed by name, because two suites in the fixture both declare `shared()` — keyed by name one silently displaced the other and the test failed on roughly half of runs depending on hash order. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two tests can differ only by argument label, and a normalisation that dropped
labels would silently merge them — giving one the other's file, which for
codeowners is the worst outcome available: confidently wrong rather than absent.
Nothing covered that.
`OverloadSuite` declares `check()`, `check(a:)` and `check(b:)`, with `check(b:)`
in a different file via an extension, so a collapse shows up as a wrong file
rather than only a wrong line. Both inputs report all three and agree on how they
spell them:
xcresult OverloadSuite/check() OverloadSuite/check(a:) OverloadSuite/check(b:)
xunit check() check(a:) check(b:)
`normalized_case` trims only trailing parens, so `check()` becomes `check` while
`check(a:)` becomes the unbalanced `check(a:`. That is untidy but correct: the
same function is applied to the language server's symbol name and to the test
identifier, so what matters is that it is identical on both sides rather than
well-formed. Because labels survive it, the three key distinctly and resolve to
their own declarations, which the parity test now covers end to end from both
artifacts.
Rewriting `normalized_case` to split on `(` instead fails the new test with both
labelled overloads resolving to `OverloadA.swift`.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… to key on
An XCTest test method takes no arguments, so there is nothing like `check(a:)` to
separate two of them — every one normalises to a bare method name and the class
name carries the entire load. That is the opposite of the swift-testing overload
case, and it was untested end to end on both inputs.
`BaseTests`, `ChildATests` and `ChildBTests` all report a test called
`testInherited`, distinguished only by class:
MyCLITests.BaseTests testInherited -> BaseTests.swift declares it
MyCLITests.ChildATests testInherited -> BaseTests.swift inherits it
MyCLITests.ChildBTests testInherited -> ChildBTests.swift overrides it
The inherited case is resolved by walking `supertypes`, built from the language
server's superclass parse, and the overriding case never needs the walk because
the subclass declares the method itself. Both inputs spell the identifier the
same way (`ChildATests/testInherited()` against classname `MyCLITests.ChildATests`
plus `testInherited`), so this is covered by the parity test too — which is also
why comparison is over pairs rather than a map, since three tests now share one
name.
`supertypes` had only a seeded-index unit test behind it before this. Disabling
the chain walk fails the `inherited` case alone, leaving `declared` and
`overridden` passing.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
Stacked on #1179, which is stacked on #1178. Review those first; this is one commit on top.
Why
swift test --xunit-outputwrites no file path for any test, and on Linux there is no.xcresultto fall back to — so today a Swift test run there can never be attributed to a file. The declaration index needs nothing from Xcode, so it can answer this too.That makes the addressable surface here arguably larger than the xcresult case:
sourcekit-lspships with the Swift toolchain on Linux, whilexcresulttoolis Xcode-only.The xunit XML carries more than it appears
classnameis the target followed by the dot-qualified suite path, collapsing to the bare target only for a top-level@Test func:Note the two
shared()cases are distinguishable, and a nested suite is fully qualified.Why the existing machinery fits almost unchanged
from_node_identifierdoes this forSuite/Inner/case(); only the separator differslookupalready falls back toTestKey { suite: None, case }(), keep argument labelsnormalized_case, with an existingparameterized_keeps_its_labelscaseParameterised tests need no special handling: swift-testing emits one entry per function, not per argument set, and that single entry is the declaration site we want.
Fixture
A real SwiftPM package plus the XML it actually produced. Its two suites both declare
shared()in different files, so a regression that ignored the suite component would make one borrow the other's — reverting that component fails both tests rather than neither.tests/fixture-src/swift-test-xunit/README.mdhas the shape-by-shape table and the regeneration steps.Scope
This is the resolver only. Nothing reads a JUnit file or writes a
fileattribute back yet, and no flag is wired up. Landing the plumbing is the next PR in the stack.XCTest works too, and needs
--parallelA single
swift testrun writes two files, which is easy to miss:<name>-swift-testing.xml@Test,@Suite)<name>XCTestCasesubclasses)XCTest's form is the same
Module.Typeplus method, minus the():so the same parse resolves it with no special handling —
an_xctest_case_resolves_to_the_class_that_declares_itproves it against the fixture package.The catch: that second file is only written when
--parallelis passed. Without it the XCTest cases still run — the console reports-[MyCLITests.LegacyXCTests testOldStyle] passed— but nothing is emitted for them, so a project omitting--parallelsilently uploads only its swift-testing results. Both files it actually produced are checked in as fixtures.Parity with the xcresult input
Both inputs feed one
TestLocationIndex, and nothing checked they agree — they were tested against different fixture packages. Three layers now:..._key_alike..._name_the_same_targetnodeIdentifierURLon one side andclassnameon the other, and only works if they agreeparity::both_inputs_resolve_every_test_to_the_same_fileswift test --xunit-outputandxcodebuildinto an.xcresult— with the full set of (test, file) pairs identicalIt found an identity divergence
The two inputs disagree on how they name a Swift XCTest method:
They resolve to the same file (keying normalises the parens away), but
namefeedsgen_info_id_base— so the same test arriving through the two inputs does not land on one identity. This is upstream of this crate and is not worked around here, only pinned bythe_two_inputs_spell_an_xctest_method_differentlyso it cannot change unnoticed. Worth deciding on before any repository is told to switch input formats.Also corrected while here: the real xcresult identifier for a Swift XCTest method is
testOldStyle()with parens, where Objective-C reports none. Both shapes are covered now rather than the one I had assumed.🤖 Generated with Claude Code