Merge pull request #95 from ombulabs/feature/validate-patterns-self-test

Add --self-test flag to bin/validate-patterns (closes #71)

Author: etagwerker

.github/workflows/validate-patterns.yml

@@ -25,3 +25,4 @@ jobs:
         with:
           ruby-version: "3.3"
       - run: ruby bin/validate-patterns
+      - run: ruby bin/validate-patterns --self-test

CLAUDE.md

@@ -7,6 +7,7 @@ This file captures project-specific conventions Claude should follow when workin
 - `bin/validate-patterns` validates every detection pattern YAML file under `rails-upgrade/detection-scripts/patterns/`. Run it before committing any change to a pattern file. Pure-stdlib Ruby, no Bundler or Gemfile required.
   - `bin/validate-patterns` validates every file
   - `bin/validate-patterns path/to/file.yml` validates one or more specific files
+  - `bin/validate-patterns --self-test` runs built-in fixture assertions covering the four positive `kind:` values and the four rejection paths (missing top-level key, missing required pattern key, broken regex, unknown `kind:` value). CI runs this alongside the file-validation step
   - Checks: YAML parses, required top-level keys present, the eight required pattern keys present on each entry, every `pattern` / `exclude` regex compiles, and the `kind:` value is one of the allowed enum values (`breaking`, `deprecation`, `migration`, `optional`)
   - Exits 0 on success, 1 on any failure with a per-file error report
 

bin/validate-patterns

@@ -6,10 +6,12 @@
 # Usage:
 #   bin/validate-patterns                # validate every patterns file
 #   bin/validate-patterns path/to/file   # validate a specific file (one or more)
+#   bin/validate-patterns --self-test    # run built-in fixture assertions
 #
-# Exits 0 if all files pass, 1 otherwise.
+# Exits 0 if all files / assertions pass, 1 otherwise.
 
 require "yaml"
+require "tmpdir"
 
 PATTERNS_DIR = File.expand_path("../rails-upgrade/detection-scripts/patterns", __dir__)
 TOP_LEVEL_KEYS = %w[version description upgrade_findings].freeze
@@ -72,6 +74,90 @@ def validate(path)
   errors
 end
 
+def self_test
+  base = {
+    "version" => "test",
+    "description" => "self-test fixture",
+    "upgrade_findings" => {
+      "high_priority" => [
+        {
+          "name" => "Test entry",
+          "kind" => "breaking",
+          "pattern" => "foo",
+          "exclude" => "",
+          "search_paths" => ["app/"],
+          "explanation" => "test",
+          "fix" => "test",
+          "variable_name" => "TEST"
+        }
+      ]
+    }
+  }
+
+  cases = []
+
+  KIND_VALUES.each do |kind|
+    doc = Marshal.load(Marshal.dump(base))
+    doc["upgrade_findings"]["high_priority"][0]["kind"] = kind
+    cases << { name: "valid kind=#{kind}", doc: doc, expect: :pass }
+  end
+
+  missing_top = Marshal.load(Marshal.dump(base))
+  missing_top.delete("upgrade_findings")
+  cases << { name: "missing upgrade_findings", doc: missing_top, expect: :fail, match: /missing top-level key: upgrade_findings/ }
+
+  missing_kind = Marshal.load(Marshal.dump(base))
+  missing_kind["upgrade_findings"]["high_priority"][0].delete("kind")
+  cases << { name: "missing pattern key kind", doc: missing_kind, expect: :fail, match: /missing key: kind/ }
+
+  bad_regex = Marshal.load(Marshal.dump(base))
+  bad_regex["upgrade_findings"]["high_priority"][0]["pattern"] = "[unclosed"
+  cases << { name: "broken regex", doc: bad_regex, expect: :fail, match: /pattern regex does not compile/ }
+
+  bad_kind = Marshal.load(Marshal.dump(base))
+  bad_kind["upgrade_findings"]["high_priority"][0]["kind"] = "bogus"
+  cases << { name: "unknown kind value", doc: bad_kind, expect: :fail, match: /invalid kind: "bogus"/ }
+
+  passed = 0
+  failed = []
+
+  Dir.mktmpdir do |dir|
+    cases.each_with_index do |c, i|
+      path = File.join(dir, "fixture_#{i}.yml")
+      File.write(path, c[:doc].to_yaml)
+      errors = validate(path)
+
+      ok =
+        case c[:expect]
+        when :pass then errors.empty?
+        when :fail then errors.any? { |e| e =~ c[:match] }
+        end
+
+      if ok
+        passed += 1
+      else
+        failed << { name: c[:name], expected: c[:expect], errors: errors }
+      end
+    end
+  end
+
+  total = cases.size
+  if failed.empty?
+    puts "OK   self-test (#{passed}/#{total} passed)"
+    0
+  else
+    puts "FAIL self-test (#{passed}/#{total} passed)"
+    failed.each do |f|
+      puts "     - #{f[:name]} (expected #{f[:expected]}, got errors: #{f[:errors].inspect})"
+    end
+    1
+  end
+end
+
+if ARGV.include?("--self-test")
+  exit self_test
+end
+
 paths =
   if ARGV.empty?
     Dir.glob(File.join(PATTERNS_DIR, "*.yml")).sort

rails-upgrade/CHANGELOG.md

@@ -5,6 +5,7 @@
 - `bin/validate-patterns` now **requires** the `kind:` field on every pattern entry (issue #53 rollout, sub-issue #67). All 12 pattern files have been classified across the preceding 12 sub-issues. Updated `CLAUDE.md` to list `kind:` among the required per-pattern fields and to remove the rollout caveat.
 - Renamed the top-level pattern-file key `breaking_changes:` → `upgrade_findings:` (issue #53 rollout, sub-issue #68). The old name was a misnomer — most entries are deprecations, migrations, or opt-in features rather than hard-breaking changes. Touched all 12 `rails-*-patterns.yml` files, `bin/validate-patterns` (the `TOP_LEVEL_KEYS` constant and the lookup variable in `validate`), and `rails-upgrade/workflows/direct-detection-workflow.md` (5 references). The validator now rejects files that still use the old key.
 - Updated `workflows/direct-detection-workflow.md` to surface the new `kind:` field in detection output (issue #53 rollout, sub-issue #69). Findings are now grouped into two top-level buckets: **fix-before-bump** (`kind: breaking` and `deprecation`) and **fix-when-ready** (`kind: migration` and `optional`). Within each bucket, `priority` (HIGH / MEDIUM / LOW) drives sub-ordering. Putting `breaking` and `deprecation` together reflects the practical reality that deprecations warn in production logs at this hop and typically become hard breaks at the next — addressing them in the same upgrade campaign is cheaper than splitting the work across two upgrades. `migration` and `optional` are silent at this hop and don't compete for the user's attention during the upgrade itself.
+- Added a `--self-test` flag to `bin/validate-patterns` (closes #71). Running `bin/validate-patterns --self-test` writes valid + invalid fixture YAMLs to a tmpdir and asserts the validator's behavior end-to-end: four positive cases (one per `kind:` value) and four rejection paths (missing top-level key, missing required pattern key, broken regex, unknown `kind:` value). Wired into the GitHub Actions workflow. Closes the test-coverage gap surfaced during PR #70 review where the new enum guard had no automated coverage. Single-file, stdlib-only — no Bundler, no test framework.
 
 ## v3.3.0, 28 April 2026
 - Added `upgrade-cleanup` companion plugin for finishing a Rails upgrade campaign. Removes dual-boot scaffolding, drops `NextRails.next?` / `NextRails.current?` branches, retires stale monkey-patches and version-conditional code, and aligns CI matrix / Dockerfile / Ruby pin to the new version baseline. Lives as a sibling plugin in this repo (`upgrade-cleanup/.claude-plugin/plugin.json` + `upgrade-cleanup/upgrade-cleanup/SKILL.md` + `upgrade-cleanup/upgrade-cleanup/workflows/upgrade-cleanup-workflow.md`). `load_defaults` alignment and deprecation triage stay with the rails-upgrade skill, cleanup deliberately does not duplicate them. Based on FastRuby.io's "Finishing an Upgrade" methodology. (Closes #1)