ci: add production analysis and smoke checks - #132
Conversation
📝 WalkthroughWalkthroughThe change adds Nix and CMake analysis tooling, scheduled deep-analysis workflows, stronger lint checks, release smoke validation, and documentation. It also updates command-line version handling, UTC date conversion, QML IDs, and a headless Qt test entry point. ChangesAnalysis and validation
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant GitHubActions
participant NixAnalysisShell
participant AnalysisScript
participant BuildTree
participant Reports
GitHubActions->>NixAnalysisShell: Start scheduled or manual analysis
NixAnalysisShell->>AnalysisScript: Run selected analysis script
AnalysisScript->>BuildTree: Configure and build with CMake/Ninja
AnalysisScript->>Reports: Generate analysis or coverage output
Reports-->>GitHubActions: Publish summary or artifact
Possibly related PRs
Suggested labels: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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. Comment |
Greptile SummaryThis PR adds scheduled and on-demand compiler analysis, sanitizer, coverage, automation-lint, QML-lint, and Release smoke checks while correcting several issues those checks exposed.
Confidence Score: 4/5The PR appears safe to merge, with one non-blocking coverage-suite omission worth correcting. The production and CI changes have no established blocking failure, but the new coverage command skips a deterministic cache suite and therefore understates the behavior exercised by its retained reports. Files Needing Attention: scripts/run-coverage.sh
|
| Filename | Overview |
|---|---|
| .github/workflows/deep-analysis.yml | Adds isolated scheduled/on-demand jobs for targeted static analysis, sanitizers, and retained coverage reports. |
| scripts/run-coverage.sh | Builds an instrumented test tree and generates several gcovr reports, but unnecessarily excludes a deterministic cache test. |
| scripts/run-sanitizers.sh | Configures separate address and thread sanitizer trees and runs deliberately scoped test targets. |
| cmake/BloomAnalysis.cmake | Adds validated, mutually exclusive sanitizer and coverage instrumentation options for GCC and Clang. |
| flake.nix | Adds the analysis shell, automation lint, and an offscreen Release CLI smoke check while preserving the default development shell. |
| src/main.cpp | Replaces Qt's conflicting short version option with a long-only version option so -v remains verbose. |
| src/ui/HeroBanner.qml | Gives action rows component-local IDs so aliases resolve without duplicate-ID diagnostics. |
| src/ui/ImageCacheStore.cpp | Removes an ineffective move into QDir::cleanPath without changing cache ownership or threading behavior. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart LR
PR[Normal PR CI] --> Build[Nix Release build]
PR --> Tests[Deterministic tests]
PR --> Lint[QML and automation lint]
PR --> Smoke[Offscreen version/help smoke]
Deep[Scheduled or manual deep analysis] --> Tidy[Targeted clang-tidy]
Deep --> ASAN[ASan and UBSan]
Deep --> TSAN[Targeted TSAN]
Deep --> Coverage[Coverage report and artifact]
Prompt To Fix All With AI
### Issue 1
scripts/run-coverage.sh:30
**Coverage skips deterministic cache tests**
The coverage command excludes `SeriesDetailsCacheTest` even though the workflow is documented as running the deterministic Linux suite. This leaves cache freshness, connection-scoped prefetch handling, and incompatible-cache rejection out of the retained coverage signal.
```suggestion
--exclude-regex '^VisualRegressionTest$'
```
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Reviews (1): Last reviewed commit: "ci: add production analysis and smoke ch..." | Re-trigger Greptile
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/providers/jellyfin/JellyfinCatalogProvider.h (1)
390-400: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winAdd regression coverage for the UTC date boundaries.
Extend the request-contract coverage in
tests/ProviderCatalogTest.cppwithminPremiereDate,maxPremiereDate, andminDateLastSaved. Assert the emittedMinPremiereDate,MaxPremiereDate, andMinDateLastSavedvalues so theQTimeZone::UTCconversion remains covered.🤖 Prompt for 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. In `@src/providers/jellyfin/JellyfinCatalogProvider.h` around lines 390 - 400, Extend the request-contract tests in ProviderCatalogTest to populate minPremiereDate, maxPremiereDate, and minDateLastSaved, then assert the emitted MinPremiereDate, MaxPremiereDate, and MinDateLastSaved query values use the expected UTC start/end-of-day boundaries.
🤖 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 `@scripts/run-coverage.sh`:
- Around line 26-31: In the coverage workflow after the cmake build command and
before ctest, delete all existing .gcda files under BUILD_DIR so persisted GCC
counters cannot be merged into the current run. Keep the existing CTest
invocation and exclusions unchanged.
---
Nitpick comments:
In `@src/providers/jellyfin/JellyfinCatalogProvider.h`:
- Around line 390-400: Extend the request-contract tests in ProviderCatalogTest
to populate minPremiereDate, maxPremiereDate, and minDateLastSaved, then assert
the emitted MinPremiereDate, MaxPremiereDate, and MinDateLastSaved query values
use the expected UTC start/end-of-day boundaries.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 10e61f2d-1994-443a-959a-099d2d0d27fa
📒 Files selected for processing (20)
.clang-tidy.github/workflows/ci.yml.github/workflows/deep-analysis.yml.github/workflows/update-dependencies.yml.gitignoreAGENTS.mdCMakeLists.txtcmake/BloomAnalysis.cmakedocs/build.mdflake.nixnix/qml-lint.nixnix/source.nixscripts/run-clang-tidy.shscripts/run-coverage.shscripts/run-sanitizers.shsrc/main.cppsrc/providers/jellyfin/JellyfinCatalogProvider.hsrc/ui/HeroBanner.qmlsrc/ui/ImageCacheStore.cpptests/ImageCacheStoreTest.cpp
|
Addressed the remaining CodeRabbit review-body nitpick in 27a3ebc: |
1 participant
Summary
-vversion/verbose collision, a misleading non-move in the image store, and deprecated UTC date constructionVerification
./scripts/dev-build.sh --testsVisualRegressionTest: 36/36 passed (18.71s)Bloom 0.8.0; help retains--verbose, -vPlatform scope
The automated smoke validates Linux offscreen Release startup/version reporting only. Live Jellyfin/Silo servers, playback hardware, GPU/HDR/display switching, and Windows runtime playback remain unavailable locally and retain their documented manual validation gates. TSAN is intentionally limited to process/display tests because the system's uninstrumented Qt reports its queued
invokeMethodallocation handoff in the image-cache worker; no broad suppression was added.Documentation
Please confirm the updated testing policy in
AGENTS.mdand detailed analysis/coverage guidance indocs/build.mdaccurately describe the intended CI split.Note
Add production analysis and smoke check workflows to CI
release-smokeandautomation-lintNix checks to both the main ci.yml and update-dependencies.yml workflows; smoke check runsbloom --versionand--helpoffscreen.BLOOM_SANITIZER(address|thread) andBLOOM_ENABLE_COVERAGEbuild options.WarningsAsErrors='*'and restricts analysis tosrc/.--versionCLI flag in src/main.cpp to printBloom <version>and exit without conflicting with-v(verbose).Macroscope summarized 27a3ebc.
Summary by CodeRabbit
New Features
--versioncommand-line option that displays the application version.Bug Fixes
Quality Improvements