[#4] Add an RSpec test suite for the checks - #6
Open
heitor-lassarote wants to merge 19 commits into
Open
Conversation
Problem: lib/serokell_danger/checks/ has no test coverage at all, and there was no RSpec setup to write any. Solution: add rspec to the Gemfile's test group, a spec_helper.rb with the standard Danger plugin testing_dangerfile/testing_ui/testing_env helpers (following the convention used across the Danger plugin ecosystem, verified against danger/danger-plugin-template and danger-rubocop's own specs), a fake GitHub Actions pull_request event fixture instead of copying the ecosystem's usual Travis CI fixture verbatim, and one real passing spec (SerokellDanger::Config) to establish the pattern. Remaining checks (Util, issue-prefix, the Git::Object::Commit monkey-patches, GitHost, and the six check_* methods) are tracked as follow-up work, not done in this commit.
heitor-lassarote
force-pushed
the
heitor-lassarote/#4-add-rspec-tests
branch
from
August 25, 2026 22:48
ef362fb to
34b3a62
Compare
Problem: In our helpers file, we have several methods, for example, `mr_title`, that are aliased to a GitHub counterpart (e.g. `pr_title`). Internally, they will perform calls like `plugin.mr_title.to_s`. But on GitHub, we need to use the "pr" version instead. Solution: Create a private helper `githost_switch` method that allows switching between the GitHub and GitLab equivalents properly and use the methods where we're only using the GitLab methods. Add a CHANGES.md entry, since this is a user-visible bug fix.
heitor-lassarote
force-pushed
the
heitor-lassarote/#4-add-rspec-tests
branch
from
August 26, 2026 15:09
f1ab2cd to
757051c
Compare
Problem: We have no tests checking for trailing whitespaces. Solution: Add a test suite checking that the danger reports fire depending on whether it's a trailing whitespace, final newline, or extra final newlines config. The fixtures backing these tests are deliberately malformed, so exclude spec/fixtures/ from this repo's own check_trailing_whitespace (this repo dogfeeds its own checks against its own PRs). Add spec/support/danger_testing.rb with the shared Dangerfile/git test setup, the fixture-path helper, and the generic danger_report expectation helper that later check specs build on (renamed from the placeholder doubles.rb referenced in spec_helper.rb's comment).
Problem: .xrefcheck.yaml excluded github.com/serokell/danger links because GitHub 404s them for unauthenticated requests while the repo is private, indistinguishable from a genuinely broken link. The repo is now public, so this exclusion would only serve to silently hide real broken links going forward. Solution: remove the exclusion. Verified xrefcheck still passes clean against the now-public repo.
Problem: We have no tests checking for outdated license headers. Solution: Add a test suite covering both rules (check_year, expected_holder), the file-selection knobs (file_extensions, ignore_paths, scan), the skip_if_title_matches skip, and edge cases (no header at all, both rules firing on the same file, an invalid scan value, a tracked file that doesn't exist on disk). The fixtures backing these tests are deliberately malformed, so exclude spec/fixtures/ from this repo's own check_license_headers (this repo dogfeeds its own checks against its own PRs). Read copyright_line_pattern from the real default config instead of duplicating the regex, so the test can't silently drift from what the check actually ships.
Problem: We have no tests checking commit subject/description style. Solution: Add a test suite covering all ten commits-style rules, the wip/fixup/exempt whole-commit exemptions, skip_if_title_matches, and the explain_workarounds markdown note. Extend the shared test helpers with make_commit/set_commits for building fake commits, and generalize set_danger_report_expectations' keyword_params to accept a per-rule Hash, since this check's rules don't all pass the same keyword arguments to danger_report.
Problem: We have no tests checking for merge commits. Solution: Add a test suite covering both detection modes (parent count and subject pattern), aggregating multiple offenders into one report, the invalid detect value error, and skip_if_title_matches. Add make_config_from_default to the shared test helpers, factoring out the "start from the real default config, silence skip_if_title_matches" pattern now shared by commits-style and merge-commits, and give make_commit a parents: param for the merge-detection rule.
Problem: We have no tests checking the merge/pull request title and body. Also, the ArgumentError raised for an invalid `ticket_links.scan` value said "expecs" instead of "expects". Solution: Add a test suite covering title_prefix, all six ticket_links scenarios (title/body scanning, scan restriction, dedup, nil, invalid scan value), the MR/PR context guard, and skip_if_title_matches. Give stub_githost a body: param, since this is the first check that reads the MR/PR body rather than just its title. Fix the typo along the way, and add a CHANGES.md entry for it, since this is a user-visible bug fix.
Problem: We have no tests checking for leftover WIP/fixup commits before merging. Solution: Add a test suite covering both wip_commit_patterns and fixup_commit_patterns firing independently, a clean commit reporting nothing, aggregating multiple offenders into one report, each pattern list being independently nil-able, and skip_if_title_matches.
Problem: We have no tests for SerokellDanger::Util, the pure helper functions the checks build on (as_list, matches_any?/pattern_matches?, path_selected?/path_ignored?, severity_for). Solution: Add a test suite covering each function directly against plain values and Config objects.
heitor-lassarote
force-pushed
the
heitor-lassarote/#4-add-rspec-tests
branch
from
August 26, 2026 19:55
757051c to
465b45d
Compare
9 tasks
Problem: We have no tests for issue-prefix.rb's module functions (valid_issue_prefix?, issue_prefix_separator, strip_issue_prefix, issue_prefix_examples, and the issue_prefix_pattern errors). Solution: Add a test suite covering each recognized prefix kind, extra_patterns, allow_multiple, and the two ArgumentError cases, against plain Hash configs.
heitor-lassarote
force-pushed
the
heitor-lassarote/#4-add-rspec-tests
branch
from
August 26, 2026 20:52
465b45d to
5692fd3
Compare
Problem: We have no tests for the methods monkey-patched onto Git::Object::Commit (subject, subject_ticked, description, blank_line_after_subject?, subject_matches?, short_ref) - every check spec so far has stood these in with a plain double instead. Solution: Add a test suite that builds a real (but offline) Commit by passing parsed commit data eagerly, so the derivation logic itself is exercised rather than a stand-in.
heitor-lassarote
force-pushed
the
heitor-lassarote/#4-add-rspec-tests
branch
from
August 26, 2026 20:59
5692fd3 to
b5fa5e2
Compare
Problem: We have no tests for SerokellDanger::GitHost, which every check goes through indirectly via stub_githost but never exercises directly. Solution: Add a test suite covering .detect, the github?/gitlab? switch, every mr_*/pr_* alias pair, mr_draft?'s three branches, mr_title_payload's draft-stripping, suggestion_fence, and the method_missing/respond_to_missing? plugin delegation, against plugin doubles for both hosts.
Problem: RSpec was set up locally but never wired into CI, so a regression in the test suite itself would go unnoticed. Solution: Add an rspec job to the CI workflow, alongside standardrb.
Problem: #4's acceptance criteria explicitly call out empty diffs as an edge case, but no spec exercised git.diff/git.commits returning no results at all. Solution: Add a "does nothing" test to each check that reads git.diff or git.commits, covering the empty case explicitly.
Problem: This PR introduces the test suite, but neither README.md nor CONTRIBUTING.md says anything about running it (or standardrb/ reuse lint) locally. Solution: Add a "Running tests and lints" section to CONTRIBUTING.md.
Problem: The repo has no test coverage report, and CI has no way to catch a merge that quietly reduces coverage. Solution: Add simplecov to the test group, start it before any production code loads in spec_helper.rb, and set a minimum_coverage floor (90%, a few points below the current 93.36% to leave room for minor fluctuations without needing constant tweaking). Ignore the generated coverage/ directory, and upload it as a CI artifact from the rspec job so the report is actually visible, not just a local side effect.
heitor-lassarote
force-pushed
the
heitor-lassarote/#4-add-rspec-tests
branch
from
August 26, 2026 21:11
b5fa5e2 to
762b517
Compare
Problem: actions/checkout@v4 and fsfe/reuse-action@v1.1 are several majors behind their current releases (v7 and v6 respectively, both verified against the GitHub API). CONTRIBUTING.md's "Running tests and lints" section also didn't mention xrefcheck (which runs in CI alongside the other three checks) or the SimpleCov coverage report just added. Solution: Bump actions/checkout to @v7 and fsfe/reuse-action to @v6. ruby/setup-ruby@v1 and serokell/xrefcheck-action@v1 are left as-is, since both are floating refs the upstream projects already move to track their latest v1.x.y release, not stale pins. Document xrefcheck and the coverage report in CONTRIBUTING.md.
Problem: Every check spec stubs dangerfile.danger_report itself (via set_danger_report_expectations), so its real implementation - severity resolution, the disable-hint suffix, dispatching to fail/warn/message/markdown, and the invalid-severity raise - has never actually run in a test, despite being shared by every check. Solution: Add a dedicated spec exercising danger_report directly against a plain SerokellDanger::Config, covering all four severities, the :off no-op, the custom hint override, the configure_with-less config, extra keyword pass-through, an explicit severity: override, and the unknown-severity raise. This raises coverage from 93.37% to 96.26%, so raise minimum_coverage from 90 to 95 to lock in the gain. Also fix a SimpleCov deprecation warning surfaced while running this (add_filter -> skip).
heitor-lassarote
marked this pull request as ready for review
August 26, 2026 21:37
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Adds an RSpec suite covering all six
check_*methods(
check_trailing_whitespace,check_license_headers,check_commits_style,check_merge_commits,check_premerge_commits,check_merge_request), plus dedicated unit tests for the standalonehelper modules they build on (
SerokellDanger::Util,issue-prefix.rb,SerokellDanger::GitHost, theGit::Object::Commitmonkey-patch).Wires
bundle exec rspecinto CI alongsidestandardrb.Several real bugs in the checks themselves were found and fixed along
the way (see individual commits), not just formalized as known-good
behavior.
Related issue(s)
Fixed #4
Closes #1
✅ Checklist for your Pull Request
Related changes (conditional)
Tests
silently reappearing again.
Documentation
Agent instructions (conditional)
it followed the agent instructions in
.claude/settings.json(plugin) and
.github/copilot-instructions.md.Public contracts
of Public Contracts policy.
and
Stylistic guide (mandatory)