diff --git a/.github/workflows/integration.yml b/.github/workflows/integration.yml index 802f93a..83b68fa 100644 --- a/.github/workflows/integration.yml +++ b/.github/workflows/integration.yml @@ -54,7 +54,7 @@ jobs: with: github-token: ${{ secrets.GITHUB_TOKEN }} dry-run: "true" - working-directory: /tmp/test-app + rails-root: /tmp/test-app - name: Assert all outdated packages appear in dry-run output run: | diff --git a/Gemfile b/Gemfile index 34af3d7..c25b3cb 100644 --- a/Gemfile +++ b/Gemfile @@ -1,9 +1,12 @@ source "https://rubygems.org" ruby ">= 3.2" +gem "octokit" + group :development, :test do gem "rake" gem "minitest" + gem "minitest-mock", "~> 5.27" end group :development do diff --git a/Gemfile.lock b/Gemfile.lock index 140c829..27a331b 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,19 +1,35 @@ GEM remote: https://rubygems.org/ specs: + addressable (2.9.0) + public_suffix (>= 2.0.2, < 8.0) ast (2.4.3) drb (2.2.3) + faraday (2.14.2) + faraday-net_http (>= 2.0, < 3.5) + json + logger + faraday-net_http (3.4.2) + net-http (~> 0.5) json (2.19.5) language_server-protocol (3.17.0.5) lint_roller (1.1.0) + logger (1.7.0) minitest (6.0.6) drb (~> 2.0) prism (~> 1.5) + minitest-mock (5.27.0) + net-http (0.9.1) + uri (>= 0.11.1) + octokit (10.0.0) + faraday (>= 1, < 3) + sawyer (~> 0.9) parallel (1.28.0) parser (3.3.11.1) ast (~> 2.4.1) racc prism (1.9.0) + public_suffix (7.0.5) racc (1.8.1) rainbow (3.1.1) rake (13.4.2) @@ -37,6 +53,9 @@ GEM rubocop (>= 1.75.0, < 2.0) rubocop-ast (>= 1.47.1, < 2.0) ruby-progressbar (1.13.0) + sawyer (0.9.3) + addressable (>= 2.3.5) + faraday (>= 0.17.3, < 3) standard (1.54.0) language_server-protocol (~> 3.17.0.2) lint_roller (~> 1.0) @@ -54,6 +73,7 @@ GEM unicode-display_width (3.2.0) unicode-emoji (~> 4.1) unicode-emoji (4.2.0) + uri (1.1.1) PLATFORMS arm64-darwin-25 @@ -61,19 +81,29 @@ PLATFORMS DEPENDENCIES minitest + minitest-mock (~> 5.27) + octokit rake standardrb CHECKSUMS + addressable (2.9.0) sha256=7fdf6ac3660f7f4e867a0838be3f6cf722ace541dd97767fa42bc6cfa980c7af ast (2.4.3) sha256=954615157c1d6a382bc27d690d973195e79db7f55e9765ac7c481c60bdb4d383 drb (2.2.3) sha256=0b00d6fdb50995fe4a45dea13663493c841112e4068656854646f418fda13373 + faraday (2.14.2) sha256=73ccb9994a9e8648f010e32eca2ae82e41c57860aa10932cda29418b9e0223ad + faraday-net_http (3.4.2) sha256=f147758260d3526939bf57ecf911682f94926a3666502e24c69992765875906c json (2.19.5) sha256=218a18553e4801d579ca7e0f5bc72bafd776d7397238a1fb4e74db5b0a812c59 language_server-protocol (3.17.0.5) sha256=fd1e39a51a28bf3eec959379985a72e296e9f9acfce46f6a79d31ca8760803cc lint_roller (1.1.0) sha256=2c0c845b632a7d172cb849cc90c1bce937a28c5c8ccccb50dfd46a485003cc87 + logger (1.7.0) sha256=196edec7cc44b66cfb40f9755ce11b392f21f7967696af15d274dde7edff0203 minitest (6.0.6) sha256=153ea36d1d987a62942382b61075745042a2b3123b1cd48f4c3675af9cc7d6f1 + minitest-mock (5.27.0) sha256=7040ed7185417a966920987eaa6eaf1be4ea1fc5b25bb03ff4703f98564a55b0 + net-http (0.9.1) sha256=25ba0b67c63e89df626ed8fac771d0ad24ad151a858af2cc8e6a716ca4336996 + octokit (10.0.0) sha256=82e99a539b7637b7e905e6d277bb0c1a4bed56735935cc33db6da7eae49a24e8 parallel (1.28.0) sha256=33e6de1484baf2524792d178b0913fc8eb94c628d6cfe45599ad4458c638c970 parser (3.3.11.1) sha256=d17ace7aabe3e72c3cc94043714be27cc6f852f104d81aa284c2281aecc65d54 prism (1.9.0) sha256=7b530c6a9f92c24300014919c9dcbc055bf4cdf51ec30aed099b06cd6674ef85 + public_suffix (7.0.5) sha256=1a8bb08f1bbea19228d3bed6e5ed908d1cb4f7c2726d18bd9cadf60bc676f623 racc (1.8.1) sha256=4a7f6929691dbec8b5209a0b373bc2614882b55fc5d2e447a21aaa691303d62f rainbow (3.1.1) sha256=039491aa3a89f42efa1d6dec2fc4e62ede96eb6acd95e52f1ad581182b79bc6a rake (13.4.2) sha256=cb825b2bd5f1f8e91ca37bddb4b9aaf345551b4731da62949be002fa89283701 @@ -82,12 +112,14 @@ CHECKSUMS rubocop-ast (1.49.1) sha256=4412f3ee70f6fe4546cc489548e0f6fcf76cafcfa80fa03af67098ffed755035 rubocop-performance (1.26.1) sha256=cd19b936ff196df85829d264b522fd4f98b6c89ad271fa52744a8c11b8f71834 ruby-progressbar (1.13.0) sha256=80fc9c47a9b640d6834e0dc7b3c94c9df37f08cb072b7761e4a71e22cff29b33 + sawyer (0.9.3) sha256=0d0f19298408047037638639fe62f4794483fb04320269169bd41af2bdcf5e41 standard (1.54.0) sha256=7a4b08f83d9893083c8f03bc486f0feeb6a84d48233b40829c03ef4767ea0100 standard-custom (1.0.2) sha256=424adc84179a074f1a2a309bb9cf7cd6bfdb2b6541f20c6bf9436c0ba22a652b standard-performance (1.9.0) sha256=49483d31be448292951d80e5e67cdcb576c2502103c7b40aec6f1b6e9c88e3f2 standardrb (1.0.1) sha256=7a1328be429f4e97a97e357e2446f3509e80164a59ff00bc6a4daa78e3351f2c unicode-display_width (3.2.0) sha256=0cdd96b5681a5949cdbc2c55e7b420facae74c4aaf9a9815eee1087cb1853c42 unicode-emoji (4.2.0) sha256=519e69150f75652e40bf736106cfbc8f0f73aa3fb6a65afe62fefa7f80b0f80f + uri (1.1.1) sha256=379fa58d27ffb1387eaada68c749d1426738bd0f654d812fcc07e7568f5c57c6 RUBY VERSION ruby 4.0.1 diff --git a/action.yml b/action.yml index c271563..5b38e28 100644 --- a/action.yml +++ b/action.yml @@ -11,7 +11,7 @@ inputs: required: false default: ".github/importmap-updates.yml" github-token: - description: "Token used by gh to list, open, edit, and close PRs." + description: "Token used by Octokit to list, open, edit, and close PRs." required: true base-branch: description: "Branch to base PRs against." @@ -32,7 +32,7 @@ inputs: description: "Git author email for commits." required: false default: "github-actions[bot]@users.noreply.github.com" - working-directory: + rails-root: description: "Directory containing the Rails app to run against. Useful when the app lives in a subdirectory or for integration testing." required: false default: "." @@ -43,14 +43,15 @@ runs: - name: Set up Ruby uses: ruby/setup-ruby@v1 with: - bundler-cache: true - ruby-version: ${{ inputs.ruby-version }} + bundler-cache: false + working-directory: ${{ github.action_path }} - name: Run importmap-update shell: bash - working-directory: ${{ inputs.working-directory }} + working-directory: ${{ github.action_path }} env: - GH_TOKEN: ${{ inputs.github-token }} + BUNDLE_GEMFILE: ${{ github.action_path }}/Gemfile + RAILS_ROOT: ${{ inputs.rails-root }} GITHUB_TOKEN: ${{ inputs.github-token }} GITHUB_REPOSITORY: ${{ github.repository }} INPUT_CONFIG_FILE: ${{ inputs.config-file }} @@ -58,4 +59,6 @@ runs: IMPORTMAP_DRY_RUN: ${{ inputs.dry-run }} IMPORTMAP_AUTHOR_NAME: ${{ inputs.author-name }} IMPORTMAP_AUTHOR_EMAIL: ${{ inputs.author-email }} - run: ${{ github.action_path }}/exe/importmap-update 2>&1 | tee "${IMPORTMAP_RUN_LOG:-/dev/null}" + run: | + bundle install + bundle exec exe/importmap-update diff --git a/exe/importmap-update b/exe/importmap-update index ed3f0ee..4fa637c 100755 --- a/exe/importmap-update +++ b/exe/importmap-update @@ -17,12 +17,17 @@ # INPUT_CONFIG_FILE Path to the YAML config (default # .github/importmap-updates.yml). # GITHUB_REPOSITORY OWNER/REPO of the consuming repo. -# GITHUB_TOKEN / GH_TOKEN Auth for gh (the action.yml passes this in). +# GITHUB_TOKEN / GH_TOKEN Auth token passed to Octokit (action.yml passes this in). # IMPORTMAP_BASE_BRANCH Base branch for PRs (default `main`). # IMPORTMAP_DRY_RUN Set to "true" for a no-side-effects run. # IMPORTMAP_AUTHOR_NAME Git author name for commits. # IMPORTMAP_AUTHOR_EMAIL Git author email for commits. - +# +Bundler.with_unbundled_env do + Dir.chdir(ENV.fetch("RAILS_ROOT", ".")) do + system("bin/importmap", "outdated") + end +end $LOAD_PATH.unshift File.expand_path("../lib", __dir__) require "optparse" @@ -31,7 +36,7 @@ require "config" require "planner" require "reconciler" require "executor" -require "gh_client" +require "github_client" require "git_client" require "commands" require "parsers/outdated_parser" @@ -156,8 +161,14 @@ when :run exit 2 end - runner = Importmap::Update::Commands::ShellRunner.new - gh = Importmap::Update::GhClient.new(repo: repo, runner: runner) + token = ENV["GITHUB_TOKEN"] || ENV["GH_TOKEN"] + if token.nil? || token.empty? + warn "GITHUB_TOKEN (or GH_TOKEN) is not set; refusing to run." + exit 2 + end + + runner = Importmap::Update::Commands::ShellRunner.new(cwd: ENV.fetch("RAILS_ROOT", ".")) + gh = Importmap::Update::GitHubClient.new(repo: repo, token: token) git = Importmap::Update::GitClient.new( runner: runner, author_name: ENV.fetch("IMPORTMAP_AUTHOR_NAME", "github-actions[bot]"), diff --git a/lib/commands.rb b/lib/commands.rb index ec06b45..c33534d 100644 --- a/lib/commands.rb +++ b/lib/commands.rb @@ -47,8 +47,10 @@ def initialize(cwd: nil, env: nil) def run(*argv) opts = {} opts[:chdir] = @cwd if @cwd - stdout, stderr, status = Open3.capture3(@env, *argv, **opts) - Result.new(stdout: stdout, stderr: stderr, exit_code: status.exitstatus) + Bundler.with_unbundled_env do + stdout, stderr, status = Open3.capture3(@env, *argv, opts) + Result.new(stdout: stdout, stderr: stderr, exit_code: status.exitstatus) + end end # Raises on non-zero exit. Use when you have no recovery strategy diff --git a/lib/executor.rb b/lib/executor.rb index ce8f806..3ecd649 100644 --- a/lib/executor.rb +++ b/lib/executor.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true require_relative "commands" -require_relative "gh_client" +require_relative "github_client" require_relative "git_client" require_relative "metadata" @@ -44,7 +44,7 @@ def failed? def initialize( gh:, git:, - base_branch:, commit_message_prefix:, runner: Commands::ShellRunner.new, + base_branch:, commit_message_prefix:, runner: Commands::ShellRunner.new(cwd: ENV.fetch("RAILS_ROOT", ".")), labels: [], dry_run: false, body_renderer: nil diff --git a/lib/gh_client.rb b/lib/gh_client.rb deleted file mode 100644 index 02dff18..0000000 --- a/lib/gh_client.rb +++ /dev/null @@ -1,156 +0,0 @@ -# frozen_string_literal: true - -require "json" -require_relative "commands" -require_relative "reconciler" - -module Importmap - module Update - # Wraps the `gh` CLI to give the executor a typed interface against - # GitHub. Every method shells out through a Commands::ShellRunner (or a - # FixtureRunner in tests). The wrapper does no orchestration logic — - # that's the executor's job — but it does parse `gh`'s JSON output into - # the structs the rest of the codebase already knows how to handle. - # - # Authentication: `gh` reads $GH_TOKEN or $GITHUB_TOKEN from the - # environment. Inside a GitHub Action, ${{ secrets.GITHUB_TOKEN }} - # passed as the `github-token` input is exposed as $GITHUB_TOKEN - # in the shell where this runs. - # - # Repository: every `gh` invocation uses --repo OWNER/REPO so we don't - # depend on the current directory being a git checkout pointed at the - # right remote. The action.yml will pass $GITHUB_REPOSITORY through. - class GhClient - # We deliberately cap below GitHub's hard limit. A consumer with more - # than this many bot-PRs open simultaneously has bigger problems - # than the action can fix, and we surface a warning in that case. - MAX_OPEN_PRS = 100 - - def initialize(repo:, runner: Commands::ShellRunner.new) - @repo = repo - @runner = runner - end - - # Returns an array of Reconciler::ExistingPR for all open PRs whose - # branch starts with `branch_prefix`. The body field is included so - # the reconciler can parse the metadata block out of it. - def list_open_prs(branch_prefix:) - # `head:foo/` is a GitHub-search-syntax prefix match. We can't use - # `gh pr list --head` because that's an exact-match filter. - result = @runner.run!( - "gh", "pr", "list", - "--repo", @repo, - "--state", "open", - "--search", "head:#{branch_prefix}/", - "--limit", MAX_OPEN_PRS.to_s, - "--json", "number,headRefName,title,body" - ) - parse_pr_list(result.stdout, branch_prefix) - end - - # Creates a new PR. Branch must already exist on the remote. - # Returns the new PR's number. - def create_pr(branch:, base:, title:, body:, labels: []) - argv = [ - "gh", "pr", "create", - "--repo", @repo, - "--head", branch, - "--base", base, - "--title", title, - "--body", body - ] - labels.each { |l| argv.push("--label", l) } - result = @runner.run!(*argv) - # `gh pr create` prints the PR URL on stdout; extract the number. - result.stdout.strip[%r{/pull/(\d+)}, 1]&.to_i - end - - # Ensures every label in +labels+ exists in the repo, creating any that - # are missing. Called once before opening PRs so that `create_pr` never - # fails with "label does not exist". Missing labels are created with a - # neutral default color; existing labels are left untouched. - def ensure_labels(labels) - return if labels.empty? - existing = list_label_names - labels.each do |label| - next if existing.include?(label) - @runner.run( - "gh", "label", "create", label, - "--repo", @repo, - "--color", "0075ca" - ) - end - end - - # Edits an existing PR's title and body. Used after force-pushing a - # changed branch — the body must be re-rendered so the metadata - # block reflects the new package set. - def update_pr(number:, title:, body:) - @runner.run!( - "gh", "pr", "edit", number.to_s, - "--repo", @repo, - "--title", title, - "--body", body - ) - nil - end - - # Closes a PR, optionally leaving a comment explaining why. The - # comment is what tells reviewers "this was managed by the action - # and was closed because…", which beats a silent close every time. - def close_pr(number:, comment: nil) - if comment && !comment.empty? - @runner.run!( - "gh", "pr", "comment", number.to_s, - "--repo", @repo, - "--body", comment - ) - end - @runner.run!( - "gh", "pr", "close", number.to_s, - "--repo", @repo - ) - nil - end - - private - - def list_label_names - result = @runner.run( - "gh", "label", "list", - "--repo", @repo, - "--json", "name", - "--limit", "200" - ) - return [] unless result.success? - JSON.parse(result.stdout.force_encoding("UTF-8")).map { |l| l["name"] } - rescue JSON::ParserError - [] - end - - def parse_pr_list(stdout, branch_prefix) - parsed = JSON.parse(stdout.force_encoding("UTF-8")) - # `head:foo/` is a *search* term, not a strict filter — GitHub's - # search can return PRs whose branch matches loosely. Belt and - # suspenders: re-filter on the client side too. - parsed - .select { |pr| pr["headRefName"].to_s.start_with?("#{branch_prefix}/") } - .map do |pr| - Reconciler::ExistingPR.new( - number: pr["number"], - branch: pr["headRefName"], - title: pr["title"], - body: pr["body"].to_s - ) - end - rescue JSON::ParserError => e - raise Commands::CommandError.new( - ["gh", "pr", "list"], - Commands::Result.new( - stdout: stdout, stderr: "Invalid JSON from gh: #{e.message}", exit_code: 1 - ) - ) - end - end - end -end diff --git a/lib/git_client.rb b/lib/git_client.rb index 9b9cbe9..a7afd9f 100644 --- a/lib/git_client.rb +++ b/lib/git_client.rb @@ -9,7 +9,7 @@ module Update # force_push action). Every command runs through the injected runner # so tests can replay fixtures the same way they do for gh. class GitClient - def initialize(author_name:, author_email:, runner: Commands::ShellRunner.new) + def initialize(author_name:, author_email:, runner: Commands::ShellRunner.new(cwd: ENV.fetch("RAILS_ROOT", "."))) @runner = runner @author_name = author_name @author_email = author_email diff --git a/lib/github_client.rb b/lib/github_client.rb new file mode 100644 index 0000000..5e536b2 --- /dev/null +++ b/lib/github_client.rb @@ -0,0 +1,87 @@ +# frozen_string_literal: true + +require "octokit" +require_relative "reconciler" + +module Importmap + module Update + # Wraps the Octokit client to give the executor a typed interface against + # GitHub. Every method calls the injected Octokit client (or a test double). + # The wrapper does no orchestration logic — that's the executor's job — but + # it does translate Octokit responses into the structs the rest of the + # codebase already knows how to handle. + # + # Authentication: pass the token directly; inside a GitHub Action use + # ${{ secrets.GITHUB_TOKEN }} exposed as $GITHUB_TOKEN in the shell. + class GitHubClient + # Matches the old gh cap; GitHub's REST API allows up to 100 per_page. + MAX_OPEN_PRS = 100 + + def initialize(repo:, token:, client: nil) + @repo = repo + @client = client || Octokit::Client.new(access_token: token) + end + + # Returns an array of Reconciler::ExistingPR for all open PRs whose + # branch starts with `branch_prefix`. Fetches up to MAX_OPEN_PRS open + # PRs and filters locally — GitHub's REST endpoint doesn't support + # branch-name prefix filtering. + def list_open_prs(branch_prefix:) + prs = @client.pull_requests(@repo, state: "open", per_page: MAX_OPEN_PRS) + prs + .select { |pr| pr.head.ref.start_with?("#{branch_prefix}/") } + .map do |pr| + Reconciler::ExistingPR.new( + number: pr.number, + branch: pr.head.ref, + title: pr.title, + body: pr.body.to_s + ) + end + end + + # Creates a new PR. Branch must already exist on the remote. + # Returns the new PR's number. + def create_pr(branch:, base:, title:, body:, labels: []) + pr = @client.create_pull_request(@repo, base, branch, title, body) + @client.add_labels_to_an_issue(@repo, pr.number, labels) unless labels.empty? + pr.number + end + + # Ensures every label in +labels+ exists in the repo, creating any that + # are missing. Missing labels are created with a neutral default color; + # existing labels are left untouched. + def ensure_labels(labels) + return if labels.empty? + existing = list_label_names + labels.each do |label| + next if existing.include?(label) + @client.create_label(@repo, label, "0075ca") + end + end + + # Edits an existing PR's title and body. + def update_pr(number:, title:, body:) + @client.update_pull_request(@repo, number, title: title, body: body) + nil + end + + # Closes a PR, optionally leaving a comment explaining why. + def close_pr(number:, comment: nil) + if comment && !comment.empty? + @client.add_comment(@repo, number, comment) + end + @client.close_pull_request(@repo, number) + nil + end + + private + + def list_label_names + @client.labels(@repo).map(&:name) + rescue Octokit::Error + [] + end + end + end +end diff --git a/test/executor_test.rb b/test/executor_test.rb index 81e59ad..fca9e38 100644 --- a/test/executor_test.rb +++ b/test/executor_test.rb @@ -13,7 +13,7 @@ class ExecutorTest < Minitest::Test # ---- fakes ---- - # Spy GhClient that records calls and lets tests configure the PR number + # Spy GitHubClient that records calls and lets tests configure the PR number # returned by create_pr. class FakeGh attr_reader :created, :updated, :closed diff --git a/test/gh_client_test.rb b/test/gh_client_test.rb deleted file mode 100644 index 1f45351..0000000 --- a/test/gh_client_test.rb +++ /dev/null @@ -1,277 +0,0 @@ -# frozen_string_literal: true - -require_relative "test_helper" -require "gh_client" -require "commands" - -class GhClientTest < Minitest::Test - GhClient = Importmap::Update::GhClient - Commands = Importmap::Update::Commands - - REPO = "example-org/example-repo" - - def setup - @runner = Commands::FixtureRunner.new - @client = GhClient.new(repo: REPO, runner: @runner) - end - - # ---- list_open_prs ---- - - def test_list_open_prs_invokes_gh_with_expected_arguments - @runner.add( - pattern: [ - "gh", "pr", "list", "--repo", REPO, - "--state", "open", - "--search", "head:importmap-updates/", - "--limit", "100", - "--json", "number,headRefName,title,body" - ], - stdout: +"[]" - ) - @client.list_open_prs(branch_prefix: "importmap-updates") - assert_equal 1, @runner.calls.size - end - - def test_list_open_prs_parses_a_realistic_response - @runner.add( - pattern: [ - "gh", "pr", "list", "--repo", REPO, - "--state", "open", - "--search", "head:importmap-updates/", - "--limit", "100", - "--json", "number,headRefName,title,body" - ], - stdout: fixture("gh_pr_list_mixed.json") - ) - - prs = @client.list_open_prs(branch_prefix: "importmap-updates") - - assert_equal 3, prs.size - assert_equal 100, prs[0].number - assert_equal "importmap-updates/security-lodash", prs[0].branch - assert_includes prs[0].body, "importmap-update:metadata" - - # The hand-written foreign PR comes through too — filtering it as - # "foreign" is the reconciler's job, not the client's. - assert_equal 102, prs[2].number - refute_includes prs[2].body, "importmap-update:metadata" - end - - def test_list_open_prs_filters_out_branches_that_dont_actually_start_with_the_prefix - # GitHub's search syntax for `head:` is a prefix match, but it's a - # *search*, not a strict filter — partial matches can leak through. - # Belt and suspenders: re-filter client-side. - @runner.add( - pattern: [ - "gh", "pr", "list", "--repo", REPO, - "--state", "open", - "--search", "head:importmap-updates/", - "--limit", "100", - "--json", "number,headRefName,title,body" - ], - stdout: +<<~JSON - [ - { "number": 1, "headRefName": "importmap-updates/patch", "title": "ours", "body": "" }, - { "number": 2, "headRefName": "importmap-updates-related-feature", "title": "leak", "body": "" } - ] - JSON - ) - prs = @client.list_open_prs(branch_prefix: "importmap-updates") - assert_equal [1], prs.map(&:number) - end - - def test_list_open_prs_raises_on_invalid_json - @runner.add( - pattern: [ - "gh", "pr", "list", "--repo", REPO, - "--state", "open", - "--search", "head:importmap-updates/", - "--limit", "100", - "--json", "number,headRefName,title,body" - ], - stdout: +"not actually json" - ) - err = assert_raises(Commands::CommandError) do - @client.list_open_prs(branch_prefix: "importmap-updates") - end - assert_includes err.message, "Invalid JSON from gh" - end - - def test_list_open_prs_propagates_gh_failure - @runner.add( - pattern: [ - "gh", "pr", "list", "--repo", REPO, - "--state", "open", - "--search", "head:importmap-updates/", - "--limit", "100", - "--json", "number,headRefName,title,body" - ], - stderr: "auth required", - exit_code: 1 - ) - err = assert_raises(Commands::CommandError) do - @client.list_open_prs(branch_prefix: "importmap-updates") - end - assert_includes err.message, "auth required" - end - - # ---- ensure_labels ---- - - def test_ensure_labels_is_a_no_op_when_labels_is_empty - @client.ensure_labels([]) - assert_equal 0, @runner.calls.size - end - - def test_ensure_labels_creates_missing_labels - @runner.add( - pattern: ["gh", "label", "list", "--repo", REPO, "--json", "name", "--limit", "200"], - stdout: +%([{"name":"dependencies"}]) - ) - @runner.add( - pattern: ["gh", "label", "create", "javascript", "--repo", REPO, "--color", "0075ca"], - stdout: +"" - ) - @client.ensure_labels(%w[dependencies javascript]) - assert_equal 2, @runner.calls.size - end - - def test_ensure_labels_skips_labels_that_already_exist - @runner.add( - pattern: ["gh", "label", "list", "--repo", REPO, "--json", "name", "--limit", "200"], - stdout: +%([{"name":"dependencies"},{"name":"javascript"}]) - ) - @client.ensure_labels(%w[dependencies javascript]) - assert_equal 1, @runner.calls.size - end - - def test_ensure_labels_tolerates_gh_label_list_failure - @runner.add( - pattern: ["gh", "label", "list", "--repo", REPO, "--json", "name", "--limit", "200"], - stderr: "not found", - exit_code: 1 - ) - @runner.add( - pattern: ["gh", "label", "create", "dependencies", "--repo", REPO, "--color", "0075ca"], - stdout: +"" - ) - @client.ensure_labels(%w[dependencies]) - assert_equal 2, @runner.calls.size - end - - # ---- create_pr ---- - - def test_create_pr_invokes_gh_with_title_body_branch_and_base - @runner.add( - pattern: [ - "gh", "pr", "create", - "--repo", REPO, - "--head", "importmap-updates/patch", - "--base", "main", - "--title", "chore(deps): patch updates", - "--body", "PR body text" - ], - stdout: fixture("gh_pr_create_success.txt") - ) - - number = @client.create_pr( - branch: "importmap-updates/patch", - base: "main", - title: "chore(deps): patch updates", - body: "PR body text" - ) - assert_equal 123, number - end - - def test_create_pr_passes_labels_individually - # `gh pr create` accepts repeated --label flags; we want each label - # on its own --label so values containing commas don't get split. - @runner.add( - pattern: [ - "gh", "pr", "create", - "--repo", REPO, - "--head", "importmap-updates/patch", - "--base", "main", - "--title", "t", - "--body", "b", - "--label", "dependencies", - "--label", "javascript" - ], - stdout: "https://github.com/example-org/example-repo/pull/200\n" - ) - number = @client.create_pr( - branch: "importmap-updates/patch", - base: "main", - title: "t", - body: "b", - labels: %w[dependencies javascript] - ) - assert_equal 200, number - end - - def test_create_pr_propagates_branch_taken_failure_with_clear_message - @runner.add( - pattern: [ - "gh", "pr", "create", - "--repo", REPO, - "--head", "importmap-updates/patch", - "--base", "main", - "--title", "t", - "--body", "b" - ], - stderr: fixture("gh_pr_create_branch_taken.txt"), - exit_code: 1 - ) - err = assert_raises(Commands::CommandError) do - @client.create_pr(branch: "importmap-updates/patch", base: "main", title: "t", body: "b") - end - assert_includes err.message, "already exists" - end - - # ---- update_pr ---- - - def test_update_pr_edits_title_and_body - @runner.add( - pattern: [ - "gh", "pr", "edit", "42", - "--repo", REPO, - "--title", "new title", - "--body", "new body" - ], - stdout: "" - ) - assert_nil @client.update_pr(number: 42, title: "new title", body: "new body") - end - - # ---- close_pr ---- - - def test_close_pr_leaves_comment_then_closes_when_comment_is_given - @runner.add( - pattern: ["gh", "pr", "comment", "99", "--repo", REPO, "--body", "no longer needed"], - stdout: "" - ) - @runner.add( - pattern: ["gh", "pr", "close", "99", "--repo", REPO], - stdout: "" - ) - @client.close_pr(number: 99, comment: "no longer needed") - assert_equal 2, @runner.calls.size - # Comment must come before close so reviewers see the explanation - # alongside the closed status. - assert_equal "comment", @runner.calls[0][2] - assert_equal "close", @runner.calls[1][2] - end - - def test_close_pr_skips_comment_step_when_no_comment_provided - @runner.add(pattern: ["gh", "pr", "close", "99", "--repo", REPO], stdout: "") - @client.close_pr(number: 99) - assert_equal 1, @runner.calls.size - assert_equal "close", @runner.calls[0][2] - end - - def test_close_pr_skips_comment_step_when_comment_is_empty - # Defensive: a caller passing comment: "" shouldn't post an empty comment. - @runner.add(pattern: ["gh", "pr", "close", "99", "--repo", REPO], stdout: "") - @client.close_pr(number: 99, comment: "") - assert_equal 1, @runner.calls.size - end -end diff --git a/test/github_client_test.rb b/test/github_client_test.rb new file mode 100644 index 0000000..4135f1d --- /dev/null +++ b/test/github_client_test.rb @@ -0,0 +1,150 @@ +# frozen_string_literal: true + +require_relative "test_helper" +require "minitest/mock" +require "github_client" + +class GitHubClientTest < Minitest::Test + GitHubClient = Importmap::Update::GitHubClient + Reconciler = Importmap::Update::Reconciler + + REPO = "example-org/example-repo" + + def setup + @octokit = Minitest::Mock.new + @client = GitHubClient.new(repo: REPO, token: "unused-in-tests", client: @octokit) + end + + def teardown + assert_mock @octokit + end + + # ---- list_open_prs ---- + + def test_list_open_prs_returns_prs_matching_prefix + @octokit.expect(:pull_requests, [ + pr_stub(number: 100, ref: "importmap-updates/security-lodash", title: "Security", body: ""), + pr_stub(number: 101, ref: "importmap-updates/patch", title: "Patch", body: ""), + pr_stub(number: 102, ref: "other-branch", title: "Unrelated", body: "") + ], [REPO], state: "open", per_page: 100) + + prs = @client.list_open_prs(branch_prefix: "importmap-updates") + + assert_equal 2, prs.size + assert_equal 100, prs[0].number + assert_equal "importmap-updates/security-lodash", prs[0].branch + assert_includes prs[0].body, "importmap-update:metadata" + assert_equal 101, prs[1].number + end + + def test_list_open_prs_filters_out_branches_without_exact_prefix_slash + # A branch named "importmap-updates-related-feature" must not leak through. + @octokit.expect(:pull_requests, [ + pr_stub(number: 1, ref: "importmap-updates/patch", title: "ours", body: ""), + pr_stub(number: 2, ref: "importmap-updates-related-feature", title: "leak", body: "") + ], [REPO], state: "open", per_page: 100) + + prs = @client.list_open_prs(branch_prefix: "importmap-updates") + assert_equal [1], prs.map(&:number) + end + + def test_list_open_prs_returns_empty_when_no_open_prs + @octokit.expect(:pull_requests, [], [REPO], state: "open", per_page: 100) + assert_equal [], @client.list_open_prs(branch_prefix: "importmap-updates") + end + + # ---- create_pr ---- + + def test_create_pr_returns_pr_number + @octokit.expect(:create_pull_request, + pr_stub(number: 123), + [REPO, "main", "importmap-updates/patch", "chore(deps): patch updates", "PR body text"]) + number = @client.create_pr(branch: "importmap-updates/patch", base: "main", title: "chore(deps): patch updates", body: "PR body text") + assert_equal 123, number + end + + def test_create_pr_adds_labels_when_given + @octokit.expect(:create_pull_request, pr_stub(number: 200), + [REPO, "main", "importmap-updates/patch", "t", "b"]) + @octokit.expect(:add_labels_to_an_issue, nil, + [REPO, 200, %w[dependencies javascript]]) + number = @client.create_pr(branch: "importmap-updates/patch", base: "main", title: "t", body: "b", labels: %w[dependencies javascript]) + assert_equal 200, number + end + + def test_create_pr_skips_label_call_when_no_labels + @octokit.expect(:create_pull_request, pr_stub(number: 42), + [REPO, "main", "importmap-updates/patch", "t", "b"]) + # No add_labels_to_an_issue call expected. + @client.create_pr(branch: "importmap-updates/patch", base: "main", title: "t", body: "b") + end + + # ---- ensure_labels ---- + + def test_ensure_labels_is_a_no_op_when_labels_is_empty + # No octokit calls expected. + @client.ensure_labels([]) + end + + def test_ensure_labels_creates_missing_labels + @octokit.expect(:labels, [label_stub("dependencies")], [REPO]) + @octokit.expect(:create_label, nil, [REPO, "javascript", "0075ca"]) + @client.ensure_labels(%w[dependencies javascript]) + end + + def test_ensure_labels_skips_labels_that_already_exist + @octokit.expect(:labels, [label_stub("dependencies"), label_stub("javascript")], [REPO]) + # No create_label calls expected. + @client.ensure_labels(%w[dependencies javascript]) + end + + def test_ensure_labels_tolerates_labels_list_failure + @octokit.expect(:labels, nil) { |_repo| raise Octokit::Error } + @octokit.expect(:create_label, nil, [REPO, "dependencies", "0075ca"]) + @client.ensure_labels(%w[dependencies]) + end + + # ---- update_pr ---- + + def test_update_pr_edits_title_and_body + @octokit.expect(:update_pull_request, nil, [REPO, 42], title: "new title", body: "new body") + assert_nil @client.update_pr(number: 42, title: "new title", body: "new body") + end + + # ---- close_pr ---- + + def test_close_pr_leaves_comment_then_closes_when_comment_is_given + order = [] + @octokit.expect(:add_comment, nil) do |repo, number, comment| + order << :comment + repo == REPO && number == 99 && comment == "no longer needed" + end + @octokit.expect(:close_pull_request, nil) do |repo, number| + order << :close + repo == REPO && number == 99 + end + @client.close_pr(number: 99, comment: "no longer needed") + assert_equal [:comment, :close], order + end + + def test_close_pr_skips_comment_step_when_no_comment_provided + @octokit.expect(:close_pull_request, nil, [REPO, 99]) + @client.close_pr(number: 99) + end + + def test_close_pr_skips_comment_step_when_comment_is_empty + @octokit.expect(:close_pull_request, nil, [REPO, 99]) + @client.close_pr(number: 99, comment: "") + end + + private + + def pr_stub(number:, ref: "importmap-updates/patch", title: "PR title", body: "") + head = Struct.new(:ref).new(ref) + Struct.new(:number, :head, :title, :body).new(number, head, title, body) + end + + def label_stub(name) + Struct.new(:name).new(name) + end +end