Skip to content

chore: stop the weekly novnc bump and report coverage on a red build - #380

Merged
KrasimirKralev merged 1 commit into
betafrom
fix/dependabot-and-coverage-report
Aug 12, 2026
Merged

KrasimirKralev merged 1 commit into
betafrom
fix/dependabot-and-coverage-report

Conversation

@KrasimirKralev

@KrasimirKralev KrasimirKralev commented Aug 12, 2026 •

Copy link
Copy Markdown
Contributor

Two small maintenance fixes, both found while verifying that earlier work actually holds.

The novnc pin was being rewritten every week. The manifest pins @novnc/novnc exactly, because 1.7.0 removes the subpath imports this codebase relies on. But that release is a minor, and the bun group only ignores majors β€” so the weekly run kept rewriting the pin and opening a PR that could never pass. Now that one bump, for that one package, is ignored. The comment says when to remove the rule.

Coverage numbers never appeared on a failing build. pr-tests-coverage.yml runs its parse step if: always(), with a comment saying that is precisely so the PR comment can show numbers even on failure β€” but Vitest does not write the summary when a test fails, so the step always took the not-found path. reportOnFailure: true makes the workflow do what it already claimed to do. The gate is unchanged: a failing test still fails the job.

Summary by CodeRabbit

  • Tests

    • Coverage reports are now generated even when test runs include failures.
  • Chores

    • Preserved compatibility with the current noVNC version by preventing unsupported minor updates.

The manifest pins @novnc/novnc exactly because 1.7.0 removes the subpath
imports this codebase uses, but that release is a minor, so the weekly run
rewrote the pin and opened a PR that could only fail. Ignore that one bump
for that one package.

Separately, the coverage summary was not written when a test failed, so the
workflow step that runs on failure specifically to report numbers never had
a file to read.
@KrasimirKralev
KrasimirKralev requested a review from a team as a code owner August 12, 2026 08:46
@coderabbitai

coderabbitai Bot commented Aug 12, 2026 •

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. πŸŽ‰

ℹ️ Recent review info
βš™οΈ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: fc38e1bb-8517-4bd4-a078-6bdc27079fbf

πŸ“₯ Commits

Reviewing files that changed from the base of the PR and between accbc70 and bacdb06.

πŸ“’ Files selected for processing (2)
  • .github/dependabot.yml
  • vitest.config.ts

πŸ“ Walkthrough

Walkthrough

The pull request pins @novnc/novnc minor updates at 1.6.0 and configures Vitest to report coverage after failed test runs.

Changes

Dependency policy

Layer / File(s) Summary
Pin incompatible noVNC minor updates
.github/dependabot.yml
Dependabot ignores semver-minor updates for @novnc/novnc and documents the incompatible 1.7.0 exports.

Coverage reporting

Layer / File(s) Summary
Report coverage after failures
vitest.config.ts
Vitest writes coverage reports when tests fail while preserving the test failure status.

Estimated code review effort: 1 (Trivial) | ~5 minutes

Possibly related PRs

Suggested reviewers: georgik77, yalexx

πŸš₯ Pre-merge checks | βœ… 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description clearly explains both changes and their reasons, but it omits the template's type, testing, and checklist sections. Add the change type, testing commands and results, checklist confirmations, and screenshots or logs if applicable.
βœ… Passed checks (4 passed)
Check name Status Explanation
Title check βœ… Passed The title clearly summarizes both maintenance changes: stopping the noVNC bump and reporting coverage on failed builds.
Docstring Coverage βœ… Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check βœ… Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check βœ… Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
πŸ“ Generate docstrings
  • Create stacked PR
  • Commit on current branch
πŸ§ͺ Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/dependabot-and-coverage-report

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.

@github-actions

Copy link
Copy Markdown

πŸ¦€ ClawReview

Scuttled over to say hello and get you oriented πŸ¦€

Two CI housekeeping fixes. The Dependabot config now ignores minor-version bumps for @novnc/novnc β€” 1.7.0 is a breaking change in minor clothing that drops the subpath imports the codebase relies on, and without this rule the weekly run kept reopening a PR that could never pass. The Vitest config adds reportOnFailure: true so the coverage summary is written even when tests fail, letting the PR comment workflow actually show numbers on a red build as it was already designed to do.

At a glance

  • 🧹 Chore Β· touches GitHub Actions CI config (Dependabot + coverage workflow) and Vitest config
  • Base branch: beta Β· +17 source / +0 tests across 2 files
  • βœ… base beta matches the beta-first convention
  • βœ… conventional PR title

Good to know

  • ℹ️ No runtime or customer-facing code touched β€” pure CI and tooling config.
  • ℹ️ The @novnc/novnc pin guard (src/tests/unit/novnc-pin.test.ts) and a TODO comment in dependabot.yml mark when to revisit the ignore rule.

β€” ClawReview πŸ¦€, scuttling off. General info only β€” see CodeRabbit for the detailed review. Conventions: docs.

@github-actions

github-actions Bot commented Aug 12, 2026 •

Copy link
Copy Markdown

CI Summary

βœ… Tests

  • Result: passed
  • View run
  • Coverage: statements 65.17%, branches 54.15%, functions 63.12%, lines 67.25%

βœ… E2E

βœ… E2E Install

@KrasimirKralev
KrasimirKralev merged commit 32abb41 into beta Aug 12, 2026
10 checks passed
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