Skip to content

MistDemo: tier-1 unit tests for pure config + command helpers#323

Merged
leogdion merged 1 commit into
v1.0.0-beta.1from
coverage/mistdemo-tier-1-pure-tests
May 11, 2026
Merged

MistDemo: tier-1 unit tests for pure config + command helpers#323
leogdion merged 1 commit into
v1.0.0-beta.1from
coverage/mistdemo-tier-1-pure-tests

Conversation

@leogdion
Copy link
Copy Markdown
Member

@leogdion leogdion commented May 11, 2026

Summary

Stacks on top of #298. Closes part of the patch-coverage gap reported by codecov on that PR (currently 13% of new lines hit, target 25.6%). This PR adds purely additive unit tests — no live CloudKit, no network — for the cheapest slice of the gap.

What's tested

File Coverage adds
AuthTokenConfigTests.swift memberwise init + parsed init via InMemoryProvider, including the missing/empty api.token error path (32 missed lines on PR #298)
UploadAssetConfigTests.swift memberwise init for every field
TestIntegrationConfigTests.swift defaults + custom values
TestPrivateConfigTests.swift defaults + private-DB pinning
QueryCommandTests+ParseFilter.swift full coverage of parseFilter / inferFieldValue / shouldIncludeField / buildComparisonFilter — parameterized over every operator alias and every error path (97 missed lines on PR #298, largely the parser)
CreateCommandTests+GenerateRecordName.swift prefix, three-part format, suffix range, distinctness across 200 calls
DemoErrorsRunnerOutputTests.swift describe(_:) placeholder + echo

Source changes

Minimal seams mirroring the existing DeleteCommand.mapConflict pattern (internal-static for testability — no behavior change):

  • QueryCommand: filter parsing helpers extracted into QueryCommand+FilterParsing.swift; privateinternal static
  • CreateCommand.generateRecordName: privateinternal

What's NOT in this PR

Command bodies that actually call CloudKitService (most of the remaining gap — DemoErrorsRunner, UploadAssetCommand, QueryCommand.execute(), etc.) require a refactor to inject a mock service. That work is tracked separately in a follow-up issue.

Test plan

  • cd Examples/MistDemo && swift test — 901 tests, 290 suites pass
  • swift test at repo root — 481 tests, 142 suites pass
  • Scripts/lint.sh — clean (pre-existing periphery warnings only)
  • mise exec -- swift-format lint — clean

🤖 Generated with Claude Code


Perform an AI-assisted review on CodePeer.com

Adds tests that don't require live CloudKit access — covers the cheapest
slice of the patch-coverage gap on PR #298 (codecov/patch: 13%).

New test files (all @testable, no network):
- Configuration/AuthTokenConfigTests.swift — memberwise init + parsed init via
  InMemoryProvider, including the missing/empty api.token error path
- Configuration/UploadAssetConfigTests.swift — memberwise init for every field
- Configuration/TestIntegrationConfigTests.swift — defaults + custom values
- Configuration/TestPrivateConfigTests.swift — defaults + private-DB pinning
- Commands/QueryCommand/QueryCommandTests+ParseFilter.swift — full coverage
  of parseFilter / inferFieldValue / shouldIncludeField / buildComparisonFilter
  (parameterized over every operator alias and error path)
- Commands/CreateCommand/CreateCommandTests+GenerateRecordName.swift —
  prefix, three-part format, suffix range, distinctness across 200 calls
- Commands/DemoErrorsRunnerOutputTests.swift — describe(_:) placeholder + echo

Source changes are minimal seams that mirror the existing
DeleteCommand.mapConflict pattern (internal-static for testability):
- QueryCommand: filter parsing helpers extracted into
  QueryCommand+FilterParsing.swift; private → internal static
- CreateCommand.generateRecordName: private → internal

901 MistDemo tests pass; lint + swift-format clean.

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

coderabbitai Bot commented May 11, 2026

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 4f164b72-21f6-40ef-b664-2cf0ef173a77

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch coverage/mistdemo-tier-1-pure-tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@claude
Copy link
Copy Markdown

claude Bot commented May 11, 2026

Code Review — PR #323: MistDemo tier-1 unit tests

Overall this is a well-structured, purely additive test PR that follows the established DeleteCommand.mapConflict testability pattern. The source changes are minimal and sensible. A few issues worth addressing before merge:


Potential Bug: inferFieldValue – redundant Int64 parse then cast to Int

File: QueryCommand+FilterParsing.swift:153-160

if let intValue = Int64(string) {
  return .int64(Int(intValue))   // ⚠️ truncates silently on 32-bit
}

FieldValue.int64 takes an Int, not Int64. The Int64(string) parse is needlessly wide — it accepts values in (Int.max, Int64.max] only to silently truncate them on 64-bit (no-op) and overflow on 32-bit. Using Int(string) directly is cleaner and semantically correct:

if let intValue = Int(string) {
  return .int64(intValue)
}

(This code was present before the PR, but the extraction makes it the right moment to fix.)


Minor Bug: in/not_in values are not whitespace-trimmed

File: QueryCommand+FilterParsing.swift:135-143

let values = value.split(separator: ",").map {
  inferFieldValue(String($0))
}

"priority:in:1, 2, 3" produces ["1", " 2", " 3"] — the space-prefixed items fall through to .string. field and operatorString are trimmed; the individual list items should be too:

let values = value.split(separator: ",").map {
  inferFieldValue(String($0).trimmingCharacters(in: .whitespaces))
}

The existing parameterized test uses "priority:in:1,2,3" (no spaces), so this gap isn't caught today.


Test Fragility: threePartFormat breaks for hyphenated record types

File: CreateCommandTests+GenerateRecordName.swift:375-376

let parts = name.split(separator: "-").map(String.init)
#expect(parts.count == 3)

recordType: "My-Article" would produce "my-article-<ts>-<suffix>" — 4 parts, test fails. Consider splitting on the last two hyphens (e.g. by extracting the suffix directly) or using a regex to match ^[a-z0-9-]+-\d+-\d{4}$ rather than counting raw parts.


Style: doc comments describe "what", not "why"

File: QueryCommand+FilterParsing.swift

Per project conventions (CLAUDE.md: "Default to writing no comments. Only add one when the WHY is non-obvious"):

  • /// Parse a single filter expression "field:operator:value" into a QueryFilter — the function name + signature already communicate this.
  • /// Build a QueryFilter from parsed components. — same issue.
  • // Include all fields if no filter specified inside shouldIncludeField — the guard condition reads plainly without the comment.

These should be removed.


Minor: unnecessary async throws on synchronous test bodies

Several tests mark their bodies async throws but the only async call is the MistDemoConfig() setup at the top. The #expect(...) assertions themselves are synchronous. This is fine from a compiler perspective but slightly misleading — worth noting for consistency with future test additions. (Non-blocking.)


Positive observations

  • Parameterized tests over every operator alias give great branch coverage at low maintenance cost.
  • valueWithColons test directly verifies the maxSplits: 2 invariant — exactly the kind of edge case worth locking in.
  • InMemoryProvider approach for config tests is clean and avoids any file/env coupling.
  • The buildComparisonFilter nil-return / buildSpecialFilter throw split is readable and well-tested.
  • The two required source visibility changes (private → internal) are the minimum seam needed — no unnecessary API surface widening.

Summary: Two real bugs worth fixing (Int narrowing, list whitespace), one test fragility, and a few comment cleanup items. The rest is solid.

🤖 Generated with Claude Code

@codecov
Copy link
Copy Markdown

codecov Bot commented May 11, 2026

Codecov Report

❌ Patch coverage is 99.71264% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 69.48%. Comparing base (104d527) to head (1353034).

Files with missing lines Patch % Lines
...mo/Sources/MistDemoKit/Commands/QueryCommand.swift 0.00% 1 Missing ⚠️
Additional details and impacted files
@@                Coverage Diff                @@
##           v1.0.0-beta.1     #323      +/-   ##
=================================================
+ Coverage          67.84%   69.48%   +1.63%     
=================================================
  Files                521      529       +8     
  Lines              14174    14453     +279     
=================================================
+ Hits                9616    10042     +426     
+ Misses              4558     4411     -147     
Flag Coverage Δ
mistdemo-spm-macos 58.65% <99.71%> (+2.66%) ⬆️
mistdemo-swift-6.2-jammy 58.66% <99.71%> (+2.66%) ⬆️
mistdemo-swift-6.2-noble 58.66% <99.71%> (+2.64%) ⬆️
mistdemo-swift-6.3-jammy 58.66% <99.71%> (+2.61%) ⬆️
mistdemo-swift-6.3-noble 58.64% <99.71%> (+2.66%) ⬆️
spm 51.51% <ø> (+0.42%) ⬆️
swift-6.1-jammy 51.11% <ø> (+0.05%) ⬆️
swift-6.1-noble 51.48% <ø> (+0.42%) ⬆️
swift-6.2-jammy 51.17% <ø> (-0.37%) ⬇️
swift-6.2-noble 51.14% <ø> (ø)
swift-6.3-jammy 51.37% <ø> (+0.25%) ⬆️
swift-6.3-noble 51.11% <ø> (-0.57%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@leogdion leogdion merged commit 1223fcb into v1.0.0-beta.1 May 11, 2026
72 checks passed
@leogdion leogdion deleted the coverage/mistdemo-tier-1-pure-tests branch May 11, 2026 15:04
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.

1 participant