[PM-33574] llm: Add build-test-verify skill#2448
Conversation
github-actions
bot
added
the
t:llm
|
Great job! No new security vulnerabilities introduced in this pull request |
|
Claude finished @SaintPatrck's task in 2m 34s —— View job Overall Assessment: APPROVE Reviewed the extraction of build, test, and troubleshooting content from No findings. |
|
Overall Assessment: APPROVE Reviewed the extraction of build/test/troubleshooting content from CLAUDE.md into a dedicated Code Review Details
Finding 1 --
Details and fixThe regular test command (line 42) correctly includes The source in Suggested fix: RECORD_MODE=1 xcodebuild test \
-workspace Bitwarden.xcworkspace \
-scheme Bitwarden \
-testPlan Bitwarden-Snapshot \
-destination 'platform=iOS Simulator,name=iPhone 16' |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2448 +/- ##
==========================================
- Coverage 86.86% 85.73% -1.14%
==========================================
Files 1841 2074 +233
Lines 162244 177273 +15029
==========================================
+ Hits 140941 151987 +11046
- Misses 21303 25286 +3983 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
SaintPatrck
force-pushed
the
llm/build-test-verify
branch
from
ce11fe5 to
008392a
Compare
fedemkr
left a comment
fedemkr
left a comment
There was a problem hiding this comment.
Looks good, just some improvements. Let me know what you think and I can do the changes, no problem.
| ## Building | ||
|
|
||
| ```bash | ||
| ./Scripts/build.sh project-pm.yml Bitwarden Simulator # PM for simulator | ||
| ./Scripts/build.sh project-bwa.yml Authenticator Simulator # Authenticator for simulator | ||
| ./Scripts/build.sh project-pm.yml Bitwarden Device # PM for device | ||
| ``` |
There was a problem hiding this comment.
🤔 Maybe we should do some testing on this, but is this smart / fast enough on detecting that to build Test Harness for device it needs to use ./Scripts/build.sh project-bwth.yml TestHarness Device? Should the general script be added here? Although maybe it just fetches the docs from build.sh so this may not be necessary.
./Scripts/build.sh project-{productAcronym}.yml {build_scheme} {build_mode}| **Re-record snapshot tests** (when visual changes are intentional): | ||
| ```bash | ||
| RECORD_MODE=1 xcodebuild test \ | ||
| -workspace Bitwarden.xcworkspace \ | ||
| -scheme Bitwarden \ | ||
| -testPlan Bitwarden-Snapshot \ | ||
| -destination 'platform=iOS Simulator,name=iPhone 16' | ||
| ``` |
There was a problem hiding this comment.
| mint run sourcery --config BitwardenKit/Sourcery/sourcery.yml | ||
|
|
||
| # Asset/localization code generation | ||
| mint run swiftgen config run --config swiftgen-pm.yml |
There was a problem hiding this comment.
⛏️ I'd use swiftgen-bwr.yml instead here as it's the most likely place to need asset/localization generation given that a lot of them are shared now in "Bitwarden Resources".
| ## Tooling Reference | ||
|
|
||
| | Tool | Config | Purpose | | ||
| |------|--------|---------| | ||
| | XcodeGen | `project-*.yml` | Generates `.xcodeproj` from YAML specs | | ||
| | Mint | `Mintfile` | Swift tool package manager | | ||
| | SwiftLint | `.swiftlint.yml` | Linting with custom rules | | ||
| | SwiftFormat | `.swiftformat` | Code formatting | | ||
| | Sourcery | `*/Sourcery/sourcery.yml` | Mock generation (`AutoMockable`) | | ||
| | SwiftGen | `swiftgen-*.yml` | Asset/localization code generation | | ||
| | Fastlane | `fastlane/Fastfile` | CI/CD automation | |
There was a problem hiding this comment.
🤔 Should typos be included here as well?
| |---------|-------|-----| | ||
| | `.xcodeproj` not found | Files are gitignored | Run `./Scripts/bootstrap.sh` | | ||
| | `MockXxx` not found | Sourcery not run | Add `// sourcery: AutoMockable`, run Sourcery or build | | ||
| | Snapshot test fails | Simulator mismatch or intentional change | Check `.test-simulator-*` files; re-record if change is intentional | |
There was a problem hiding this comment.
🤔 I'd indicate that these are disabled for the time being and if any snapshot test is run then it should actually disable it by adding disable to the test function so it's transformed to something like disabletest_snapshot_*.
3 participants
🎟️ Tracking
https://bitwarden.atlassian.net/browse/PM-33574
📔 Objective
Consolidates all build, test, lint, and troubleshooting content from CLAUDE.md into a dedicated on-demand skill.
What changed:
skills/build-test-verify/SKILL.md— Complete reference for: initial setup (bootstrap.sh), project generation (XcodeGen specs), build commands, test execution with correct simulator requirements, lint/format/spell check, Sourcery mock generation, SwiftGen asset generation, tooling reference table, common failures table (missing.xcodeproj, missing mocks, snapshot failures, extension memory crashes, SwiftLint TODO warning), and debug tips.CLAUDE.md — Removes Pre-build Scripts subsection, the entire Deployment section (building, initial setup, git hooks, code generation, CI/CD, tooling table), and the entire Troubleshooting section. Replaces ~100 lines with a two-line pointer to the
build-test-verifyskill.