From 836ca5d4f7d2619ebbf490d1b1f8f9fe3e38ed51 Mon Sep 17 00:00:00 2001 From: Heitor Toledo Lassarote de Paula Date: Thu, 20 Aug 2026 20:30:11 -0300 Subject: [PATCH 01/19] [#4] Add RSpec test infrastructure and a first spec 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. --- .rspec | 7 ++++ Gemfile | 4 ++ REUSE.toml | 6 +++ spec/fixtures/pull_request_event.json | 6 +++ spec/serokell_danger/config_spec.rb | 60 +++++++++++++++++++++++++++ spec/spec_helper.rb | 43 +++++++++++++++++++ 6 files changed, 126 insertions(+) create mode 100644 .rspec create mode 100644 spec/fixtures/pull_request_event.json create mode 100644 spec/serokell_danger/config_spec.rb create mode 100644 spec/spec_helper.rb diff --git a/.rspec b/.rspec new file mode 100644 index 0000000..dae8c18 --- /dev/null +++ b/.rspec @@ -0,0 +1,7 @@ +# SPDX-FileCopyrightText: 2026 Serokell +# +# SPDX-License-Identifier: MPL-2.0 + +--require spec_helper +--format documentation +--color diff --git a/Gemfile b/Gemfile index 3bc8236..a412bbe 100644 --- a/Gemfile +++ b/Gemfile @@ -10,3 +10,7 @@ group :development do gem "ruby-lsp" gem "standard" end + +group :test do + gem "rspec" +end diff --git a/REUSE.toml b/REUSE.toml index eb30b97..293eef4 100644 --- a/REUSE.toml +++ b/REUSE.toml @@ -11,3 +11,9 @@ path = ".github/**" precedence = "aggregate" SPDX-FileCopyrightText = "2026 Serokell " SPDX-License-Identifier = "CC0-1.0" + +[[annotations]] +path = "spec/fixtures/pull_request_event.json" +precedence = "aggregate" +SPDX-FileCopyrightText = "2026 Serokell " +SPDX-License-Identifier = "CC0-1.0" diff --git a/spec/fixtures/pull_request_event.json b/spec/fixtures/pull_request_event.json new file mode 100644 index 0000000..49bee15 --- /dev/null +++ b/spec/fixtures/pull_request_event.json @@ -0,0 +1,6 @@ +{ + "number": 42, + "repository": { + "clone_url": "https://github.com/serokell/danger.git" + } +} diff --git a/spec/serokell_danger/config_spec.rb b/spec/serokell_danger/config_spec.rb new file mode 100644 index 0000000..43ab93a --- /dev/null +++ b/spec/serokell_danger/config_spec.rb @@ -0,0 +1,60 @@ +# SPDX-FileCopyrightText: 2026 Serokell +# +# SPDX-License-Identifier: MPL-2.0 + +RSpec.describe SerokellDanger::Config do + let(:config) do + described_class.new("example-check", {severity: :warn, max_length: 80}) + end + + describe "#[]" do + it "returns the configured value for a known key" do + expect(config[:severity]).to eq(:warn) + end + + it "accepts a string key, same as the symbol" do + expect(config["severity"]).to eq(:warn) + end + + it "raises ArgumentError for an unknown key" do + expect { config[:does_not_exist] }.to raise_error(ArgumentError, /Unknown option/) + end + end + + describe "method_missing dot-access" do + it "resolves a known key the same way [] does" do + expect(config.severity).to eq(:warn) + end + + it "still raises NoMethodError for something that isn't a key at all" do + expect { config.not_a_real_method }.to raise_error(NoMethodError) + end + end + + describe "#merge (aliased as #with)" do + it "returns a new Config with the override applied" do + overridden = config.merge(severity: :error) + expect(overridden[:severity]).to eq(:error) + end + + it "does not mutate the original config" do + config.merge(severity: :error) + expect(config[:severity]).to eq(:warn) + end + + it "leaves keys it doesn't override untouched" do + overridden = config.with(severity: :error) + expect(overridden[:max_length]).to eq(80) + end + + it "raises ArgumentError when overriding an unknown key" do + expect { config.merge(unknown_key: 1) }.to raise_error(ArgumentError, /Unknown option/) + end + end + + describe "#to_h" do + it "returns a plain hash of the configured values" do + expect(config.to_h).to eq({severity: :warn, max_length: 80}) + end + end +end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb new file mode 100644 index 0000000..55b2be1 --- /dev/null +++ b/spec/spec_helper.rb @@ -0,0 +1,43 @@ +# SPDX-FileCopyrightText: 2026 Serokell +# +# SPDX-License-Identifier: MPL-2.0 + +require "danger" +require "serokell_danger" + +RSpec.configure do |config| + config.color = true +end + +# A silent UI, following the convention used across the Danger plugin +# ecosystem (see danger/danger-plugin-template's spec_helper.rb). +def testing_ui + output = StringIO.new + def output.winsize + [20, 9999] + end + Cork::Board.new(out: output) +end + +# A fake GitHub Actions environment, matching the CI this repo actually +# runs on. Danger reads GITHUB_EVENT_PATH as a real JSON file containing +# the pull_request webhook payload; see spec/fixtures/pull_request_event.json. +def testing_env + { + "GITHUB_ACTION" => "danger", + "GITHUB_EVENT_NAME" => "pull_request", + "GITHUB_REPOSITORY" => "serokell/danger", + "GITHUB_EVENT_PATH" => File.join(__dir__, "fixtures", "pull_request_event.json"), + "DANGER_GITHUB_API_TOKEN" => "faketoken" + } +end + +# A real Danger::Dangerfile, with our checks monkey-patched onto it via +# `require "serokell_danger"` above. git/github access still needs +# stubbing per-example (see spec/support/doubles.rb). +def testing_dangerfile + env = Danger::EnvironmentManager.new(testing_env) + Danger::Dangerfile.new(env, testing_ui) +end + +Dir[File.join(__dir__, "support", "**", "*.rb")].each { |f| require f } From 1f75752aa2b819f6c6b21a122851943804a59f44 Mon Sep 17 00:00:00 2001 From: Heitor Toledo Lassarote de Paula Date: Fri, 21 Aug 2026 18:41:46 -0300 Subject: [PATCH 02/19] [#4] Fix "mr" prefixes being used with GitHub 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. --- CHANGES.md | 19 +++++++++++++++++++ lib/serokell_danger/helpers.rb | 11 ++++++++--- 2 files changed, 27 insertions(+), 3 deletions(-) create mode 100644 CHANGES.md diff --git a/CHANGES.md b/CHANGES.md new file mode 100644 index 0000000..0d8551f --- /dev/null +++ b/CHANGES.md @@ -0,0 +1,19 @@ + + +# Changelog + +All notable changes to this project will be documented in this file. + +The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). + +## [Unreleased] + +### Fixed + +- `check_merge_request` no longer calls the GitHub-only `mr_title`/`mr_body` + methods against a GitHub PR, which raised instead of reading the PR's + title/body (#6). diff --git a/lib/serokell_danger/helpers.rb b/lib/serokell_danger/helpers.rb index e4c3328..b63c2c1 100644 --- a/lib/serokell_danger/helpers.rb +++ b/lib/serokell_danger/helpers.rb @@ -33,17 +33,17 @@ def gitlab? end def mr_title - plugin.mr_title.to_s + githost_switch(:pr_title, :mr_title).to_s end alias_method :pr_title, :mr_title def mr_body - plugin.mr_body.to_s + githost_switch(:pr_body, :mr_body).to_s end alias_method :pr_body, :mr_body def mr_json - plugin.mr_json + githost_switch(:pr_json, :mr_json) end alias_method :pr_json, :mr_json @@ -106,6 +106,11 @@ def respond_to_missing?(name, include_private = false) private + def githost_switch(on_github, on_gitlab, *args, **kwargs, &block) + return plugin.public_send(on_github, *args, **kwargs, &block) if github? && plugin.respond_to?(on_github) + plugin.public_send(on_gitlab, *args, **kwargs, &block) + end + def json_field(gitlab_key, github_key) json = mr_json key = gitlab? ? gitlab_key : github_key From 6bad59b74bf277ef4b515b8a592836ce0a99db84 Mon Sep 17 00:00:00 2001 From: Heitor Toledo Lassarote de Paula Date: Fri, 21 Aug 2026 18:44:43 -0300 Subject: [PATCH 03/19] [#4] Add trailing whitespaces tests 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). --- Dangerfile | 4 +- spec/fixtures/NoFinalNewline.hs | 7 + .../TrailingWhitespaceExtraFinalNewlines.hs | 8 + .../trailing_whitespaces_spec.rb | 142 ++++++++++++++++++ spec/spec_helper.rb | 2 +- spec/support/danger_testing.rb | 62 ++++++++ 6 files changed, 223 insertions(+), 2 deletions(-) create mode 100644 spec/fixtures/NoFinalNewline.hs create mode 100644 spec/fixtures/TrailingWhitespaceExtraFinalNewlines.hs create mode 100644 spec/serokell_danger/trailing_whitespaces_spec.rb create mode 100644 spec/support/danger_testing.rb diff --git a/Dangerfile b/Dangerfile index 6b810c9..ce27f4b 100644 --- a/Dangerfile +++ b/Dangerfile @@ -10,5 +10,7 @@ check_merge_request check_merge_commits check_license_headers check_trailing_whitespace( - trailing_whitespace_default_config.with(ignore_paths: ["LICENSE", "LICENSES/"]) + trailing_whitespace_default_config.with( + ignore_paths: ["LICENSE", "LICENSES/", "spec/fixtures/"] + ) ) diff --git a/spec/fixtures/NoFinalNewline.hs b/spec/fixtures/NoFinalNewline.hs new file mode 100644 index 0000000..fb3d396 --- /dev/null +++ b/spec/fixtures/NoFinalNewline.hs @@ -0,0 +1,7 @@ +-- SPDX-FileCopyrightText: 2026 Serokell +-- +-- SPDX-License-Identifier: MPL-2.0 +module NoFinalNewline (main) where + +main :: IO () +main = putStrLn "Hello, world!" \ No newline at end of file diff --git a/spec/fixtures/TrailingWhitespaceExtraFinalNewlines.hs b/spec/fixtures/TrailingWhitespaceExtraFinalNewlines.hs new file mode 100644 index 0000000..ff725c3 --- /dev/null +++ b/spec/fixtures/TrailingWhitespaceExtraFinalNewlines.hs @@ -0,0 +1,8 @@ +-- SPDX-FileCopyrightText: 2026 Serokell +-- +-- SPDX-License-Identifier: MPL-2.0 +module TrailingWhitespaceExtraFinalNewlines (main) where + +main :: IO () +main = putStrLn "Hello, world!" + diff --git a/spec/serokell_danger/trailing_whitespaces_spec.rb b/spec/serokell_danger/trailing_whitespaces_spec.rb new file mode 100644 index 0000000..c18a726 --- /dev/null +++ b/spec/serokell_danger/trailing_whitespaces_spec.rb @@ -0,0 +1,142 @@ +# SPDX-FileCopyrightText: 2026 Serokell +# +# SPDX-License-Identifier: MPL-2.0 + +require_relative "../../lib/serokell_danger/checks/trailing-whitespaces" + +RSpec.describe "trailing-whitespace check" do + include_context "danger testing" + + let(:trailing_whitespace_extra_final_newlines_path) do + make_fixture_path("TrailingWhitespaceExtraFinalNewlines.hs") + end + let(:no_final_newline_path) { make_fixture_path("NoFinalNewline.hs") } + + def make_config(rules, file_extensions: nil, ignore_paths: [], + skip_if_title_matches: nil) + config = + { + trailing_whitespace: false, + final_newline: false, + extra_final_newlines: false, + file_extensions: file_extensions, + ignore_paths: ignore_paths, + severity: :fail, + skip_if_title_matches: skip_if_title_matches + } + config = rules.reduce(config) { |acc, rule| acc.merge({rule => true}) } + SerokellDanger::Config.new( + "trailing-whitespace", + config, + configure_with: "check_trailing_whitespace" + ) + end + + def set_rule_receive_expectations(rules_to_test) + set_danger_report_expectations( + [:trailing_whitespace, :final_newline, :extra_final_newlines], + rules_to_test + ) + end + + # `enable` defaults to the rules under test, but a test that needs a rule + # turned on without expecting it to fire (e.g. "ignores binary files", + # which must prove the binary-skip happens before the rule check even + # runs) can pass its own list. + def simple_test(fixture, rules_to_test, enable: rules_to_test.keys, + binary: false, **config_overrides) + config = make_config(enable, **config_overrides) + set_diff(fixture, binary: binary) + set_rule_receive_expectations(rules_to_test) + yield if block_given? + dangerfile.check_trailing_whitespace(config) + end + + it "warns about a file with trailing whitespaces" do + simple_test( + trailing_whitespace_extra_final_newlines_path, + {trailing_whitespace: {}} + ) + end + + it "warns about a file with no final newline" do + simple_test(no_final_newline_path, {final_newline: {}}) + end + + it "warns about a file with extra final newlines" do + simple_test( + trailing_whitespace_extra_final_newlines_path, + {extra_final_newlines: {}} + ) + end + + it "suggests removing exactly the extra newlines" do + # The fixture has a single extra blank line at the end (8 lines total), + # so the suggested diff should be empty (removing that one line) and + # anchored at line 8. + simple_test( + trailing_whitespace_extra_final_newlines_path, + {extra_final_newlines: {}} + ) do + expect(dangerfile).to receive(:markdown).with( + "Extra newline at the end of the file.\n```suggestion\n```\n", + file: trailing_whitespace_extra_final_newlines_path, line: 8 + ) + end + end + + it "ignores binary files" do + simple_test( + no_final_newline_path, + {}, + enable: [:final_newline], + binary: true + ) + end + + it "ignores files in ignore_paths" do + simple_test( + no_final_newline_path, + {}, + enable: [:final_newline], + ignore_paths: [no_final_newline_path] + ) + end + + it "runs with files in file_extensions" do + simple_test( + no_final_newline_path, + {final_newline: {}}, + file_extensions: [".hs"] + ) + end + + it "does not run with files not in file_extensions" do + simple_test( + no_final_newline_path, + {}, + enable: [:final_newline], + file_extensions: [".rb"] + ) + end + + it "does not run if the title matches a pattern (GitHub)" do + stub_githost(:github, title: "Merge branch 'foo' into 'main'") + simple_test( + no_final_newline_path, + {}, + enable: [:final_newline], + skip_if_title_matches: dangerfile.default_branch_merge_title_patterns + ) + end + + it "does not run if the title matches a pattern (GitLab)" do + stub_githost(:gitlab, title: "Merge branch 'foo' into 'main'") + simple_test( + no_final_newline_path, + {}, + enable: [:final_newline], + skip_if_title_matches: dangerfile.default_branch_merge_title_patterns + ) + end +end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 55b2be1..3e20910 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -34,7 +34,7 @@ def testing_env # A real Danger::Dangerfile, with our checks monkey-patched onto it via # `require "serokell_danger"` above. git/github access still needs -# stubbing per-example (see spec/support/doubles.rb). +# stubbing per-example (see spec/support/danger_testing.rb). def testing_dangerfile env = Danger::EnvironmentManager.new(testing_env) Danger::Dangerfile.new(env, testing_ui) diff --git a/spec/support/danger_testing.rb b/spec/support/danger_testing.rb new file mode 100644 index 0000000..6da83bf --- /dev/null +++ b/spec/support/danger_testing.rb @@ -0,0 +1,62 @@ +# SPDX-FileCopyrightText: 2026 Serokell +# +# SPDX-License-Identifier: MPL-2.0 + +RSpec.shared_context "danger testing" do + let(:dangerfile) { testing_dangerfile } + + def make_fixture_path(fixture) + File.join(__dir__, "..", "fixtures", fixture) + end + + def set_diff(fixture, binary: false) + allow(dangerfile.git).to receive(:diff).and_return([ + instance_double( + Git::Diff::DiffFile, + type: "modified", + binary?: binary, + destination_path: fixture + ) + ]) + end + + def set_tracked_files(added_files: [], modified_files: []) + allow(dangerfile.git).to receive(:added_files).and_return(added_files) + allow(dangerfile.git).to receive(:modified_files).and_return(modified_files) + end + + # Stubs dangerfile.githost to a fake PR/MR with the given title. + # host is :github or :gitlab. + def stub_githost(host, title:) + plugin = + case host + when :github then double("plugin", pr_title: title, pr_body: "") + when :gitlab then double("plugin", mr_title: title, mr_body: "") + else raise ArgumentError, "expected :github or :gitlab, got #{host.inspect}" + end + allow(dangerfile).to receive(:githost).and_return(SerokellDanger::GitHost.new(plugin, host)) + end + + # Expects dangerfile.danger_report to fire once for each rule in + # rules_to_test, and not to fire for any other rule in possible_rules. + # + # rules_to_test maps rule => expected attributes, e.g. + # `{trailing_whitespace: {text: "some exact message"}}`. An empty + # attributes hash means "expect it to fire, don't care what with". + # + # keyword_params lists the keyword arguments danger_report is called + # with (e.g. `[:file, :line]`), so they get matched too (or defaulted + # to `anything`). + def set_danger_report_expectations(possible_rules, rules_to_test, keyword_params: []) + possible_rules.each do |rule| + if rules_to_test.key?(rule) + attrs = rules_to_test[rule] + keywords = keyword_params.to_h { |param| [param, attrs.fetch(param, anything)] } + expect(dangerfile).to receive(:danger_report).with(anything, rule, attrs.fetch(:text, anything), **keywords) + else + keywords = keyword_params.to_h { |param| [param, anything] } + expect(dangerfile).not_to receive(:danger_report).with(anything, rule, anything, **keywords) + end + end + end +end From fca918d21b65bb63e9998062bbf2658d88bcc8e0 Mon Sep 17 00:00:00 2001 From: Heitor Toledo Lassarote de Paula Date: Mon, 24 Aug 2026 13:57:23 -0300 Subject: [PATCH 04/19] [#2] Remove now-obsolete private-repo xrefcheck exclusion 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. --- .xrefcheck.yaml | 6 ------ 1 file changed, 6 deletions(-) diff --git a/.xrefcheck.yaml b/.xrefcheck.yaml index 5441dda..8c222c8 100644 --- a/.xrefcheck.yaml +++ b/.xrefcheck.yaml @@ -9,9 +9,3 @@ exclusions: # Notion requires authentication; xrefcheck cannot verify these links. ignoreExternalRefsTo: - "https://app.notion.com/.*" - # TODO: remove once this repo goes public. While it's private, GitHub - # returns 404 (not 403) for any of its own pages (issues, PRs, Actions - # runs, etc.) to an unauthenticated request, indistinguishable from a - # genuinely broken link. Once public, this exclusion would silently - # hide real broken links instead. - - "https://github\\.com/serokell/danger(/.*)?" From 7042ccae47a5130d1f93a5742496f05cd9c1989f Mon Sep 17 00:00:00 2001 From: Heitor Toledo Lassarote de Paula Date: Mon, 24 Aug 2026 19:13:45 -0300 Subject: [PATCH 05/19] [#4] Add license headers tests 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. --- Dangerfile | 4 +- REUSE.toml | 6 + spec/fixtures/no_license_header.md | 1 + spec/fixtures/outdated_license_year.md | 7 + .../outdated_year_and_wrong_holder.md | 7 + spec/fixtures/wrong_copyright_holder.md | 7 + spec/serokell_danger/license_headers_spec.rb | 158 ++++++++++++++++++ 7 files changed, 189 insertions(+), 1 deletion(-) create mode 100644 spec/fixtures/no_license_header.md create mode 100644 spec/fixtures/outdated_license_year.md create mode 100644 spec/fixtures/outdated_year_and_wrong_holder.md create mode 100644 spec/fixtures/wrong_copyright_holder.md create mode 100644 spec/serokell_danger/license_headers_spec.rb diff --git a/Dangerfile b/Dangerfile index ce27f4b..724eb55 100644 --- a/Dangerfile +++ b/Dangerfile @@ -8,7 +8,9 @@ check_commits_style check_premerge_commits check_merge_request check_merge_commits -check_license_headers +check_license_headers( + license_headers_default_config.with(ignore_paths: ["spec/fixtures/"]) +) check_trailing_whitespace( trailing_whitespace_default_config.with( ignore_paths: ["LICENSE", "LICENSES/", "spec/fixtures/"] diff --git a/REUSE.toml b/REUSE.toml index 293eef4..9165729 100644 --- a/REUSE.toml +++ b/REUSE.toml @@ -17,3 +17,9 @@ path = "spec/fixtures/pull_request_event.json" precedence = "aggregate" SPDX-FileCopyrightText = "2026 Serokell " SPDX-License-Identifier = "CC0-1.0" + +[[annotations]] +path = "spec/fixtures/no_license_header.md" +precedence = "aggregate" +SPDX-FileCopyrightText = "2026 Serokell " +SPDX-License-Identifier = "MPL-2.0" diff --git a/spec/fixtures/no_license_header.md b/spec/fixtures/no_license_header.md new file mode 100644 index 0000000..3e49642 --- /dev/null +++ b/spec/fixtures/no_license_header.md @@ -0,0 +1 @@ +This fixture has no license header at all and should be silently skipped. diff --git a/spec/fixtures/outdated_license_year.md b/spec/fixtures/outdated_license_year.md new file mode 100644 index 0000000..87de45c --- /dev/null +++ b/spec/fixtures/outdated_license_year.md @@ -0,0 +1,7 @@ + + +This fixture has an outdated license year and should be flagged by the test. diff --git a/spec/fixtures/outdated_year_and_wrong_holder.md b/spec/fixtures/outdated_year_and_wrong_holder.md new file mode 100644 index 0000000..3b8b356 --- /dev/null +++ b/spec/fixtures/outdated_year_and_wrong_holder.md @@ -0,0 +1,7 @@ + + +This fixture has both an outdated license year and the wrong copyright holder. diff --git a/spec/fixtures/wrong_copyright_holder.md b/spec/fixtures/wrong_copyright_holder.md new file mode 100644 index 0000000..6b31251 --- /dev/null +++ b/spec/fixtures/wrong_copyright_holder.md @@ -0,0 +1,7 @@ + + +This fixture has the wrong copyright holder. diff --git a/spec/serokell_danger/license_headers_spec.rb b/spec/serokell_danger/license_headers_spec.rb new file mode 100644 index 0000000..abd30db --- /dev/null +++ b/spec/serokell_danger/license_headers_spec.rb @@ -0,0 +1,158 @@ +# SPDX-FileCopyrightText: 2026 Serokell +# +# SPDX-License-Identifier: MPL-2.0 + +require_relative "../../lib/serokell_danger/checks/license-headers" + +RSpec.describe "license-headers check" do + include_context "danger testing" + + let(:outdated_license_year) { make_fixture_path("outdated_license_year.md") } + let(:wrong_copyright_holder) do + make_fixture_path("wrong_copyright_holder.md") + end + let(:outdated_year_and_wrong_holder) do + make_fixture_path("outdated_year_and_wrong_holder.md") + end + let(:no_license_header) { make_fixture_path("no_license_header.md") } + let(:nonexistent_file) { make_fixture_path("does_not_exist.md") } + + def make_config( + scan: :added_files, file_extensions: nil, ignore_paths: [], + check_year: true, expected_holder: nil, skip_if_title_matches: nil + ) + SerokellDanger::Config.new( + "license-headers", + { + scan: scan, + file_extensions: file_extensions, + ignore_paths: ignore_paths, + copyright_line_pattern: + dangerfile.license_headers_default_config[:copyright_line_pattern], + check_year: check_year, + expected_holder: expected_holder, + skip_if_title_matches: skip_if_title_matches, + severity: :warn + }, + configure_with: "check_license_headers" + ) + end + + def set_rule_receive_expectations(rules_to_test) + set_danger_report_expectations( + [:check_year, :expected_holder], + rules_to_test, + keyword_params: [:file, :line] + ) + end + + def simple_test(rules_to_test, added_files: [], modified_files: [], + **config_overrides) + config = make_config(**config_overrides) + set_tracked_files(added_files: added_files, modified_files: modified_files) + set_rule_receive_expectations(rules_to_test) + yield if block_given? + dangerfile.check_license_headers(config) + end + + it "warns about a file with outdated license year" do + simple_test({check_year: {}}, added_files: [outdated_license_year]) + end + + it "warns about a file with wrong copyright holder" do + simple_test( + { + expected_holder: { + text: "The copyright holder is `example `, " \ + "expected `serokell `." + } + }, + added_files: [wrong_copyright_holder], + check_year: false, expected_holder: "Serokell " + ) + end + + it "runs with files in file_extensions" do + simple_test( + {check_year: {}}, + added_files: [outdated_license_year], + file_extensions: [".md"] + ) + end + + it "does not run with files not in file_extensions" do + simple_test( + {}, + added_files: [outdated_license_year], + file_extensions: [".rb"] + ) + end + + it "ignores files in ignore_paths" do + simple_test( + {}, + added_files: [outdated_license_year], + ignore_paths: [outdated_license_year] + ) + end + + it "scans modified files too when scan: :changed_files" do + simple_test( + {check_year: {}}, + modified_files: [outdated_license_year], + scan: :changed_files + ) + end + + it "warns about both an outdated year and a wrong holder on the same file" do + simple_test( + { + check_year: {}, + expected_holder: { + text: "The copyright holder is `example `, " \ + "expected `serokell `." + } + }, + added_files: [outdated_year_and_wrong_holder], + expected_holder: "Serokell " + ) + end + + # This check only validates the year/holder on a header that's already + # there; it's not meant to catch a missing header entirely. That's + # reuse lint's job (REUSE/SPDX compliance), so a file with no + # header-shaped line at all is silently skipped here. + it "does not report a file with no license header at all" do + simple_test({}, added_files: [no_license_header]) + end + + it "does not report a tracked file that does not exist on disk" do + simple_test({}, added_files: [nonexistent_file]) + end + + it "raises for an invalid scan value" do + config = make_config(scan: :invalid) + + expect { dangerfile.check_license_headers(config) }.to raise_error( + ArgumentError, /`scan` expects/ + ) + end + + it "does not run if the title matches a pattern (GitHub)" do + stub_githost(:github, title: "Merge branch 'foo' into 'main'") + simple_test( + {}, + added_files: [outdated_license_year], + skip_if_title_matches: dangerfile.default_branch_merge_title_patterns + ) + end + + it "does not run if the title matches a pattern (GitLab)" do + stub_githost(:gitlab, title: "Merge branch 'foo' into 'main'") + simple_test( + {}, + added_files: [outdated_license_year], + skip_if_title_matches: dangerfile.default_branch_merge_title_patterns + ) + end +end From c10b974c5dbacbf8673637bdcb6e6f161c9b59a3 Mon Sep 17 00:00:00 2001 From: Heitor Toledo Lassarote de Paula Date: Tue, 25 Aug 2026 11:02:09 -0300 Subject: [PATCH 06/19] [#4] Add commits-style tests 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. --- spec/serokell_danger/commits_style_spec.rb | 206 +++++++++++++++++++++ spec/support/danger_testing.rb | 34 +++- 2 files changed, 237 insertions(+), 3 deletions(-) create mode 100644 spec/serokell_danger/commits_style_spec.rb diff --git a/spec/serokell_danger/commits_style_spec.rb b/spec/serokell_danger/commits_style_spec.rb new file mode 100644 index 0000000..6950d70 --- /dev/null +++ b/spec/serokell_danger/commits_style_spec.rb @@ -0,0 +1,206 @@ +# SPDX-FileCopyrightText: 2026 Serokell +# +# SPDX-License-Identifier: MPL-2.0 + +require_relative "../../lib/serokell_danger/checks/commits-style" + +RSpec.describe "commits-style check" do + include_context "danger testing" + + # Unlike trailing-whitespace/license-headers, most of this check's rules + # are already true/set in the real default config, so a made-up commit + # would otherwise trip several of them at once. Start from the real + # defaults and override only what a given example cares about, e.g. + # `make_config(commit_msg_prefix: nil, max_line_length: nil)` to silence + # unrelated rules while testing one in isolation. + def make_config(**overrides) + dangerfile.commits_style_default_config.merge({skip_if_title_matches: nil}.merge(overrides)) + end + + def set_rule_receive_expectations(rules_to_test) + set_danger_report_expectations( + [ + :commit_msg_prefix, + :subject_spacing, + :subject_starts_with_uppercase, + :subject_no_trailing_dot, + :max_subject_length, + :max_subject_length_hard, + :require_description, + :blank_line_after_subject, + :commit_description_patterns, + :max_line_length + ], + rules_to_test, + keyword_params: {commit_msg_prefix: [:hint]} + ) + end + + def simple_test(commit, rules_to_test, **config_overrides) + config = make_config(**config_overrides) + set_commits(commit) + set_rule_receive_expectations(rules_to_test) + yield if block_given? + dangerfile.check_commits_style(config) + end + + # --- subject rules --- + + it "warns about a commit missing an issue prefix" do + simple_test( + make_commit(subject: "No issue prefix"), + {commit_msg_prefix: {}} + ) + end + + it "does not warn about a commit with a valid YouTrack-style issue prefix" do + simple_test(make_commit(subject: "[SRK-160] Issue prefix"), {}) + end + + it "warns about a lowercase subject after a valid YouTrack-style issue prefix" do + # Regression case: the prefix itself contains an uppercase letter + # (the "S" in "SRK"), so this proves subject_starts_with_uppercase + # checks the payload after the full prefix is stripped, not just + # whether an uppercase letter appears anywhere near the start. + simple_test( + make_commit(subject: "[SRK-160] issue prefix"), + {subject_starts_with_uppercase: {}} + ) + end + + it "warns about wrong spacing between the issue prefix and the rest of the subject" do + simple_test(make_commit(subject: "[#4]No spacing"), {subject_spacing: {}}) + end + + it "warns about a subject not starting with an uppercase letter" do + simple_test( + make_commit(subject: "[#4] lowercase subject"), + {subject_starts_with_uppercase: {}} + ) + end + + it "warns about a subject with a trailing dot" do + simple_test( + make_commit(subject: "[#4] Trailing dot in subject."), + {subject_no_trailing_dot: {}} + ) + end + + it "warns about a subject longer than max_subject_length" do + simple_test( + make_commit(subject: "[#4] This subject is too long and should be rejected by max_subject_length"), + {max_subject_length: {}} + ) + end + + it "warns (harder) about a subject longer than max_subject_length_hard" do + simple_test( + make_commit(subject: "[#4] This subject is definitely way too long and should be rejected by max_subject_length_hard"), + {max_subject_length_hard: {}} + ) + end + + # --- description rules --- + + it "warns about a commit with no description" do + simple_test( + make_commit(subject: "[#4] Subject", description: ""), + {require_description: {}} + ) + end + + it "does not warn about a missing description on a chore commit" do + simple_test( + make_commit(subject: "[Chore] WIP", description: ""), + {} + ) + end + + it "warns about a missing blank line after the subject" do + simple_test( + make_commit(subject: "[#4] No blank line", blank_line_after_subject: false), + {blank_line_after_subject: {}} + ) + end + + it "warns about a description not matching commit_description_patterns" do + simple_test( + make_commit(subject: "[#4] Subject", description: "Not in problem/solution format"), + {commit_description_patterns: {}} + ) + end + + it "does not warn when description_escape_hatch matches" do + simple_test( + make_commit(subject: "[#4] Subject", description: "I don't care about templates"), + {} + ) + end + + it "warns about description lines longer than max_line_length" do + simple_test( + make_commit( + subject: "[#4] Subject", + description: "Problem: This description is definitely way too long and should be rejected by max_line_length.\n\nSolution: Fail the test expectation." + ), + {max_line_length: {}} + ) + end + + # --- whole-commit exemptions --- + + it "skips a commit entirely if its subject matches wip_commit_patterns" do + simple_test( + make_commit(subject: "wip multiple problems.", description: "Not in problem/solution format"), + {} + ) + end + + it "skips a commit entirely if its subject matches fixup_commit_patterns" do + simple_test( + make_commit(subject: "fixup! wip multiple problems.", description: "Not in problem/solution format"), + {} + ) + end + + it "skips a commit entirely if its subject matches exempt_commit_patterns" do + simple_test( + make_commit(subject: "changelog wip multiple problems.", description: "Not in problem/solution format"), + {} + ) + end + + # --- skip_if_title_matches (mirrors trailing-whitespace/license-headers) --- + + it "does not run if the title matches a pattern (GitHub)" do + stub_githost(:github, title: "Merge branch 'foo' into 'main'") + simple_test( + make_commit(subject: "No issue prefix"), + {}, + skip_if_title_matches: dangerfile.default_branch_merge_title_patterns + ) + end + + it "does not run if the title matches a pattern (GitLab)" do + stub_githost(:gitlab, title: "Merge branch 'foo' into 'main'") + simple_test( + make_commit(subject: "No issue prefix"), + {}, + skip_if_title_matches: dangerfile.default_branch_merge_title_patterns + ) + end + + # --- explain_workarounds --- + + it "adds an explanatory markdown note when something was reported and explain_workarounds is true" do + simple_test(make_commit(subject: "No issue prefix"), {commit_msg_prefix: {}}) do + expect(dangerfile).to receive(:markdown).with(a_string_matching(/Every rule above can be turned off/)) + end + end + + it "adds no markdown note when nothing was reported" do + simple_test(make_commit(subject: "[#4] Well-formed commit subject"), {}) do + expect(dangerfile).not_to receive(:markdown) + end + end +end diff --git a/spec/support/danger_testing.rb b/spec/support/danger_testing.rb index 6da83bf..21d6e8f 100644 --- a/spec/support/danger_testing.rb +++ b/spec/support/danger_testing.rb @@ -46,17 +46,45 @@ def stub_githost(host, title:) # # keyword_params lists the keyword arguments danger_report is called # with (e.g. `[:file, :line]`), so they get matched too (or defaulted - # to `anything`). + # to `anything`). Pass an Array to use it for every rule, or a Hash to + # use a different list per rule, e.g. `{check_year: [:file, :line], + # expected_holder: [:file]}`. def set_danger_report_expectations(possible_rules, rules_to_test, keyword_params: []) possible_rules.each do |rule| + rule_keyword_params = + keyword_params.is_a?(Hash) ? keyword_params.fetch(rule, []) : keyword_params + if rules_to_test.key?(rule) attrs = rules_to_test[rule] - keywords = keyword_params.to_h { |param| [param, attrs.fetch(param, anything)] } + keywords = rule_keyword_params.to_h { |param| [param, attrs.fetch(param, anything)] } expect(dangerfile).to receive(:danger_report).with(anything, rule, attrs.fetch(:text, anything), **keywords) else - keywords = keyword_params.to_h { |param| [param, anything] } + keywords = rule_keyword_params.to_h { |param| [param, anything] } expect(dangerfile).not_to receive(:danger_report).with(anything, rule, anything, **keywords) end end end + + # Builds a fake commit with the given subject, description, and other + # attributes used by the checks. + def make_commit( + subject:, + description: "Problem: This is a placeholder description.\n\nSolution: It exists so unrelated rules don't fire.\n", + blank_line_after_subject: true, short_ref: "deadbeef" + ) + commit = instance_double( + Git::Object::Commit, + subject: subject, + subject_ticked: "`#{subject.tr("`", "'")}`", + description: description, + blank_line_after_subject?: blank_line_after_subject, + short_ref: short_ref + ) + allow(commit).to receive(:subject_matches?) { |patterns| SerokellDanger::Util.matches_any?(subject, patterns) } + commit + end + + def set_commits(*commits) + allow(dangerfile.git).to receive(:commits).and_return(commits) + end end From 59ae729f7087f5c3a70f83c0b1e6f943d9f3b479 Mon Sep 17 00:00:00 2001 From: Heitor Toledo Lassarote de Paula Date: Tue, 25 Aug 2026 12:35:31 -0300 Subject: [PATCH 07/19] [#4] Add merge-commits tests 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. --- spec/serokell_danger/commits_style_spec.rb | 2 +- spec/serokell_danger/merge_commits_spec.rb | 102 +++++++++++++++++++++ spec/support/danger_testing.rb | 12 ++- 3 files changed, 113 insertions(+), 3 deletions(-) create mode 100644 spec/serokell_danger/merge_commits_spec.rb diff --git a/spec/serokell_danger/commits_style_spec.rb b/spec/serokell_danger/commits_style_spec.rb index 6950d70..8127668 100644 --- a/spec/serokell_danger/commits_style_spec.rb +++ b/spec/serokell_danger/commits_style_spec.rb @@ -14,7 +14,7 @@ # `make_config(commit_msg_prefix: nil, max_line_length: nil)` to silence # unrelated rules while testing one in isolation. def make_config(**overrides) - dangerfile.commits_style_default_config.merge({skip_if_title_matches: nil}.merge(overrides)) + make_config_from_default(:commits_style_default_config, **overrides) end def set_rule_receive_expectations(rules_to_test) diff --git a/spec/serokell_danger/merge_commits_spec.rb b/spec/serokell_danger/merge_commits_spec.rb new file mode 100644 index 0000000..5c81b9a --- /dev/null +++ b/spec/serokell_danger/merge_commits_spec.rb @@ -0,0 +1,102 @@ +# SPDX-FileCopyrightText: 2026 Serokell +# +# SPDX-License-Identifier: MPL-2.0 + +require_relative "../../lib/serokell_danger/checks/merge-commits" + +RSpec.describe "merge-commits check" do + include_context "danger testing" + + def make_config(**overrides) + make_config_from_default(:merge_commits_default_config, **overrides) + end + + def set_rule_receive_expectations(rules_to_test) + set_danger_report_expectations([:detect], rules_to_test, keyword_params: [:hint]) + end + + def simple_test(commits, rules_to_test, **config_overrides) + config = make_config(**config_overrides) + set_commits(*commits) + set_rule_receive_expectations(rules_to_test) + yield if block_given? + dangerfile.check_merge_commits(config) + end + + it "reports a commit with more than one parent" do + simple_test( + [make_commit(subject: "[#4] Subject", parents: %i[parent1 parent2])], + {detect: {}} + ) + end + + it "does not report a commit with a single parent" do + simple_test( + [make_commit(subject: "[#4] Subject", parents: %i[parent])], + {} + ) + end + + # It's not confirmed whether this exact set of prefixes is a strict + # requirement or just a heuristic to avoid false triggers; this test + # only pins current behavior, not necessarily the intended contract. + # See https://github.com/serokell/danger/issues/11. + it "reports a commit whose subject matches merge_commit_subject_patterns when detect includes :subject" do + simple_test( + [make_commit(subject: "Merge branch 'foo' into 'main")], + {detect: {}}, + detect: %i[subject] + ) + end + + it "does not detect by subject when detect does not include :subject (the default)" do + simple_test( + [make_commit(subject: "Merge branch 'foo' into 'main")], + {} + ) + end + + it "mentions every offending commit in a single danger_report call" do + simple_test( + [ + make_commit(subject: "[#4] Subject", parents: %i[parent1 parent2]), + make_commit(subject: "Merge branch 'foo' into 'main", short_ref: "fee1dead") + ], + { + detect: { + text: "Please, no merge commits, rebase for the win. Found: " \ + "deadbeef (`[#4] Subject`), fee1dead (`Merge branch 'foo' into 'main`)." + } + }, + detect: %i[parents subject] + ) + end + + it "raises for an invalid detect value" do + config = make_config(detect: %i[this_option_does_not_exist]) + set_commits(make_commit(subject: "[#4] Subject")) + + expect { dangerfile.check_merge_commits(config) }.to raise_error( + ArgumentError, + "`detect` expects :parents and/or :subject, got :this_option_does_not_exist." + ) + end + + it "does not run if the title matches a pattern (GitHub)" do + stub_githost(:github, title: "Merge branch 'foo' into 'main'") + simple_test( + [make_commit(subject: "[#4] Subject", parents: %i[parent1 parent2])], + {}, + skip_if_title_matches: dangerfile.default_branch_merge_title_patterns + ) + end + + it "does not run if the title matches a pattern (GitLab)" do + stub_githost(:gitlab, title: "Merge branch 'foo' into 'main'") + simple_test( + [make_commit(subject: "[#4] Subject", parents: %i[parent1 parent2])], + {}, + skip_if_title_matches: dangerfile.default_branch_merge_title_patterns + ) + end +end diff --git a/spec/support/danger_testing.rb b/spec/support/danger_testing.rb index 21d6e8f..8dbb83b 100644 --- a/spec/support/danger_testing.rb +++ b/spec/support/danger_testing.rb @@ -70,7 +70,7 @@ def set_danger_report_expectations(possible_rules, rules_to_test, keyword_params def make_commit( subject:, description: "Problem: This is a placeholder description.\n\nSolution: It exists so unrelated rules don't fire.\n", - blank_line_after_subject: true, short_ref: "deadbeef" + blank_line_after_subject: true, short_ref: "deadbeef", parents: [nil] ) commit = instance_double( Git::Object::Commit, @@ -78,7 +78,8 @@ def make_commit( subject_ticked: "`#{subject.tr("`", "'")}`", description: description, blank_line_after_subject?: blank_line_after_subject, - short_ref: short_ref + short_ref: short_ref, + parents: parents ) allow(commit).to receive(:subject_matches?) { |patterns| SerokellDanger::Util.matches_any?(subject, patterns) } commit @@ -87,4 +88,11 @@ def make_commit( def set_commits(*commits) allow(dangerfile.git).to receive(:commits).and_return(commits) end + + # Builds a config from a check's real default config (e.g. + # :commits_style_default_config), overriding skip_if_title_matches to + # nil unless overrides sets it. + def make_config_from_default(default_config_method, **overrides) + dangerfile.public_send(default_config_method).merge({skip_if_title_matches: nil}.merge(overrides)) + end end From e8fd5185de252e2160f4058f9add3645e9b1529d Mon Sep 17 00:00:00 2001 From: Heitor Toledo Lassarote de Paula Date: Tue, 25 Aug 2026 17:23:12 -0300 Subject: [PATCH 08/19] [#4] Add merge-request tests 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. --- CHANGES.md | 2 + lib/serokell_danger/checks/merge-request.rb | 2 +- spec/serokell_danger/merge_request_spec.rb | 127 ++++++++++++++++++++ spec/support/danger_testing.rb | 14 ++- 4 files changed, 138 insertions(+), 7 deletions(-) create mode 100644 spec/serokell_danger/merge_request_spec.rb diff --git a/CHANGES.md b/CHANGES.md index 0d8551f..052d7fc 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -17,3 +17,5 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). - `check_merge_request` no longer calls the GitHub-only `mr_title`/`mr_body` methods against a GitHub PR, which raised instead of reading the PR's title/body (#6). +- Fixed a typo in `check_merge_request`'s `ticket_links.scan` error message + ("expecs" → "expects") (#6). diff --git a/lib/serokell_danger/checks/merge-request.rb b/lib/serokell_danger/checks/merge-request.rb index 7ff990e..eb5a269 100644 --- a/lib/serokell_danger/checks/merge-request.rb +++ b/lib/serokell_danger/checks/merge-request.rb @@ -54,7 +54,7 @@ def check_merge_request(config = merge_request_default_config) case where.to_sym when :title then title when :body then body - else raise ArgumentError, "`ticket_links.scan` expecs :title and/or :body, got #{where.inspect}." + else raise ArgumentError, "`ticket_links.scan` expects :title and/or :body, got #{where.inspect}." end end.join("\n") diff --git a/spec/serokell_danger/merge_request_spec.rb b/spec/serokell_danger/merge_request_spec.rb new file mode 100644 index 0000000..5ac5de6 --- /dev/null +++ b/spec/serokell_danger/merge_request_spec.rb @@ -0,0 +1,127 @@ +# SPDX-FileCopyrightText: 2026 Serokell +# +# SPDX-License-Identifier: MPL-2.0 + +require_relative "../../lib/serokell_danger/checks/merge-request" + +RSpec.describe "merge-request check" do + include_context "danger testing" + + # Unlike the other checks, check_merge_request reads + # githost.mr_title_payload/mr_body unconditionally (mr_context? is + # checked first, before anything else) - every example needs + # stub_githost(:github/:gitlab, title:, body:), there's no "works fine + # with the real unstubbed githost" fallback here. + def make_config(**overrides) + make_config_from_default(:merge_request_default_config, **overrides) + end + + def set_rule_receive_expectations(rules_to_test) + set_danger_report_expectations( + [:title_prefix, :ticket_links], + rules_to_test, + keyword_params: {title_prefix: [:hint]} + ) + end + + def simple_test(rules_to_test, title:, body: "", **config_overrides) + %i[github gitlab].each do |githost| + stub_githost(githost, title: title, body: body) + config = make_config(**config_overrides) + set_rule_receive_expectations(rules_to_test) + yield if block_given? + dangerfile.check_merge_request(config) + end + end + + # --- title_prefix --- + + it "warns about a title missing an issue prefix" do + simple_test({title_prefix: {}}, title: "No issue prefix") + end + + it "does not warn about a title with a valid issue prefix" do + simple_test({}, title: "[#4] Issue prefix") + end + + # --- ticket_links --- + + it "reports ticket links mentioned in the title" do + simple_test({ticket_links: {}}, title: "[SRK-160] Issue prefix") + end + + it "reports ticket links mentioned in the body" do + simple_test({ticket_links: {}}, title: "[#4] Issue prefix", body: "Fixes SRK-160") + end + + it "does not scan the body for ticket links when ticket_links.scan does not include :body" do + simple_test( + {}, + title: "[#4] Issue prefix", + body: "Fixes SRK-160", + ticket_links: SerokellDanger::Config.new( + "ticket-links", + { + scan: %i[title], + pattern: /\b[A-Z][A-Z0-9]*-\d+\b/, + base_url: "https://issues.serokell.io/issue/", + tracker_name: "YouTrack" + } + ) + ) + end + + it "does not report the same ticket link twice" do + simple_test( + {ticket_links: {text: "Mentioned YouTrack tickets: [SRK-160](https://issues.serokell.io/issue/SRK-160)."}}, + title: "[SRK-160] Issue prefix", + body: "Fixes SRK-160" + ) + end + + it "does nothing when ticket_links is nil" do + simple_test( + {}, + title: "[SRK-160] Issue prefix", + body: "Fixes SRK-160", + ticket_links: nil + ) + end + + it "raises for an invalid ticket_links.scan value" do + config = make_config( + ticket_links: SerokellDanger::Config.new( + "ticket-links", + { + scan: %i[this_item_does_not_exist], + pattern: /\b[A-Z][A-Z0-9]*-\d+\b/, + base_url: "https://issues.serokell.io/issue/", + tracker_name: "YouTrack" + } + ) + ) + + stub_githost(:github, title: "[#4] Dummy") + expect { dangerfile.check_merge_request(config) }.to raise_error( + ArgumentError, + "`ticket_links.scan` expects :title and/or :body, got :this_item_does_not_exist." + ) + end + + # --- context/skip --- + + it "does nothing outside of an MR/PR context" do + allow(dangerfile).to receive(:githost).and_return(nil) + config = make_config + set_rule_receive_expectations({}) + dangerfile.check_merge_request(config) + end + + it "does not run if the title matches a pattern" do + simple_test( + {}, + title: "Merge branch 'foo' into 'main'", + skip_if_title_matches: dangerfile.default_branch_merge_title_patterns + ) + end +end diff --git a/spec/support/danger_testing.rb b/spec/support/danger_testing.rb index 8dbb83b..71adedf 100644 --- a/spec/support/danger_testing.rb +++ b/spec/support/danger_testing.rb @@ -25,13 +25,13 @@ def set_tracked_files(added_files: [], modified_files: []) allow(dangerfile.git).to receive(:modified_files).and_return(modified_files) end - # Stubs dangerfile.githost to a fake PR/MR with the given title. - # host is :github or :gitlab. - def stub_githost(host, title:) + # Stubs dangerfile.githost to a fake PR/MR with the given title and + # body. host is :github or :gitlab. + def stub_githost(host, title:, body: "") plugin = case host - when :github then double("plugin", pr_title: title, pr_body: "") - when :gitlab then double("plugin", mr_title: title, mr_body: "") + when :github then double("plugin", pr_title: title, pr_body: body) + when :gitlab then double("plugin", mr_title: title, mr_body: body) else raise ArgumentError, "expected :github or :gitlab, got #{host.inspect}" end allow(dangerfile).to receive(:githost).and_return(SerokellDanger::GitHost.new(plugin, host)) @@ -93,6 +93,8 @@ def set_commits(*commits) # :commits_style_default_config), overriding skip_if_title_matches to # nil unless overrides sets it. def make_config_from_default(default_config_method, **overrides) - dangerfile.public_send(default_config_method).merge({skip_if_title_matches: nil}.merge(overrides)) + default_config = dangerfile.public_send(default_config_method) + values = default_config.to_h.merge(skip_if_title_matches: nil, **overrides) + SerokellDanger::Config.new(default_config.check_name, values, configure_with: default_config.configure_with) end end From 9cb427cdf957be30cc7386c86656eaeabb0add27 Mon Sep 17 00:00:00 2001 From: Heitor Toledo Lassarote de Paula Date: Tue, 25 Aug 2026 18:17:42 -0300 Subject: [PATCH 09/19] [#4] Add premerge-commits tests 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. --- spec/serokell_danger/premerge_commits_spec.rb | 94 +++++++++++++++++++ 1 file changed, 94 insertions(+) create mode 100644 spec/serokell_danger/premerge_commits_spec.rb diff --git a/spec/serokell_danger/premerge_commits_spec.rb b/spec/serokell_danger/premerge_commits_spec.rb new file mode 100644 index 0000000..6d93f46 --- /dev/null +++ b/spec/serokell_danger/premerge_commits_spec.rb @@ -0,0 +1,94 @@ +# SPDX-FileCopyrightText: 2026 Serokell +# +# SPDX-License-Identifier: MPL-2.0 + +require_relative "../../lib/serokell_danger/checks/premerge-commits" + +RSpec.describe "premerge-commits check" do + include_context "danger testing" + + def make_config(**overrides) + make_config_from_default(:premerge_commits_default_config, **overrides) + end + + def set_rule_receive_expectations(rules_to_test) + set_danger_report_expectations([:wip_commit_patterns, :fixup_commit_patterns], rules_to_test) + end + + def simple_test(commits, rules_to_test, **config_overrides) + config = make_config(**config_overrides) + set_commits(*commits) + set_rule_receive_expectations(rules_to_test) + yield if block_given? + dangerfile.check_premerge_commits(config) + end + + it "reports a commit whose subject matches wip_commit_patterns" do + simple_test( + [make_commit(subject: "wip write a test for premerge-commits")], + {wip_commit_patterns: {}} + ) + end + + it "reports a commit whose subject matches fixup_commit_patterns" do + simple_test( + [make_commit(subject: "fixup! add premerge-commits tests")], + {fixup_commit_patterns: {}} + ) + end + + it "does not report a commit whose subject matches neither pattern" do + simple_test( + [make_commit(subject: "[#4] Add premerge-commits tests")], + {} + ) + end + + it "mentions every offending commit in a single danger_report call" do + simple_test( + [ + make_commit(subject: "wip add premerge-commits tests"), + make_commit(subject: "wip add merge-request tests", short_ref: "fee1dead") + ], + { + wip_commit_patterns: { + text: "Some work-in-progress commits are still there: deadbeef (`wip add premerge-commits tests`), fee1dead (`wip add merge-request tests`)." + } + } + ) + end + + it "does not check for wip commits when wip_commit_patterns is nil" do + simple_test( + [make_commit(subject: "wip add premerge-commits tests")], + {}, + wip_commit_patterns: nil + ) + end + + it "does not check for fixup commits when fixup_commit_patterns is nil" do + simple_test( + [make_commit(subject: "fixup add premerge-commits tests")], + {}, + fixup_commit_patterns: nil + ) + end + + it "does not run if the title matches a pattern (GitHub)" do + stub_githost(:github, title: "Merge branch 'foo' into 'main'") + simple_test( + [make_commit(subject: "wip: still in progress")], + {}, + skip_if_title_matches: dangerfile.default_branch_merge_title_patterns + ) + end + + it "does not run if the title matches a pattern (GitLab)" do + stub_githost(:gitlab, title: "Merge branch 'foo' into 'main'") + simple_test( + [make_commit(subject: "wip: still in progress")], + {}, + skip_if_title_matches: dangerfile.default_branch_merge_title_patterns + ) + end +end From 0a22ca5e931015723352d417121c2dcaf9568787 Mon Sep 17 00:00:00 2001 From: Heitor Toledo Lassarote de Paula Date: Tue, 25 Aug 2026 19:04:13 -0300 Subject: [PATCH 10/19] [#4] Add SerokellDanger::Util tests 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. --- spec/serokell_danger/util_spec.rb | 137 ++++++++++++++++++++++++++++++ 1 file changed, 137 insertions(+) create mode 100644 spec/serokell_danger/util_spec.rb diff --git a/spec/serokell_danger/util_spec.rb b/spec/serokell_danger/util_spec.rb new file mode 100644 index 0000000..2b9d024 --- /dev/null +++ b/spec/serokell_danger/util_spec.rb @@ -0,0 +1,137 @@ +# SPDX-FileCopyrightText: 2026 Serokell +# +# SPDX-License-Identifier: MPL-2.0 + +require "serokell_danger" + +RSpec.describe SerokellDanger::Util do + describe ".as_list" do + it "turns nil into an empty array" do + expect(described_class.as_list(nil)).to eq([]) + end + + it "leaves an array as is" do + expect(described_class.as_list([1, 2])).to eq([1, 2]) + end + + it "wraps a single value in an array" do + expect(described_class.as_list(:foo)).to eq([:foo]) + end + end + + describe ".pattern_matches?" do + it "matches a Regexp against the string" do + expect(described_class.pattern_matches?("foobar", /oob/)).to be true + expect(described_class.pattern_matches?("foobar", /xyz/)).to be false + end + + it "checks substring inclusion for a non-Regexp pattern" do + expect(described_class.pattern_matches?("foobar", "oob")).to be true + expect(described_class.pattern_matches?("foobar", "xyz")).to be false + end + end + + describe ".matches_any?" do + it "is true if any pattern in the list matches" do + expect(described_class.matches_any?("foobar", [/xyz/, /oob/])).to be true + end + + it "is false if no pattern in the list matches" do + expect(described_class.matches_any?("foobar", [/xyz/, "abc"])).to be false + end + + it "accepts a single pattern instead of a list" do + expect(described_class.matches_any?("foobar", /oob/)).to be true + end + end + + describe ".path_selected?" do + it "is true with no file_extensions and no ignore_paths" do + expect(described_class.path_selected?("lib/foo.rb")).to be true + end + + it "is false for a path matching ignore_paths (glob)" do + expect( + described_class.path_selected?("lib/foo.rb", ignore_paths: ["lib/*.rb"]) + ).to be false + end + + it "is false for a path matching ignore_paths (Regexp)" do + expect( + described_class.path_selected?("lib/foo.rb", ignore_paths: [/\.rb\z/]) + ).to be false + end + + it "lets ignore_paths take precedence over file_extensions" do + expect( + described_class.path_selected?( + "lib/foo.rb", file_extensions: [".rb"], ignore_paths: ["lib/*.rb"] + ) + ).to be false + end + + it "is true when the path's extension is in file_extensions" do + expect( + described_class.path_selected?("lib/foo.rb", file_extensions: [".rb"]) + ).to be true + end + + it "is false when the path's extension is not in file_extensions" do + expect( + described_class.path_selected?("lib/foo.rb", file_extensions: [".hs"]) + ).to be false + end + + it "accepts file_extensions without a leading dot" do + expect( + described_class.path_selected?("lib/foo.rb", file_extensions: ["rb"]) + ).to be true + end + end + + describe ".path_ignored?" do + it "matches a Regexp against the path" do + expect(described_class.path_ignored?("lib/foo.rb", /\.rb\z/)).to be true + expect(described_class.path_ignored?("lib/foo.rb", /\.hs\z/)).to be false + end + + it "matches a glob pattern against the path" do + expect(described_class.path_ignored?("lib/foo.rb", "lib/*.rb")).to be true + expect( + described_class.path_ignored?("lib/foo.rb", "spec/*.rb") + ).to be false + end + + it "matches a plain path prefix" do + expect(described_class.path_ignored?("lib/foo.rb", "lib/")).to be true + expect(described_class.path_ignored?("lib/foo.rb", "spec/")).to be false + end + end + + describe ".severity_for" do + def make_config(**values) + SerokellDanger::Config.new("test-check", values) + end + + it "returns the override when one is given" do + config = make_config(severity: :warn) + severity = described_class.severity_for(config, :some_rule, :fail) + expect(severity).to eq(:fail) + end + + it "returns the rule's own entry in severities when present" do + config = make_config(severity: :warn, severities: {some_rule: :fail}) + expect(described_class.severity_for(config, :some_rule)).to eq(:fail) + end + + it "falls back to the top-level severity when severities has no entry" do + config = make_config(severity: :message, severities: {other_rule: :fail}) + expect(described_class.severity_for(config, :some_rule)).to eq(:message) + end + + it "falls back to :warn when neither severities nor severity is set" do + config = make_config(some_key: "irrelevant") + expect(described_class.severity_for(config, :some_rule)).to eq(:warn) + end + end +end From c31997bbdcdbe2acbe41072e3fd3cbaa5f8f47e3 Mon Sep 17 00:00:00 2001 From: Heitor Toledo Lassarote de Paula Date: Tue, 25 Aug 2026 19:04:25 -0300 Subject: [PATCH 11/19] [#4] Add issue-prefix.rb tests 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. --- spec/serokell_danger/issue_prefix_spec.rb | 116 ++++++++++++++++++++++ 1 file changed, 116 insertions(+) create mode 100644 spec/serokell_danger/issue_prefix_spec.rb diff --git a/spec/serokell_danger/issue_prefix_spec.rb b/spec/serokell_danger/issue_prefix_spec.rb new file mode 100644 index 0000000..a5ef5de --- /dev/null +++ b/spec/serokell_danger/issue_prefix_spec.rb @@ -0,0 +1,116 @@ +# SPDX-FileCopyrightText: 2026 Serokell +# +# SPDX-License-Identifier: MPL-2.0 + +require "serokell_danger" + +RSpec.describe SerokellDanger do + describe ".valid_issue_prefix?" do + def valid?(text, config) + described_class.valid_issue_prefix?(text, config) + end + + it "recognizes a github/gitlab-style issue prefix" do + expect(valid?("[#4] Fix bug", {kinds: [:github_issue]})).to be true + end + + it "recognizes a youtrack-style issue prefix" do + config = {kinds: [:youtrack_issue]} + expect(valid?("[SRK-160] Fix bug", config)).to be true + end + + it "recognizes a chore prefix" do + expect(valid?("[Chore] Cleanup", {kinds: [:chore]})).to be true + end + + it "rejects text with no recognized prefix" do + expect(valid?("No prefix here", {kinds: [:github_issue]})).to be false + end + + it "recognizes a custom extra_patterns prefix" do + config = {kinds: [], extra_patterns: [/XYZ-\d+/]} + expect(valid?("XYZ-1 Fix bug", config)).to be true + end + + it "allows stacked prefixes with no separator when allow_multiple is true" do + config = {kinds: %i[github_issue chore], allow_multiple: true} + expect(valid?("[#4][Chore] Fix bug", config)).to be true + end + + it "rejects stacked prefixes with no separator when allow_multiple is false" do + config = {kinds: %i[github_issue chore], allow_multiple: false} + expect(valid?("[#4][Chore] Fix bug", config)).to be false + end + end + + describe ".issue_prefix_pattern" do + it "builds a pattern that matches a configured prefix" do + config = {kinds: [:github_issue]} + pattern = described_class.issue_prefix_pattern(config) + expect(pattern).to match("[#4] Fix bug") + expect(pattern).not_to match("No prefix here") + end + + it "raises for an unknown kind" do + config = {kinds: [:bogus]} + expect { described_class.issue_prefix_pattern(config) }.to raise_error( + ArgumentError, /Unknown issue prefix kind `:bogus`/ + ) + end + + it "raises when no kinds or extra_patterns are configured" do + config = {kinds: [], extra_patterns: []} + expect { described_class.issue_prefix_pattern(config) }.to raise_error( + ArgumentError, "No issue prefix kinds configured." + ) + end + end + + describe ".issue_prefix_separator" do + it "returns the whitespace between the prefix and the rest of the text" do + config = {kinds: [:github_issue]} + separator = described_class.issue_prefix_separator("[#4] Fix bug", config) + expect(separator).to eq(" ") + end + + it "returns an empty string when there is no separator" do + config = {kinds: [:github_issue]} + separator = described_class.issue_prefix_separator("[#4]Fix bug", config) + expect(separator).to eq("") + end + + it "returns nil when the text has no recognized prefix" do + config = {kinds: [:github_issue]} + separator = described_class.issue_prefix_separator("No prefix", config) + expect(separator).to be_nil + end + end + + describe ".strip_issue_prefix" do + it "removes the prefix and separator, leaving the rest of the text" do + config = {kinds: [:github_issue]} + stripped = described_class.strip_issue_prefix("[#4] Fix bug", config) + expect(stripped).to eq("Fix bug") + end + + it "leaves the text unchanged when there is no recognized prefix" do + config = {kinds: [:github_issue]} + stripped = described_class.strip_issue_prefix("Fix bug", config) + expect(stripped).to eq("Fix bug") + end + end + + describe ".issue_prefix_examples" do + it "formats an example for each configured kind" do + config = {kinds: %i[github_issue chore]} + examples = described_class.issue_prefix_examples(config) + expect(examples).to eq("`[#123]`, `[Chore]`") + end + + it "includes extra_patterns as examples too" do + config = {kinds: [], extra_patterns: [/XYZ-\d+/]} + examples = described_class.issue_prefix_examples(config) + expect(examples).to eq("`XYZ-\\d+`") + end + end +end From 7c2a1303b8789442cf9e7f7e63117af6d8f2f163 Mon Sep 17 00:00:00 2001 From: Heitor Toledo Lassarote de Paula Date: Tue, 25 Aug 2026 19:04:31 -0300 Subject: [PATCH 12/19] [#4] Add Git::Object::Commit monkey-patch tests 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. --- .../serokell_danger/git_object_commit_spec.rb | 105 ++++++++++++++++++ 1 file changed, 105 insertions(+) create mode 100644 spec/serokell_danger/git_object_commit_spec.rb diff --git a/spec/serokell_danger/git_object_commit_spec.rb b/spec/serokell_danger/git_object_commit_spec.rb new file mode 100644 index 0000000..f71b9a6 --- /dev/null +++ b/spec/serokell_danger/git_object_commit_spec.rb @@ -0,0 +1,105 @@ +# SPDX-FileCopyrightText: 2026 Serokell +# +# SPDX-License-Identifier: MPL-2.0 + +require "serokell_danger" + +RSpec.describe Git::Object::Commit do + # Builds a real Commit, entirely offline: passing `init` eagerly (see + # Git::Object::Commit#initialize/#from_data in the git gem) sets @tree, + # so #check_commit's lazy-fetch guard never triggers and no real + # repository is ever touched. + def make_commit(message, sha: "deadbeef" * 5, parent_count: 0) + data = { + "sha" => sha, + "committer" => "Test 1700000000 +0000", + "author" => "Test 1700000000 +0000", + "tree" => "a" * 40, + "parent" => Array.new(parent_count) { |i| "p#{i}" * 10 }, + "message" => message + } + described_class.new(nil, sha, data) + end + + describe "#subject" do + it "is the first line of the message, without the trailing newline" do + commit = make_commit("Fix the bug\n\nProblem: it broke.") + expect(commit.subject).to eq("Fix the bug") + end + + it "is the whole message when there is no description" do + commit = make_commit("Only a subject") + expect(commit.subject).to eq("Only a subject") + end + end + + describe "#subject_ticked" do + it "wraps the subject in backticks" do + commit = make_commit("Fix the bug") + expect(commit.subject_ticked).to eq("`Fix the bug`") + end + + it "replaces literal backticks in the subject with single quotes" do + commit = make_commit("Add `foo` helper") + expect(commit.subject_ticked).to eq("`Add 'foo' helper`") + end + end + + describe "#description" do + it "is everything after the subject and the blank line following it" do + commit = make_commit("Subject\n\nProblem: X.\n\nSolution: Y.") + expect(commit.description).to eq("Problem: X.\n\nSolution: Y.") + end + + it "drops every leading blank line, not just one" do + commit = make_commit("Subject\n\n\n\nBody after extra blanks") + expect(commit.description).to eq("Body after extra blanks") + end + + it "keeps the second line as-is when it is not blank" do + commit = make_commit("Subject\nDirectly attached body") + expect(commit.description).to eq("Directly attached body") + end + + it "is empty when the message has no second line at all" do + commit = make_commit("Only a subject") + expect(commit.description).to eq("") + end + end + + describe "#blank_line_after_subject?" do + it "is true when the subject is followed by a blank line" do + commit = make_commit("Subject\n\nBody") + expect(commit.blank_line_after_subject?).to be true + end + + it "is false when the body is directly attached to the subject" do + commit = make_commit("Subject\nBody") + expect(commit.blank_line_after_subject?).to be false + end + + it "is false when there is no second line at all" do + commit = make_commit("Only a subject") + expect(commit.blank_line_after_subject?).to be false + end + end + + describe "#subject_matches?" do + it "is true when the subject matches one of the given patterns" do + commit = make_commit("wip: still in progress") + expect(commit.subject_matches?([/\bwip\b/i])).to be true + end + + it "is false when the subject matches none of the given patterns" do + commit = make_commit("Fix the bug") + expect(commit.subject_matches?([/\bwip\b/i])).to be false + end + end + + describe "#short_ref" do + it "is the first 8 characters of the SHA" do + commit = make_commit("Subject", sha: "0123456789abcdef") + expect(commit.short_ref).to eq("01234567") + end + end +end From 23479b48701695af29b479c3a7a011b23f950b0c Mon Sep 17 00:00:00 2001 From: Heitor Toledo Lassarote de Paula Date: Tue, 25 Aug 2026 19:04:39 -0300 Subject: [PATCH 13/19] [#4] Add SerokellDanger::GitHost tests 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. --- spec/serokell_danger/git_host_spec.rb | 211 ++++++++++++++++++++++++++ 1 file changed, 211 insertions(+) create mode 100644 spec/serokell_danger/git_host_spec.rb diff --git a/spec/serokell_danger/git_host_spec.rb b/spec/serokell_danger/git_host_spec.rb new file mode 100644 index 0000000..eab5021 --- /dev/null +++ b/spec/serokell_danger/git_host_spec.rb @@ -0,0 +1,211 @@ +# SPDX-FileCopyrightText: 2026 Serokell +# +# SPDX-License-Identifier: MPL-2.0 + +require "serokell_danger" + +RSpec.describe SerokellDanger::GitHost do + describe ".detect" do + it "wraps dangerfile.github when it is present" do + dangerfile = double("dangerfile", github: double("gh"), gitlab: nil) + expect(described_class.detect(dangerfile).host).to eq(:github) + end + + it "wraps dangerfile.gitlab when github is absent" do + dangerfile = double("dangerfile", github: nil, gitlab: double("gl")) + expect(described_class.detect(dangerfile).host).to eq(:gitlab) + end + + it "is nil when neither github nor gitlab is present" do + dangerfile = double("dangerfile", github: nil, gitlab: nil) + expect(described_class.detect(dangerfile)).to be_nil + end + end + + describe "#github?/#gitlab?" do + it "is true for the host it was built with" do + github = described_class.new(double("plugin"), :github) + gitlab = described_class.new(double("plugin"), :gitlab) + expect(github.github?).to be true + expect(github.gitlab?).to be false + expect(gitlab.gitlab?).to be true + expect(gitlab.github?).to be false + end + end + + describe "#mr_title / #pr_title" do + it "delegates to plugin.pr_title on GitHub" do + plugin = double("plugin", pr_title: "My PR") + host = described_class.new(plugin, :github) + expect(host.mr_title).to eq("My PR") + expect(host.pr_title).to eq("My PR") + end + + it "delegates to plugin.mr_title on GitLab" do + plugin = double("plugin", mr_title: "My MR") + host = described_class.new(plugin, :gitlab) + expect(host.mr_title).to eq("My MR") + expect(host.pr_title).to eq("My MR") + end + end + + describe "#mr_body / #pr_body" do + it "delegates to plugin.pr_body on GitHub" do + plugin = double("plugin", pr_body: "Body text") + host = described_class.new(plugin, :github) + expect(host.mr_body).to eq("Body text") + expect(host.pr_body).to eq("Body text") + end + + it "delegates to plugin.mr_body on GitLab" do + plugin = double("plugin", mr_body: "Body text") + host = described_class.new(plugin, :gitlab) + expect(host.mr_body).to eq("Body text") + expect(host.pr_body).to eq("Body text") + end + end + + describe "#branch_for_head / #source_branch" do + it "delegates to plugin.branch_for_head regardless of host" do + plugin = double("plugin", branch_for_head: "feature-branch") + host = described_class.new(plugin, :github) + expect(host.branch_for_head).to eq("feature-branch") + expect(host.source_branch).to eq("feature-branch") + end + end + + describe "#branch_for_base / #target_branch" do + it "delegates to plugin.branch_for_base regardless of host" do + plugin = double("plugin", branch_for_base: "master") + host = described_class.new(plugin, :gitlab) + expect(host.branch_for_base).to eq("master") + expect(host.target_branch).to eq("master") + end + end + + describe "#mr_iid / #pr_iid" do + it "reads iid from the JSON payload on GitLab" do + plugin = double("plugin", mr_json: {"iid" => 7}) + host = described_class.new(plugin, :gitlab) + expect(host.mr_iid).to eq(7) + expect(host.pr_iid).to eq(7) + end + + it "reads number from the JSON payload on GitHub" do + plugin = double("plugin", pr_json: {"number" => 42}) + host = described_class.new(plugin, :github) + expect(host.mr_iid).to eq(42) + expect(host.pr_iid).to eq(42) + end + + it "is nil when the JSON payload can't be read" do + plugin = double("plugin") + allow(plugin).to receive(:mr_json).and_raise("boom") + host = described_class.new(plugin, :gitlab) + expect(host.mr_iid).to be_nil + end + end + + describe "#mr_url / #pr_url" do + it "reads web_url from the JSON payload on GitLab" do + plugin = double("plugin", mr_json: {"web_url" => "https://x/1"}) + host = described_class.new(plugin, :gitlab) + expect(host.mr_url).to eq("https://x/1") + expect(host.pr_url).to eq("https://x/1") + end + + it "reads html_url from the JSON payload on GitHub" do + plugin = double("plugin", pr_json: {"html_url" => "https://x/2"}) + host = described_class.new(plugin, :github) + expect(host.mr_url).to eq("https://x/2") + expect(host.pr_url).to eq("https://x/2") + end + end + + describe "#mr_draft? / #pr_draft?" do + it "delegates to plugin.pr_draft? on GitHub when it responds to it" do + plugin = double("plugin", pr_draft?: true) + host = described_class.new(plugin, :github) + expect(host.mr_draft?).to be true + expect(host.pr_draft?).to be true + end + + it "is true when the JSON payload's work_in_progress is truthy" do + plugin = double( + "plugin", mr_json: {"work_in_progress" => true}, mr_title: "Title" + ) + host = described_class.new(plugin, :gitlab) + expect(host.mr_draft?).to be true + end + + it "is true when the title matches the draft title pattern" do + plugin = double("plugin", mr_json: {}, mr_title: "WIP: something") + host = described_class.new(plugin, :gitlab) + expect(host.mr_draft?).to be true + end + + it "is false otherwise" do + plugin = double("plugin", mr_json: {}, mr_title: "Normal title") + host = described_class.new(plugin, :gitlab) + expect(host.mr_draft?).to be false + expect(host.pr_draft?).to be false + end + end + + describe "#mr_title_payload / #pr_title_payload" do + it "strips a [Draft] prefix from the title" do + plugin = double("plugin", mr_title: "[Draft] Add feature") + host = described_class.new(plugin, :gitlab) + expect(host.mr_title_payload).to eq("Add feature") + end + + it "strips a WIP: prefix from the title" do + plugin = double("plugin", mr_title: "WIP: Add feature") + host = described_class.new(plugin, :gitlab) + expect(host.mr_title_payload).to eq("Add feature") + end + + it "leaves a non-draft title unchanged" do + plugin = double("plugin", mr_title: "Add feature") + host = described_class.new(plugin, :gitlab) + expect(host.mr_title_payload).to eq("Add feature") + expect(host.pr_title_payload).to eq("Add feature") + end + end + + describe "#suggestion_fence" do + it "uses GitLab's suggestion:-above+below syntax on GitLab" do + host = described_class.new(double("plugin"), :gitlab) + expect(host.suggestion_fence(above: 1, below: 2)).to eq("suggestion:-1+2") + end + + it "uses plain 'suggestion' on GitHub with no offsets" do + host = described_class.new(double("plugin"), :github) + expect(host.suggestion_fence).to eq("suggestion") + end + + it "uses 'diff' on GitHub when there are offsets" do + host = described_class.new(double("plugin"), :github) + expect(host.suggestion_fence(above: 1)).to eq("diff") + end + end + + describe "#method_missing / #respond_to_missing?" do + it "delegates unknown methods to the plugin" do + plugin = double("plugin", custom_method: "hello") + host = described_class.new(plugin, :github) + expect(host.custom_method).to eq("hello") + end + + it "reports responding to methods the plugin responds to" do + plugin = double("plugin", custom_method: "hello") + host = described_class.new(plugin, :github) + expect(host.respond_to?(:custom_method)).to be true + end + + it "raises for methods neither GitHost nor the plugin define" do + host = described_class.new(double("plugin"), :github) + expect { host.totally_unknown_method }.to raise_error(NoMethodError) + end + end +end From d0a0f778bfe23aa1c3c33762b486a695c31b4ca3 Mon Sep 17 00:00:00 2001 From: Heitor Toledo Lassarote de Paula Date: Tue, 25 Aug 2026 19:34:10 -0300 Subject: [PATCH 14/19] [#4] Run RSpec in CI 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. --- .github/workflows/ci.yml | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f616923..495af2d 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -21,6 +21,17 @@ jobs: bundler-cache: true - run: bundle exec standardrb + rspec: + name: RSpec + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: ruby/setup-ruby@v1 + with: + ruby-version: "3.4" + bundler-cache: true + - run: bundle exec rspec + reuse: name: REUSE Compliance Check runs-on: ubuntu-latest From af9ecf59272a846586ecb2802e34e470754876e7 Mon Sep 17 00:00:00 2001 From: Heitor Toledo Lassarote de Paula Date: Tue, 25 Aug 2026 19:34:25 -0300 Subject: [PATCH 15/19] [#4] Add empty-diff/empty-commits edge case tests 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. --- spec/serokell_danger/commits_style_spec.rb | 7 +++++++ spec/serokell_danger/license_headers_spec.rb | 4 ++++ spec/serokell_danger/merge_commits_spec.rb | 4 ++++ spec/serokell_danger/premerge_commits_spec.rb | 4 ++++ spec/serokell_danger/trailing_whitespaces_spec.rb | 8 ++++++++ 5 files changed, 27 insertions(+) diff --git a/spec/serokell_danger/commits_style_spec.rb b/spec/serokell_danger/commits_style_spec.rb index 8127668..3de2ab6 100644 --- a/spec/serokell_danger/commits_style_spec.rb +++ b/spec/serokell_danger/commits_style_spec.rb @@ -203,4 +203,11 @@ def simple_test(commit, rules_to_test, **config_overrides) expect(dangerfile).not_to receive(:markdown) end end + + it "does nothing when there are no commits" do + config = make_config + set_commits + set_rule_receive_expectations({}) + dangerfile.check_commits_style(config) + end end diff --git a/spec/serokell_danger/license_headers_spec.rb b/spec/serokell_danger/license_headers_spec.rb index abd30db..3ec2b88 100644 --- a/spec/serokell_danger/license_headers_spec.rb +++ b/spec/serokell_danger/license_headers_spec.rb @@ -130,6 +130,10 @@ def simple_test(rules_to_test, added_files: [], modified_files: [], simple_test({}, added_files: [nonexistent_file]) end + it "does nothing when there are no tracked files at all" do + simple_test({}) + end + it "raises for an invalid scan value" do config = make_config(scan: :invalid) diff --git a/spec/serokell_danger/merge_commits_spec.rb b/spec/serokell_danger/merge_commits_spec.rb index 5c81b9a..63f71a5 100644 --- a/spec/serokell_danger/merge_commits_spec.rb +++ b/spec/serokell_danger/merge_commits_spec.rb @@ -99,4 +99,8 @@ def simple_test(commits, rules_to_test, **config_overrides) skip_if_title_matches: dangerfile.default_branch_merge_title_patterns ) end + + it "does nothing when there are no commits" do + simple_test([], {}) + end end diff --git a/spec/serokell_danger/premerge_commits_spec.rb b/spec/serokell_danger/premerge_commits_spec.rb index 6d93f46..26ae6d7 100644 --- a/spec/serokell_danger/premerge_commits_spec.rb +++ b/spec/serokell_danger/premerge_commits_spec.rb @@ -91,4 +91,8 @@ def simple_test(commits, rules_to_test, **config_overrides) skip_if_title_matches: dangerfile.default_branch_merge_title_patterns ) end + + it "does nothing when there are no commits" do + simple_test([], {}) + end end diff --git a/spec/serokell_danger/trailing_whitespaces_spec.rb b/spec/serokell_danger/trailing_whitespaces_spec.rb index c18a726..be5199e 100644 --- a/spec/serokell_danger/trailing_whitespaces_spec.rb +++ b/spec/serokell_danger/trailing_whitespaces_spec.rb @@ -139,4 +139,12 @@ def simple_test(fixture, rules_to_test, enable: rules_to_test.keys, skip_if_title_matches: dangerfile.default_branch_merge_title_patterns ) end + + it "does nothing when the diff is empty" do + rules = [:trailing_whitespace, :final_newline, :extra_final_newlines] + config = make_config(rules) + allow(dangerfile.git).to receive(:diff).and_return([]) + set_rule_receive_expectations({}) + dangerfile.check_trailing_whitespace(config) + end end From d4d5d5c9f2f534871b032899344d912aee38d354 Mon Sep 17 00:00:00 2001 From: Heitor Toledo Lassarote de Paula Date: Tue, 25 Aug 2026 19:50:16 -0300 Subject: [PATCH 16/19] [#4] Document how to run tests and lints 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. --- CONTRIBUTING.md | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 3b9e14b..ec0759b 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -25,6 +25,17 @@ Our pull request template contains a [checklist](.github/pull_request_template.m of acceptance criteria for your pull request. Please read it before you start contributing and make sure your contributions adhere to this checklist. +### Running tests and lints + +```sh +bundle install +bundle exec rspec # run the test suite +bundle exec standardrb # lint +bundle exec reuse lint # REUSE/SPDX compliance +``` + +All three run in CI on every pull request. + ## Legal We want to make sure that our projects come with correct licensing information From ab5b5c83763abed0c401999657c9842f5a86e765 Mon Sep 17 00:00:00 2001 From: Heitor Toledo Lassarote de Paula Date: Wed, 26 Aug 2026 17:46:37 -0300 Subject: [PATCH 17/19] [#4] Add test coverage reporting with SimpleCov 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. --- .github/workflows/ci.yml | 5 +++++ .gitignore | 3 +++ Gemfile | 1 + spec/spec_helper.rb | 6 ++++++ 4 files changed, 15 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 495af2d..4e93065 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -31,6 +31,11 @@ jobs: ruby-version: "3.4" bundler-cache: true - run: bundle exec rspec + - uses: actions/upload-artifact@v7.0.1 + if: always() + with: + name: coverage-report + path: coverage/ reuse: name: REUSE Compliance Check diff --git a/.gitignore b/.gitignore index 7f6cc26..dfea040 100644 --- a/.gitignore +++ b/.gitignore @@ -9,3 +9,6 @@ # Gem build artifacts /pkg/ /*.gem + +# Test coverage report +/coverage/ diff --git a/Gemfile b/Gemfile index a412bbe..c2a4e4a 100644 --- a/Gemfile +++ b/Gemfile @@ -13,4 +13,5 @@ end group :test do gem "rspec" + gem "simplecov", require: false end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 3e20910..0a8beb2 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -2,6 +2,12 @@ # # SPDX-License-Identifier: MPL-2.0 +require "simplecov" +SimpleCov.start do + add_filter "/spec/" + minimum_coverage 90 +end + require "danger" require "serokell_danger" From 3d764dc3ae625bc4b901d942742a29041d204355 Mon Sep 17 00:00:00 2001 From: Heitor Toledo Lassarote de Paula Date: Wed, 26 Aug 2026 18:14:32 -0300 Subject: [PATCH 18/19] [#4] Bump CI action versions and document xrefcheck/coverage 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. --- .github/workflows/ci.yml | 14 +++++++------- CONTRIBUTING.md | 9 ++++++++- 2 files changed, 15 insertions(+), 8 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 4e93065..1c4d1e3 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -14,7 +14,7 @@ jobs: name: Ruby lint (standardrb) runs-on: ubuntu-latest steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@v7 - uses: ruby/setup-ruby@v1 with: ruby-version: "3.4" @@ -25,13 +25,13 @@ jobs: name: RSpec runs-on: ubuntu-latest steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@v7 - uses: ruby/setup-ruby@v1 with: ruby-version: "3.4" bundler-cache: true - run: bundle exec rspec - - uses: actions/upload-artifact@v7.0.1 + - uses: actions/upload-artifact@v7 if: always() with: name: coverage-report @@ -41,14 +41,14 @@ jobs: name: REUSE Compliance Check runs-on: ubuntu-latest steps: - - uses: actions/checkout@v4 - - uses: fsfe/reuse-action@v1.1 + - uses: actions/checkout@v7 + - uses: fsfe/reuse-action@v6 xrefcheck: name: xrefcheck runs-on: ubuntu-latest steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@v7 - uses: serokell/xrefcheck-action@v1 danger: @@ -60,7 +60,7 @@ jobs: issues: read pull-requests: write steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@v7 with: fetch-depth: 0 - uses: ruby/setup-ruby@v1 diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index ec0759b..6d5b8fe 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -32,9 +32,16 @@ bundle install bundle exec rspec # run the test suite bundle exec standardrb # lint bundle exec reuse lint # REUSE/SPDX compliance +xrefcheck # check for broken links between files ``` -All three run in CI on every pull request. +All four run in CI on every pull request. `xrefcheck` isn't a Ruby +gem; see [serokell/xrefcheck](https://github.com/serokell/xrefcheck) +for installation instructions. + +`bundle exec rspec` also produces a test coverage report via +[SimpleCov](https://github.com/simplecov-ruby/simplecov), written to +`coverage/index.html`, and fails the run if coverage drops below 90%. ## Legal From 1fb78fe0a1d4760e32d985ccede02c943b497178 Mon Sep 17 00:00:00 2001 From: Heitor Toledo Lassarote de Paula Date: Wed, 26 Aug 2026 18:36:05 -0300 Subject: [PATCH 19/19] [#4] Test danger_report's own implementation 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). --- spec/serokell_danger/danger_report_spec.rb | 97 ++++++++++++++++++++++ spec/spec_helper.rb | 4 +- 2 files changed, 99 insertions(+), 2 deletions(-) create mode 100644 spec/serokell_danger/danger_report_spec.rb diff --git a/spec/serokell_danger/danger_report_spec.rb b/spec/serokell_danger/danger_report_spec.rb new file mode 100644 index 0000000..46bf34a --- /dev/null +++ b/spec/serokell_danger/danger_report_spec.rb @@ -0,0 +1,97 @@ +# SPDX-FileCopyrightText: 2026 Serokell +# +# SPDX-License-Identifier: MPL-2.0 + +RSpec.describe "danger_report" do + include_context "danger testing" + + def make_config(**overrides) + SerokellDanger::Config.new( + "test-check", + {severity: :warn}.merge(overrides), + configure_with: "check_test" + ) + end + + # method is the underlying report call danger_report is expected to make + # (:fail/:warn/:message/:markdown), or nil to expect none of them to fire. + # receive_kwargs are the keyword arguments expected on that call. + # report_kwargs are passed through to danger_report itself. + def simple_test(method, expected_text = anything, receive_kwargs: {}, + report_kwargs: {}, **config_overrides) + config = make_config(**config_overrides) + if method.nil? + %i[fail warn message markdown].each { |m| expect(dangerfile).not_to receive(m) } + else + expect(dangerfile).to receive(method).with(expected_text, **receive_kwargs) + end + dangerfile.danger_report(config, :rule, "Some text", **report_kwargs) + end + + it "does nothing when severity resolves to :off" do + simple_test(nil, severity: :off) + end + + it "reports via fail with a disable hint for :fail severity" do + simple_test( + :fail, + "[test-check/rule] Some text (To disable this rule: `rule: nil` in `check_test`.)", + severity: :fail + ) + end + + it "reports via warn with a disable hint for :warn severity" do + simple_test( + :warn, + "[test-check/rule] Some text (To disable this rule: `rule: nil` in `check_test`.)", + severity: :warn + ) + end + + it "reports via message with no disable hint for :message severity" do + simple_test(:message, "[test-check/rule] Some text", severity: :message) + end + + it "reports via markdown with no disable hint for :markdown severity" do + simple_test(:markdown, "[test-check/rule] Some text", severity: :markdown) + end + + it "uses a custom hint instead of the default one when given" do + simple_test( + :warn, + "[test-check/rule] Some text (To disable this rule: `custom: false` in `check_test`.)", + report_kwargs: {hint: "custom: false"}, + severity: :warn + ) + end + + it "omits the 'in `configure_with`' clause when the config has no configure_with" do + config = SerokellDanger::Config.new("test-check", {severity: :warn}) + expect(dangerfile).to receive(:warn).with( + "[test-check/rule] Some text (To disable this rule: `rule: nil`.)" + ) + dangerfile.danger_report(config, :rule, "Some text") + end + + it "passes extra keyword arguments through to the underlying report call" do + simple_test( + :warn, + receive_kwargs: {file: "foo.rb", line: 3}, + report_kwargs: {file: "foo.rb", line: 3}, + severity: :warn + ) + end + + it "lets an explicit severity: override win over the config" do + simple_test(:fail, report_kwargs: {severity: :fail}, severity: :off) + end + + it "raises for an unknown severity" do + config = make_config(severity: :bogus) + expect { dangerfile.danger_report(config, :rule, "Some text") }.to raise_error( + ArgumentError, + "Unknown severity `:bogus` for `test-check/rule`. " \ + "Expected one of: :fail, :warn, :message, :off." + ) + end +end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 0a8beb2..cd76c4a 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -4,8 +4,8 @@ require "simplecov" SimpleCov.start do - add_filter "/spec/" - minimum_coverage 90 + skip "/spec/" + minimum_coverage 95 end require "danger"