Add shellcheck, SwiftFormat and SwiftLint by tylervick · Pull Request #410 · XCTestHTMLReport/XCTestHTMLReport · GitHub
Skip to content

Add shellcheck, SwiftFormat and SwiftLint - #410

Merged
tylervick merged 2 commits into
mainfrom
lint-workflow
Aug 8, 2026
Merged

tylervick merged 2 commits into
mainfrom
lint-workflow

Conversation

@tylervick

@tylervick tylervick commented Aug 7, 2026 •

Copy link
Copy Markdown
Member

Closes #380 and #382; the CI half of #381. Second of the maintenance-automation batch.

Formatting had already drifted

22 of 41 files failed swiftformat --lint before any of today's work. The .swiftformat config existed with nothing enforcing it. I checked whether I had caused it — the answer is one file (FaultReportingTests.swift, mine) out of 22; the other 21 predate this session.

Applying the config touches 21 files: 155 insertions, 91 deletions. Mechanical and reproducible with swiftformat ..

HTMLTemplates.swift is excluded, deliberately

It wanted 1,200 indent changes alone. The file is 2-space Allman while the config wants 4-space K&R, so reformatting moves the closing """ delimiters of its multiline strings — and in Swift the closing delimiter's indentation determines how much leading whitespace is stripped from every line. That would change the bytes of every generated report.

It also carries a DO NOT EDIT! This file is autogenerated header, so hand-formatting it is wrong regardless. Excluded from both tools.

Verified the reformat is behaviour-preserving

Generated a report before and after and compared. That comparison found something unrelated and worth knowing: the same binary produces different output on consecutive runs.

run A: selectDevice('6CBE1E1A-E712-42AD-BAA8-A179C1823B56')
run B: selectDevice('E2105793-8D30-4944-AF86-2BBC4017C91F')

Five call sites mint a fresh UUID() per run for DOM handles (TestSummary, TargetDevice, Test ×2, Iteration). Harmless for rendering, but reports are not reproducible — which matters for diffing across runs or content-addressed caching. Filed separately; not changed here.

Without that check I would have blamed the formatting for the differing output.

SwiftLint

Scoped correctly it reports 65 warnings / 24 errors; 31 of those are in the excluded HTMLTemplates.swift. (Unscoped it reports 7,285 warnings because it lints .build/checkouts — i.e. your dependencies.)

The config starts green, non-strict:

  • Layout rules SwiftFormat already owns are disabled, so the two tools cannot disagree about the same lines.
  • line_length errors above what a formatter can fix — SwiftFormat wraps at 100 but cannot wrap string literals.
  • todo is off; TODOs are tracked as issues here, not lint failures.

A gate that is switched on already failing gets ignored. This one can be tightened deliberately.

Two genuine violations are fixed rather than suppressed: a manual add-and-assign that should be += (UnattachedFiles.swift), and a test helper whose leading underscore tripped the naming rule (renamed _testAttachmentsExist → assertAttachmentsExist, which says "not a test" more clearly anyway).

shellcheck

prepareTestResults.sh already passes. So this job guards against regression rather than fixing anything.

I should correct something I said when proposing this batch: I claimed shellcheck would have caught two bugs I shipped today. It would not have. The lexicographic sort was Python inside a heredoc, and the set -e + read interaction isn't a standard shellcheck rule.

Also

swift test writes index.html/report.junit into the fixture directory. Those were untracked and produced a swift build warning on every subsequent build (#389). Now gitignored.

Verification

  • swiftformat --lint . → clean
  • swiftlint → 0 errors, 26 warnings
  • swift test → 22 tests, 1 skipped, 0 failures

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Chores

    • Added automated code-quality checks for shell scripts and Swift sources on pushes, pull requests, and manual runs.
    • Added exclusions for generated test-report files and generated templates.
  • Tests

    • Strengthened result-count and attachment assertions.
    • Improved test maintenance by removing unnecessary error declarations and updating test helpers.
  • Refactor

    • Standardized formatting, comments, naming, and file metadata throughout the project without changing runtime behavior.
    • Updated the sample app to use the current Swift application entry-point syntax.

Closes #380, #382; the CI half of #381.

Formatting had drifted: 22 of 41 files failed swiftformat --lint before any
of today's work, so the .swiftformat config existed without anything
enforcing it. Applying it touches 21 files.

HTMLTemplates.swift is excluded from both tools. It carries a DO NOT EDIT
header and is almost entirely multiline string literals whose contents are
the emitted HTML; the file is 2-space Allman while the config wants 4-space
K&R, so reformatting would move the closing string delimiters. In Swift the
closing delimiter's indentation determines how much leading whitespace is
stripped from every line, which would change the bytes of every generated
report.

Verified the reformat is behaviour-preserving by generating a report before
and after. That comparison also turned up something unrelated: the same
binary produces different output on consecutive runs, because five call
sites mint a fresh UUID() per run for DOM handles. Filed separately.

shellcheck already passes on prepareTestResults.sh, so that job is a guard
against regression rather than a cleanup.

SwiftLint starts non-strict with a config that is green today: layout rules
that SwiftFormat already owns are disabled so the two tools cannot disagree,
and thresholds are set so the gate can be tightened deliberately rather than
switched on already failing and then ignored. Two genuine violations are
fixed rather than suppressed — a manual add-and-assign that should be +=,
and a test helper whose leading underscore tripped the naming rule.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Aug 7, 2026 •

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.github/workflows/lint.yml:
- Around line 1-7: Update the workflow-level configuration in lint.yml by adding
a permissions block with contents set to read, ensuring all lint jobs use
least-privilege repository access.
- Line 13: Update both actions/checkout@v4 steps in the workflow to set
persist-credentials to false, ensuring credential persistence is disabled for
each checkout.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 60b107c3-6131-40d2-b9ac-29dd5c90c8d3

📥 Commits

Reviewing files that changed from the base of the PR and between 047d077 and 84aa6b7.

📒 Files selected for processing (26)
  • .github/workflows/lint.yml
  • .gitignore
  • .swiftformat
  • .swiftlint.yml
  • Sources/XCTestHTMLReport/XCTestHtmlReport.swift
  • Sources/XCTestHTMLReportCore/Classes/Helpers/ImageHelpers.swift
  • Sources/XCTestHTMLReportCore/Classes/Helpers/UnattachedFiles.swift
  • Sources/XCTestHTMLReportCore/Classes/Models/Attachment.swift
  • Sources/XCTestHTMLReportCore/Classes/Models/Iteration.swift
  • Sources/XCTestHTMLReportCore/Classes/Models/JUnitReport.swift
  • Sources/XCTestHTMLReportCore/Classes/Models/Run.swift
  • Sources/XCTestHTMLReportCore/Classes/Models/Summary.swift
  • Sources/XCTestHTMLReportCore/Classes/Models/TargetDevice.swift
  • Sources/XCTestHTMLReportCore/Classes/Models/Test.swift
  • Sources/XCTestHTMLReportCore/Classes/Models/TestScreenshotFlow.swift
  • Sources/XCTestHTMLReportCore/Classes/Models/TestSummary.swift
  • Sources/XCTestHTMLReportCore/Classes/Protocols/EmittableOutput.swift
  • Sources/XCTestHTMLReportCore/Classes/Protocols/TestConforming.swift
  • Tests/XCTestHTMLReportTests/CliTests.swift
  • Tests/XCTestHTMLReportTests/CoreTests.swift
  • Tests/XCTestHTMLReportTests/FaultReportingTests.swift
  • Tests/XCTestHTMLReportTests/JUnitReportTests.swift
  • Tests/XCTestHTMLReportTests/TestSupport.swift
  • XCTestHTMLReportSampleApp/SampleApp/AppDelegate.swift
  • XCTestHTMLReportSampleApp/SampleAppUITests/RetryTests.swift
  • XCTestHTMLReportSampleApp/SampleAppUnitTests/SampleAppUnitTests.swift

Comment thread .github/workflows/lint.yml
Comment thread .github/workflows/lint.yml
Both findings from CodeRabbit on #410, both valid.

The repository default workflow permission is write, so these jobs were
inheriting a write-scoped GITHUB_TOKEN they never use. Pinned to
contents: read.

actions/checkout also leaves the token in .git/config by default. Neither
job pushes anything, and the swift job runs third-party code via brew
install, so the token was sitting on disk while unrelated code executed.
persist-credentials: false on both.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@tylervick

Copy link
Copy Markdown
Member Author

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.

Run shellcheck on shell scripts in CI

1 participant