Skip to content

Support mixed children test types in test suites - #349

Merged
tylervick merged 4 commits into
XCTestHTMLReport:mainfrom
kevin-brotcke:main
Jan 23, 2024
Merged

tylervick merged 4 commits into
XCTestHTMLReport:mainfrom
kevin-brotcke:main

Conversation

@kevin-brotcke

@kevin-brotcke kevin-brotcke commented Jan 3, 2024

Copy link
Copy Markdown
Contributor

In the latest Xcode build tools 15.1 and iOS 17.0.1 versions we've been randomly getting some "dangling" test data, meaning they are in a test group a level higher than they should be. This causes the HTML report tool to end early resulting in many missing tests because it assumes since there is a child test there can't be a child test group.

This changes handles the bad data better by checking all children regardless of their type. We'll reach out to Apple support to see if they are already aware of the issue but for now this unblocks us.

I ran the unit tests and manually ran reports on known good and bad data. There is no difference to users not experiencing the issue.

tvick-sc

This comment was marked as duplicate.

@tylervick

Copy link
Copy Markdown
Member

Thank you @kevin-brotcke ! I'll get a release published with this shortly.

If you hear back from Apple please let us know in a new issue or discussion topic!

@tylervick
tylervick merged commit 9229533 into XCTestHTMLReport:main Jan 23, 2024
tothszabi added a commit to bitrise-io/XCTestHTMLReport that referenced this pull request Oct 14, 2024
* Update ci.yaml with xcode 15, 14, 13 and macos-13, replace xcode-select action (XCTestHTMLReport#335)

* Update ci.yaml with xcode 15, 14, 13 and macos-13, replace xcode-select action

* remove xcode 13 from ci since macos-13 builders start with 14

* create iPhone 12

* Update ci.yml

* Update test-artifacts.yml

* Update ci.yml

* Update test-artifacts.yml

* Lazy load screenshots (XCTestHTMLReport#339)

All screenshots are loaded immediately when opening the report even if they aren't in view. When opening a large report hosted remotely containing thousands of screenshots it can take several minutes to load. A quick fix is to enable native image lazy loading which is supported in most popular browsers today.

* Update ci.yml (XCTestHTMLReport#340)

* Update ci.yml

* Create swift.yml (XCTestHTMLReport#341)

* Create swift.yml

* Update and rename swift.yml to test.yml

* Update test.yml

* Update test.yml

* Update test.yml

* Update test.yml

* Update test.yml

* Update test.yml

* Update test.yml

* fix tests ahead of xcode 15 change (XCTestHTMLReport#342)

* remove xcpretty install step

* Add parallel test processing (XCTestHTMLReport#334)

* Update actions for xcode 15 (XCTestHTMLReport#343)

* Update release.yml

* Update codecov.yml (XCTestHTMLReport#344)

* 2.4.1-pre.ca8c215 version bump (XCTestHTMLReport#345)

Co-authored-by: tylervick <[email protected]>

* bump xcresultkit to use xcresulttool version 22007 (XCTestHTMLReport#346)

* 2.4.1

* Update homebrew-bump.yml

* Lazy load video and everything else (XCTestHTMLReport#350)

Co-authored-by: Kevin Brotcke <[email protected]>

* Support mixed children test types in test suites (XCTestHTMLReport#349)

* Support mixed children test types in test suites

* Add verbose logging for test case and group initialization

* Add XCTestHTMLReportSampleApp build folder to gitignore

---------

Co-authored-by: Kevin Brotcke <[email protected]>
Co-authored-by: Tyler Vick <[email protected]>

* 2.4.2 Version Bump

* 2.4.3-pre.4005e15 version bump (XCTestHTMLReport#352)

Co-authored-by: tylervick <[email protected]>

* replace source of XCResultKit from tylervick/XCResultKit.git to davidahouse/XCResultKit.git (XCTestHTMLReport#359)

* 2.5.1-pre.7bbc8db version bump (XCTestHTMLReport#360)

Co-authored-by: tylervick <[email protected]>

* Bump actions/download-artifact from 2 to 4.1.7 in /.github/workflows (XCTestHTMLReport#363)

Bumps [actions/download-artifact](https://github.com/actions/download-artifact) from 2 to 4.1.7.
- [Release notes](https://github.com/actions/download-artifact/releases)
- [Commits](actions/download-artifact@v2...v4.1.7)

---
updated-dependencies:
- dependency-name: actions/download-artifact
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Fix skipped test cases that have no logs  (XCTestHTMLReport#362)

* Add .skipped status to JUnitReport and place the respective tags in the xml output

* Allow testfolder locally

* merge remote branch into local branch

* clean-up

* clean-up

* remove tabs

---------

Co-authored-by: Thomas Maier <[email protected]>
Co-authored-by: tam <[email protected]>
Co-authored-by: merlibird <[email protected]>

* Disable original actions

* Use empy on

* Use manual triggers

* empty to test ci action

* try without xcode select

* use another xcode select

* use xcode version from matrix

* fix param

* add bitrise-release action

* Disable ci for PRs for now

* Update bitrise-release.yml

* Update bitrise-release.yml

* Update bitrise-release.yml

* Fine tune bitrise-release action

* Parallelise test processing

* Remove log tab

* Use lazy loading

* Lazy load with javascript

* Fix

* Refactor screenshots

* Refactor src editing

* Add data-src in template

* Copy src attribute only once

* Fix typo

* Fix paranthesis

* Set 1.0.0 version for bitrise release

* 1.0.1-pre.8512da8 version bump

* Remove archiving from the release flow

* Show failing test tab when there are failing tests

* Fix lazy loading for tail screenshots and videos (#11)

* Update Version.swift

* 1.0.2-pre.c941897 version bump (#14)

Co-authored-by: zsolt-vicze <[email protected]>

* Make src attributes html friendly (#15)

* Release 1.0.2 (#16)

* 1.0.3-pre.95c1ab9 version bump (#17)

Co-authored-by: zsolt-vicze <[email protected]>

* Lazy load videos (#18)

* Release 1.0.3

* 1.0.4-pre.579f509 version bump (#19)

Co-authored-by: zsolt-vicze <[email protected]>

* Disable test job

* Update Version

---------

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: Tyler Vick <[email protected]>
Co-authored-by: Kevin Brotcke <[email protected]>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: tylervick <[email protected]>
Co-authored-by: Kevin Brotcke <[email protected]>
Co-authored-by: juanitos23 <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Thoromis <[email protected]>
Co-authored-by: Thomas Maier <[email protected]>
Co-authored-by: tam <[email protected]>
Co-authored-by: merlibird <[email protected]>
Co-authored-by: zsolt-vicze <[email protected]>
Co-authored-by: zsolt-vicze <[email protected]>
Co-authored-by: zsolt-vicze <[email protected]>
tylervick added a commit that referenced this pull request Aug 11, 2026
Review caught that --json listed four permitted value differences while the
allow-list has three entries, leaving testCase.arguments as an implicit fourth
rule. It is not one: nothing renders arguments, so it cannot appear in the HTML
differential at all and needs no masking rule. Now classified separately as a
model-level capability difference, with the instruction that --json compares it
by asserting legacy is empty rather than asserting the two sides match -- and a
note that until the parameterized @test lands, both sides are empty and a naive
equality assertion would pass vacuously.

Also corrects reasoning I got wrong in the previous commit. Task 5 justified
leaving the orphaned TIME and ITEM_CLASS placeholders by claiming the template
generator is missing and hand-editing is forbidden. The spec already establishes
the opposite: createTemplates.sh was deleted in #295, HTMLTemplates.swift is
hand-maintained, and #349 edited it directly. The outcome is unchanged --
templates stay untouched -- but because the Global Constraints scope this plan
out of report markup and the redesign workstream must first settle which
template copy is the source of truth, not because the edit is impossible.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
tylervick added a commit that referenced this pull request Aug 12, 2026
* docs: design spec for migrating off xcresulttool --legacy (#391)

Records the strategy decision and the evidence behind it: read the new
xcresulttool format directly and drop XCResultKit, behind a reader
abstraction that keeps the legacy path alive until Apple removes it.

The central finding is that the new format is not a superset of the
legacy one. Activity types and finish times, user-supplied attachment
names, attachment UTIs, structured failure locations, and log
emittedOutput all have no new-format equivalent, and exportRecursiveJson
has no replacement at all. Byte-identical output across the two backends
is therefore not achievable, so the bar is a declared and CI-asserted
diff rather than an empty one.

Every measurement in the spec was taken on freshly generated fixtures
under Xcode 26.2 rather than inferred, including two that shape the
plan: reports are not byte-reproducible (18 differing lines on the
smallest fixture, all synthetic UUIDs, identical after one regex), and
the two read paths are equal within noise on a same-runner interleaved
A/B, so no performance claim is made in either direction.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>

* docs: implementation plan for the xcresulttool legacy migration (#391)

Fifteen tasks, each ending in a green test run and a commit, following the
spec's phasing: extract a backend-neutral port, add the modern reader, prove
parity differentially, then flip selection on.

Two ordering decisions carry most of the risk reduction. The UUID normalizer
and a pre-refactor baseline capture land before the large renderer refactor,
so "behaviour-preserving" is something the implementer diffs rather than
asserts. And the differential harness lands with the migration rather than
after it, because it only works while xcresulttool still supports both
formats.

Fixtures are regenerated on every CI run, so checked-in golden HTML is
impossible; every comparison in the plan is between two renders produced
within one run. Both assertions that could pass on empty input are paired
with an explicit non-vacuity guard.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>

* docs: close verification gaps in the migration plan from PR review

The differential test was the weak point. It only checked that declared
markers appeared on legacy and vanished on modern, which says nothing about
lines nobody declared — an undeclared regression would have passed. Checking
the marker strings against HTMLTemplates.swift also showed three of them did
not exist: activity durations render as a bare "(0.00s)" suffix and attachment
names as bare text, with no class to key on.

Replaced with masking: strip exactly the declared losses from both renders and
require what remains to be byte-identical. That is implementable against the
real markup and is a stronger claim than the marker match ever was. Each rule
maps 1:1 to an allow-list entry, and the masked comparison asserts every test
title survives masking so an over-broad mask cannot make it pass vacuously.

Baseline capture no longer skips missing fixtures. It previously continued past
one, which would let Task 5 diff two partial directories and report them
identical — the same vacuous-verification shape.

Also from review: the attachment export leaked its temp directory (a full copy
of every screen recording) on every run; attachment comparison used Set<Data>,
which collapses duplicates; testNodes and testRuns were non-optional and would
throw keyNotFound on a bundle where xcresulttool omits the key; a failed
activities query returned an empty list with no fault, so a visibly gutted
report would exit 0; and Task 5 quoted a stale expected test count.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>

* docs: make the backend flag live and align the reader contract

Three more from review. The --result-reader option was declared on
SummaryOptions but never passed into Summary, so it would have parsed,
validated, and done nothing; the end-to-end check now renders through both
readers and requires the wrapper-group counts to differ, which fails if the
flag is not reaching Summary.

ResultBackend.resolved() only consulted the version string, so an explicit
--result-reader legacy on a post-removal toolchain would have selected a
backend that cannot work. It now demotes to modern with a warning, which is
the degradation rule the spec already stated. The accompanying test is
conditional on toolchain capability rather than asserting .legacy -> .legacy
outright, since that assertion would start failing exactly when the fallback
becomes load-bearing.

The spec listed read() as throwing while the plan returned an optional. Settled
on the optional, matching the existing getInvocationRecord() contract that
Summary.init already guards with a fault, and documented why the sub-level
failures are the ones that needed a new fault kind.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>

* docs: prove the partial-read fault actually reaches the report

The plan recorded .missingActivities when an activities query fails, but
nothing asserted the fault reaches summary.faults. Since a failed query
degrades to an empty activity list rather than aborting, unproven plumbing
means the CLI could exit 0 on a report whose tests have no activities — the
exact outcome the fault exists to prevent.

Added an XCResultToolInvoking seam so a client that fails only on `activities`
can be injected, and a test that reads through it and asserts both halves: the
read still succeeds, and the fault lands on the caller's collector. The
collector is the one Summary.init owns, so the existing exit-3 path covers the
rest.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>

* docs: source failure text from the node that keeps file and line

Three more from review, two of them substantive.

The modern reader was sourcing failure text from the activities document. The
tests tree's Failure Message nodes are strictly better: measured on
TestResults, they give "FirstSuite.swift:66: XCTAssertTrue failed - Test
failed" where the activity title gives only "XCTAssertTrue failed - Test
failed". The plan was reading from the lossier of two available sources and
the spec described that loss as unavoidable. Both corrected; skip reasons ride
the same node.

The modern reader also collapsed every destination to devices.first, where
legacy emits one run per ActionRecord. Every fixture boots a single simulator,
so no test would have caught the difference — and the differential's zip()
truncates to the shorter sequence, so it would have compared the runs that did
exist and passed. Now one run per device, with an explicit run-count assertion
before the zip, and the coverage gap stated in both documents rather than
implied to be tested.

Also: the legacy-capability probe never drained stderr, which deadlocks if the
pipe fills — the same bug the codebase already documents in TestSupport and
XCResultToolClient.run.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>

* docs: retarget the migration at 4.0 and rebase it onto #430

Four amendments to the design spec and implementation plan, none of which
change the strategy.

**Milestone.** Both documents targeted 3.0, inherited from #391's label.
3.0.0 shipped 2026-08-07, so the breaking changes here — the `--json` schema
and the declared output diff — land in 4.0. Notes that the report redesign is
a sibling workstream in that same major rather than a later release, since
`activityType`'s removal breaks the visual contract either way, and that the
two are strictly sequenced: templates stay frozen until the differential is
proven.

**#430 replaces Task 1's normalizer, but does not retire it.** Task 1 had
rediscovered #411 independently and solved it in the harness with a UUID
regex. #430 fixes it in the product via `IdentifierPath`. The subtlety worth
recording: identifiers are a digest of each element's *structural path*, and
the two backends disagree on structure — the modern tree drops the "All tests"
and "<bundle>.xctest" wrapper levels — so cross-backend identifiers still
diverge. The normalizer survives, retargeted from RFC-4122 to `[0-9a-f]{32}`;
a regex left matching UUIDs would silently match nothing and the differential
would compare raw digests and fail on every run.

Task 1 also collided with #430 on `ReproducibilityTests.swift`. It now appends
to that file instead of creating it, and #430 is a stated prerequisite.

**Task 2 captures raw renders.** Same-backend renders are byte-identical after
#430, so normalizing the baseline is unnecessary — and harmful: Task 5 moves
the renderer onto `ParsedResult`, and a refactor that perturbed the tree would
move every affected digest, which a normalized baseline would hide. Phase 1's
gate is correspondingly strengthened to exact equality with no normalization.

**HTMLTemplates.swift is not generated.** Both documents asserted it was.
Its `DO NOT EDIT … autogenerated by createTemplates.sh` header is stale —
that script was deleted in #295 — and both linters exempt the file on that
basis, which is how `HTML/*.html` drifted 28 hunks behind while the "generated"
file was hand-edited. No consequence for this work, which touches neither; it
is a live trap for the redesign workstream.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>

* docs: gate ParsedResult behind an information-model decision (Task 2.5)

`ParsedResult` is the one artifact this migration could build twice. Shaped so
the current templates render unchanged, it is not backend-neutral — it is
legacy-shaped, and `ModernResultReader` spends its life supplying nil for
fields that exist only because the old UI reads them. The proposed model
already shows the pattern: `activityType`, `finish`, `name`, and
`uniformTypeIdentifier` are each documented as "nil on the modern backend",
and `statusRawValue` would have the modern reader emit legacy spellings it
never saw.

Adds Task 2.5 between the baseline capture and the model: a decision task, not
a code task, with eight questions and a recommended answer for each. The
redesign's visual work stays a sibling workstream and does not gate this one —
but its information model is exactly what the port encodes, and that is an
afternoon rather than a design phase.

Every answer either removes a field from the port or an entry from the
differential allow-list, because holding the legacy backend down to the modern
backend's capability makes the two agree and an unmasked diff proves more than
a masked one. On the recommended defaults, `activityTypeClasses` and
`durations` leave the allow-list entirely: with no `finish` and no
`activityType` in the model, there is no divergence left to mask. The cost is
one-way and stated — the legacy backend stops rendering some things it could
have — but that is a 4.0 behaviour change made once and visible in the model.

Two answers go the other way for reasons worth keeping: Swift Testing
`Arguments` is added now because it is a reshape *inside* the tree and
expensive to retrofit, while insights and metrics are left out because they
attach at the top level beside `runs` and are cheap to add when something
renders them.

Also records the rule the task exists to enforce: no reader code whose only
purpose is to satisfy the render-level diff. The current templates are a
verification scaffold with a retirement date, not a compatibility target.

Updates both #430 prerequisite notes — merged 2026-08-11 as a28b131.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>

* docs: settle the information model before writing ParsedResult

Applies Task 2.5's eight recommendations as written and propagates them
through the port, the readers, the renderer task, and the differential.

Three fields leave ParsedResult (activity finish, activity type, attachment
UTI), one arrives (Swift Testing arguments), one is retyped (status raw string
becomes a neutral enum), and ObjectClass is deleted rather than threaded
through. The direction is the point: each removal makes the two backends agree
by construction instead of by mask, so the differential allow-list drops from
five entries to three. An unmasked diff proves more than a masked one, and the
masked region is exactly where a regression can hide.

Two constraints surfaced while applying the answers, both recorded rather than
assumed away. Answer 6's `arguments` is unexercised: `Arguments` is in the
published TestNodeType enum, but SwiftTestingSuite has no parameterized case
and all three fixtures contain zero such nodes, so Task 8 gains a step that
adds one and a test that fails until it lands. Answer 4 cannot use
UTType(filenameExtension:), which is macOS 11+ against a 10.15 floor, so the
mapping keeps an explicit table.

Rebasing onto #430 also exposed stale guidance in Task 5: it had Activity.uuid
becoming UUID().uuidString, which would compile and silently undo the
reproducibility #430 just established. Activity now mints from IdentifierPath
like every other model.

Removing the activity-type and duration fields leaves two template
placeholders fed with empty strings rather than deleted, because
createTemplates.sh — the generator named in HTMLTemplates.swift's own
DO-NOT-EDIT header — is not in this repository. That leaves a cosmetic empty
paren on activity rows, noted for the redesign workstream, and is still better
than rendering a fabricated (0.00s).

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>

* docs: scope backend demotion, name the surviving allow-list entries, require a --json contract

Three findings from review, all against the spec.

The demotion rule was written as "any hard failure of a legacy command demotes
to modern", which is both broader than the plan implements and a hazard: a
corrupt bundle or permission error would silently retry on the modern reader
and produce a partial report where a clear failure belonged. Demotion is now
scoped to capability detection only, with everything else propagating through
FaultCollector to the exit-3 path.

The spec claimed the allow-list drops from five entries to three without saying
which three. Since this record is what Task 12 is read against, it now names
them with what still differs and which fixture exercises each, plus the
standing instruction to prefer deleting a field from the port over adding an
entry.

--json was specified only as "our schema". ParsedResult is an internal Swift
model, so deriving public output from a synthesized Encodable would make every
later field rename a silent breaking change. Task 14 gains a step requiring the
wire contract be written first -- field names, enum encoding, null-versus-
omitted, units, ordering, and a schema version -- with the encoder made to
match the document rather than the reverse.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>

* docs: make backend detection tri-state and stop substituting an explicit legacy reader

Review caught a vacuous-verification hazard I had introduced. Making an
explicit --result-reader legacy fall back to modern meant a modern-only host
would run the modern reader twice and the differential would compare a backend
against itself and report parity -- the exact failure that suite exists to
prevent.

Detection is now tri-state (available / unavailable / unknown) and only `auto`
ever substitutes. An explicit legacy request that cannot be honoured is an
error; an unparseable version string is `unknown`, which degrades `auto` to
modern but lets an explicit legacy attempt proceed, since a string we cannot
read is not proof the commands are gone. requireBothBackends now asserts on the
backend it actually resolved rather than trusting the request.

--json parity was contradictory: the spec promised output "identical on both
backends" while the same document preserves three render-level differences.
Restated as schema identity -- same field names, nesting, enum encoding and
schemaVersion -- with the four permitted value differences enumerated and
everything else declared a reader bug. The accompanying test compared only
top-level keys, which cannot see a nested field present on one side; it now
compares full key paths.

Log write failures recorded no fault in either provider, so a report missing a
log it had successfully read would still exit 0. Both now record
.logExportFailed. The legacy one is pre-existing rather than introduced here,
but leaving one of two implementations silent is how that asymmetry survives.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>

* docs: separate the arguments capability difference from the allow-list

Review caught that --json listed four permitted value differences while the
allow-list has three entries, leaving testCase.arguments as an implicit fourth
rule. It is not one: nothing renders arguments, so it cannot appear in the HTML
differential at all and needs no masking rule. Now classified separately as a
model-level capability difference, with the instruction that --json compares it
by asserting legacy is empty rather than asserting the two sides match -- and a
note that until the parameterized @test lands, both sides are empty and a naive
equality assertion would pass vacuously.

Also corrects reasoning I got wrong in the previous commit. Task 5 justified
leaving the orphaned TIME and ITEM_CLASS placeholders by claiming the template
generator is missing and hand-editing is forbidden. The spec already establishes
the opposite: createTemplates.sh was deleted in #295, HTMLTemplates.swift is
hand-maintained, and #349 edited it directly. The outcome is unchanged --
templates stay untouched -- but because the Global Constraints scope this plan
out of report markup and the redesign workstream must first settle which
template copy is the source of truth, not because the edit is impossible.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>

* docs: correct two Task 2.5 answers and re-derive the propagation

An xhigh multi-agent review found 30 distinct defects in #435, most of them
downstream of two wrong decisions. Correcting the decisions and re-deriving,
rather than patching the symptoms.

Decision 3 becomes replace-not-delete. Deleting ObjectClass empties ITEM_CLASS
and renders <div class=" failed"> on every row, which breaks the report's own
"show only failures" filter (showElementsWithSelector on .test-summary.failed
and siblings), group expand/collapse (querySelectorAll on .test-summary-group),
the stylesheet rules keyed on both classes, and four test call sites -- one in
CoreTests and three in ReproducibilityTests. The raw IDESchemeActionTest*
values were the legacy part and still go; the emitted class names are the
report's own contract and stay, behind a neutral renderer-side NodeKind. The
port needs no field, since ParsedNode already distinguishes group from case.

Decision 1 still removes the field, with two corrections. `start` is now named
as the replacement ordering key: the sort it fed interleaves failure rows among
activities so a failure renders where it occurred, and deleting it rather than
re-keying it would silently append every failure to the end. And the `durations`
allow-list entry is restored -- deleting it assumed removing `finish` removed
all duration divergence, but the surviving divergence is in group durations.
Verified: durationInSeconds is null on every Test Suite, Test Plan and test
bundle node in all three fixtures, while legacy reports FirstSuite 0.699s,
SecondSuite 0.126s, ThirdSuite 0.132s, SampleAppUnitTests 0.213s. wrapperGroups
only drops wrapper lines, so real suite headings were diverging unmasked.

Task 5 is split. It conflated a pure refactor with mandated behaviour changes
and then demanded a byte-identical gate, so the gate could only ever be waived
-- leaving the migration's largest refactor with no behaviour check. 5a moves
the renderer onto ParsedResult and must be byte-identical; 5b applies the
decisions that change output against an enumerated three-shape diff.

Also fixed, from the same review: failure rows were double-counted on modern
because both documents describe the same failure; the failureTitlePrefix mask
stripped only the legacy shape so every failing test still differed after
masking; read() handed every device the same testables array; an empty devices
list returned a non-nil empty result that exited 0 where legacy exits 3;
PayloadProviding omitted three members the call sites use, including the
downsize path from #428; Task 5 deleted three accessors LegacyResultReader
calls; Summary.init dropped the resultIndex/actionIndex seeding #430 needs;
ValidationError was thrown from a non-throwing init in a target without
ArgumentParser; legacyCapability returned false where the type wanted .unknown;
run??.activities double-chained a flattened optional; and Task 1 called a
render helper that does not exist.

Two refinements beyond the brief. The durations mask is necessarily
over-broad -- the duration sits on a different line from the group class, so it
cannot be scoped -- which also hides XCTest case durations that do agree, so a
targeted model-level assertion restores that coverage. And the empty-read rule
is stated for both readers rather than fixed only on the modern one, since
legacy has the identical shape.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>

---------

Co-authored-by: Claude Opus 5 (1M context) <[email protected]>
tylervick added a commit that referenced this pull request Aug 13, 2026
…of truth (#434) (#452)

* Delete stale HTML/ copy; HTMLTemplates.swift is the template source of truth (#434)

Tyler's ruling on #434: option 1 — delete the stale copy. The HTML/
directory was excluded from the build, 28 diff hunks behind the compiled
templates, and last touched (#350) one day before #349 hand-edited the
"generated" file it supposedly generated.

- Delete Sources/XCTestHTMLReportCore/HTML/ and the Package.swift exclude.
- Replace the false DO NOT EDIT header (createTemplates.sh was deleted in
  #295) with one declaring the file the hand-maintained source of truth.
- Keep both lint exemptions with corrected justifications (see PR).
- Fix .githooks/pre-commit: SwiftLint ignores the config excluded: list
  for explicitly passed paths, so the first-ever staged edit to
  HTMLTemplates.swift failed the hook on rules the config exempts; add
  --force-exclude to match CI behavior.

Zero rendered-output change: only the header comment changed in
HTMLTemplates.swift. Full suite 100/100 (2 pre-existing skips), incl.
DifferentialTests 6/6 and ReproducibilityTests 7/7.

Co-Authored-By: Claude Fable 5 <[email protected]>

* pre-commit: tolerate the all-staged-Swift-files-excluded case (review fix)

When HTMLTemplates.swift is the only staged Swift file, --force-exclude
leaves SwiftLint zero lintable paths and it exits 1 with "No lintable
files found", so the hook still blocked exactly the template-only
commits #434 set out to unblock. Treat that specific outcome as success:
zero lintable files means zero possible violations, so it cannot mask a
real lint failure. Any other nonzero exit still fails the hook.

Verified in a scratch worktree: HTMLTemplates.swift-only staging exits 0;
a staged 259-char-line Swift file still fails (alone and alongside the
excluded file); non-Swift-only staging exits 0. shellcheck clean.

Co-Authored-By: Claude Fable 5 <[email protected]>

---------

Co-authored-by: Claude Fable 5 <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants