From 16f37decd98c069ee171b2e184b2a1b458b0c639 Mon Sep 17 00:00:00 2001 From: kairox Date: Mon, 15 Jun 2026 15:01:39 +0900 Subject: [PATCH 1/2] chore: add tooling and tests, drop dead CI config (Phase 3) - add ameba 1.6 as a development dependency and satisfy its lints - fix broken spec_helper (required a non-existent file) and replace the placeholder spec with model unit tests (Subject#color/#update?/#comment_url, Notification#mention?, JSON parsing, Slack serialization) - add GitHub Actions CI running format check / ameba / spec - remove obsolete .travis.yml; gitignore built bin/ refs #84 Co-Authored-By: Claude Opus 4.8 (1M context) --- .github/workflows/ci.yml | 31 +++++++ .gitignore | 5 +- .travis.yml | 6 -- shard.lock | 8 +- shard.yml | 5 ++ spec/github/models_spec.cr | 113 ++++++++++++++++++++++++ spec/github_notifications_slack_spec.cr | 9 -- spec/slack/models_spec.cr | 22 +++++ spec/spec_helper.cr | 3 +- src/main.cr | 6 +- src/runtime/lambda.cr | 4 +- 11 files changed, 185 insertions(+), 27 deletions(-) create mode 100644 .github/workflows/ci.yml delete mode 100644 .travis.yml create mode 100644 spec/github/models_spec.cr delete mode 100644 spec/github_notifications_slack_spec.cr create mode 100644 spec/slack/models_spec.cr diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 0000000..074d23c --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,31 @@ +name: ci + +on: + push: + branches: [master] + pull_request: + +jobs: + check: + name: format / lint / spec + runs-on: ubuntu-24.04-arm + steps: + - name: checkout + uses: actions/checkout@v4 + + - name: install crystal + uses: crystal-lang/install-crystal@v1 + with: + crystal: latest + + - name: install shards + run: shards install + + - name: check format + run: crystal tool format --check + + - name: lint with ameba + run: ./bin/ameba + + - name: run specs + run: crystal spec diff --git a/.gitignore b/.gitignore index ed141fe..0248cdc 100644 --- a/.gitignore +++ b/.gitignore @@ -6,7 +6,8 @@ jspm_packages .serverless bootstrap +bin lib env.yml - -.idea/ + +.idea/ diff --git a/.travis.yml b/.travis.yml deleted file mode 100644 index 765f0e9..0000000 --- a/.travis.yml +++ /dev/null @@ -1,6 +0,0 @@ -language: crystal - -# Uncomment the following if you'd like Travis to run specs and check code formatting -# script: -# - crystal spec -# - crystal tool format --check diff --git a/shard.lock b/shard.lock index 4feacb6..f612c4d 100644 --- a/shard.lock +++ b/shard.lock @@ -1,6 +1,6 @@ -version: 1.0 +version: 2.0 shards: - clim: - github: at-grandpa/clim - version: 0.4.1 + ameba: + git: https://github.com/crystal-ameba/ameba.git + version: 1.6.4 diff --git a/shard.yml b/shard.yml index 015c1cd..dec4eb0 100644 --- a/shard.yml +++ b/shard.yml @@ -10,4 +10,9 @@ targets: crystal: ">= 1.20.0" +development_dependencies: + ameba: + github: crystal-ameba/ameba + version: ~> 1.6 + license: MIT diff --git a/spec/github/models_spec.cr b/spec/github/models_spec.cr new file mode 100644 index 0000000..d6759cb --- /dev/null +++ b/spec/github/models_spec.cr @@ -0,0 +1,113 @@ +require "../spec_helper" + +private def subject_from(type : String, url = "", latest_comment_url = "") + Github::Subject.from_json({ + type: type, + title: "title", + url: url, + latest_comment_url: latest_comment_url, + }.to_json) +end + +describe Github::Subject do + describe "#update?" do + it "is true for tracked subject types" do + [ + Github::Subject::Type::PULL_REQUEST, + Github::Subject::Type::ISSUE, + Github::Subject::Type::COMMIT, + Github::Subject::Type::DISCUSSION, + ].each do |type| + subject_from(type).update?.should be_true + end + end + + it "is false for unknown subject types" do + subject_from("Release").update?.should be_false + end + end + + describe "#color" do + it "returns a distinct color per known type" do + subject_from(Github::Subject::Type::PULL_REQUEST).color.should eq "#F6CEE3" + subject_from(Github::Subject::Type::ISSUE).color.should eq "#A9D0F5" + subject_from(Github::Subject::Type::COMMIT).color.should eq "#f5d7a9" + subject_from(Github::Subject::Type::DISCUSSION).color.should eq "#7fffd4" + end + + it "falls back to a default color for unknown types" do + subject_from("Release").color.should eq "#D8D8D8" + end + end + + describe "#comment_url" do + it "prefers latest_comment_url when present" do + subject = subject_from("Issue", url: "u", latest_comment_url: "c") + subject.comment_url.should eq "c" + end + + it "falls back to url when latest_comment_url is blank" do + subject = subject_from("Issue", url: "u", latest_comment_url: "") + subject.comment_url.should eq "u" + end + end +end + +describe Github::Notification do + describe "#mention?" do + it "is true for reasons that mention the user" do + Github::Notification::MENTION_REASONS.each do |reason| + notification_from(reason).mention?.should be_true + end + end + + it "is false for non-mention reasons" do + notification_from("subscribed").mention?.should be_false + notification_from("ci_activity").mention?.should be_false + end + end + + it "parses a GitHub notifications API payload" do + notifications = Array(Github::Notification).from_json(NOTIFICATIONS_FIXTURE) + notifications.size.should eq 1 + + notification = notifications.first + notification.reason.should eq "mention" + notification.subject.type.should eq "Issue" + notification.subject.title.should eq "Spurious failure" + notification.repository.full_name.should eq "octocat/Hello-World" + notification.mention?.should be_true + end +end + +private def notification_from(reason : String) + Github::Notification.from_json({ + reason: reason, + subject: {type: "Issue", title: "title"}, + repository: {owner: {login: "octocat"}}, + }.to_json) +end + +NOTIFICATIONS_FIXTURE = <<-JSON +[ + { + "reason": "mention", + "subject": { + "title": "Spurious failure", + "url": "https://api.github.com/repos/octocat/Hello-World/issues/1", + "latest_comment_url": "https://api.github.com/repos/octocat/Hello-World/issues/comments/1", + "type": "Issue" + }, + "repository": { + "full_name": "octocat/Hello-World", + "html_url": "https://github.com/octocat/Hello-World", + "owner": { + "login": "octocat", + "avatar_url": "https://github.com/images/error/octocat.gif", + "html_url": "https://github.com/octocat" + } + }, + "subscription_url": "https://api.github.com/notifications/threads/1/subscription" + } +] +JSON diff --git a/spec/github_notifications_slack_spec.cr b/spec/github_notifications_slack_spec.cr deleted file mode 100644 index 1a4c2f2..0000000 --- a/spec/github_notifications_slack_spec.cr +++ /dev/null @@ -1,9 +0,0 @@ -require "./spec_helper" - -describe GithubNotificationsSlack do - # TODO: Write tests - - it "works" do - false.should eq(true) - end -end diff --git a/spec/slack/models_spec.cr b/spec/slack/models_spec.cr new file mode 100644 index 0000000..26da433 --- /dev/null +++ b/spec/slack/models_spec.cr @@ -0,0 +1,22 @@ +require "../spec_helper" + +describe Slack::Attachment do + it "omits unset fields when serialized" do + json = Slack::Attachment.new(text: "hello", color: "#000000").to_json + parsed = JSON.parse(json) + + parsed["text"].should eq "hello" + parsed["color"].should eq "#000000" + parsed.as_h.has_key?("title").should be_false + end +end + +describe Slack::Post do + it "wraps attachments under an attachments key" do + post = Slack::Post.new([Slack::Attachment.new(text: "a")]) + parsed = JSON.parse(post.to_json) + + parsed["attachments"].as_a.size.should eq 1 + parsed["attachments"][0]["text"].should eq "a" + end +end diff --git a/spec/spec_helper.cr b/spec/spec_helper.cr index 6ca3d64..eb8812d 100644 --- a/spec/spec_helper.cr +++ b/spec/spec_helper.cr @@ -1,2 +1,3 @@ require "spec" -require "../src/github_notifications_slack" +require "../src/github/models" +require "../src/slack/models" diff --git a/src/main.cr b/src/main.cr index 38b4a8c..dda9585 100644 --- a/src/main.cr +++ b/src/main.cr @@ -29,8 +29,8 @@ error_uc = Error::Usecase.new( Serverless::Lambda.handler "github_notifications_slack" do |_| begin notify_uc.check_notifications - rescue err - error_uc.alert err - raise err + rescue error + error_uc.alert error + raise error end end diff --git a/src/runtime/lambda.cr b/src/runtime/lambda.cr index ce70d76..a10eae5 100644 --- a/src/runtime/lambda.cr +++ b/src/runtime/lambda.cr @@ -22,10 +22,10 @@ module Serverless body = yield event header = nil url = "http://#{ENV["AWS_LAMBDA_RUNTIME_API"]}/2018-06-01/runtime/invocation/#{request_id}/response" - rescue err + rescue error body = { msg: "Internal Lambda Error", - err: err.message, + err: error.message, } header = HTTP::Headers{"Lambda-Runtime-Function-Error-Type" => "Unhandled"} url = "http://#{ENV["AWS_LAMBDA_RUNTIME_API"]}/2018-06-01/runtime/invocation/#{request_id}/error" From 55782fcf15ac9a80895eef3ce40c17c1dc60be30 Mon Sep 17 00:00:00 2001 From: kairox Date: Mon, 15 Jun 2026 19:16:14 +0900 Subject: [PATCH 2/2] fix: fall back to exception class name when message is nil Address review feedback on PR #88: when error.message is nil the Lambda error response field would be null, hurting debuggability. Fall back to the exception class name. refs #84 Co-Authored-By: Claude Opus 4.8 (1M context) --- src/runtime/lambda.cr | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/runtime/lambda.cr b/src/runtime/lambda.cr index a10eae5..2830488 100644 --- a/src/runtime/lambda.cr +++ b/src/runtime/lambda.cr @@ -25,7 +25,7 @@ module Serverless rescue error body = { msg: "Internal Lambda Error", - err: error.message, + err: error.message || error.class.name, } header = HTTP::Headers{"Lambda-Runtime-Function-Error-Type" => "Unhandled"} url = "http://#{ENV["AWS_LAMBDA_RUNTIME_API"]}/2018-06-01/runtime/invocation/#{request_id}/error"