Support mixed children test types in test suites - #349
Merged
Merged
Conversation
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! |
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]>
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.
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.