Add tests for cache store, healthcheck controller error handling

Co-authored-by: jaracursorsh <jaracursorsh@mitomail.com>

Author: 2 people

test/allgood/cache_store_test.rb

@@ -71,6 +71,35 @@ def delete_matched(pattern)
     end
   end
 
+  def test_cleanup_old_keys_logs_warning_on_error
+    fake_cache = Class.new do
+      def write(*) = true
+      def read(*) = "true"
+      def delete_matched(*)
+        raise "delete failed"
+      end
+    end.new
+
+    messages = []
+    fake_logger = Logger.new($stdout)
+    def fake_logger.warn(msg)
+      (@_msgs ||= []) << msg
+    end
+    def fake_logger.messages
+      @_msgs || []
+    end
+
+    @store.stub(:rails_cache_available?, true) do
+      Rails.stub(:cache, fake_cache) do
+        Rails.stub(:logger, fake_logger) do
+          # Should not raise
+          @store.cleanup_old_keys
+          assert fake_logger.messages.any? { |m| m.include?("Failed to cleanup old cache keys") }
+        end
+      end
+    end
+  end
+
   def test_rails_cache_available_checks_read_write
     store = ActiveSupport::Cache::MemoryStore.new
     Rails.stub(:cache, store) do

test/allgood/healthcheck_controller_test.rb

@@ -33,6 +33,24 @@ def test_html_ok_when_all_checks_pass
     assert check_lines.any? { |t| t.include?("✅") }
   end
 
+  def test_html_escapes_check_name_and_message
+    @config.check("<script>alert('x')</script>") { make_sure false, "<b>bad</b>" }
+    get "/"
+    doc = Nokogiri::HTML.parse(last_response.body)
+    # Ensure escaped content appears literally
+    assert_includes doc.text, "<script>alert('x')</script>"
+    assert_includes doc.text, "<b>bad</b>"
+    # Raw script tag should not be rendered
+    refute doc.at_css('script')
+    # The name is wrapped in a <b> by the view; ensure its content is the escaped name
+    name_b = doc.at_css('.check b')
+    assert_equal "<script>alert('x')</script>", name_b.text
+    # The message is inside <i> and should not contain raw <b> elements
+    msg_i = doc.at_css('.check i')
+    assert_includes msg_i.text, "<b>bad</b>"
+    refute msg_i.at_css('b')
+  end
+
   def test_json_error_when_any_check_fails
     @config.check("Fail") { expect(1).to_eq(2) }
     header "Accept", "application/json"
@@ -52,6 +70,21 @@ def test_json_error_when_any_check_fails
     assert_kind_of Numeric, first["duration"]
   end
 
+  def test_json_schema_for_skipped_check
+    Rails.stub(:env, ActiveSupport::StringInquirer.new("test")) do
+      @config.check("Only prod", only: :production) { make_sure true }
+    end
+    header "Accept", "application/json"
+    get "/"
+    assert_equal 200, last_response.status
+    json = JSON.parse(last_response.body)
+    skipped = json["checks"].find { |c| c["name"] == "Only prod" }
+    assert skipped["success"]
+    assert_equal true, skipped["skipped"]
+    assert_kind_of String, skipped["message"]
+    assert_equal 0, skipped["duration"]
+  end
+
   def test_timeout_is_handled
     @config.check("Timeout", timeout: 0.01) { sleep 0.1; make_sure true }
 
@@ -103,6 +136,37 @@ def test_rate_limit_resets_next_day
     end
   end
 
+  def test_rate_limit_error_persists_across_periods_and_blocks_execution
+    travel_to Time.utc(2024, 12, 31, 23, 55, 0) do
+      # Define a check that fails first time, then passes after midnight
+      attempts = 0
+      @config.check("Flaky API", run: "10 times per hour") do
+        attempts += 1
+        if attempts == 1
+          make_sure false, "boom"
+        else
+          make_sure true, "ok"
+        end
+      end
+
+      # First run fails and stores error state
+      get "/"
+      assert_equal 503, last_response.status
+
+      # Subsequent run within same hour should be skipped due to previous error
+      get "/"
+      assert last_response.ok? || last_response.status == 503
+      assert_includes text_content(last_response.body), "Rate limited"
+
+      # Even after period change, previous error should keep the check skipped until success
+      travel 10.minutes
+      travel_to Time.utc(2025, 1, 1, 0, 5, 0)
+      get "/"
+      # Still rate limited due to previous error persistence
+      assert_includes text_content(last_response.body), "Rate limited"
+    end
+  end
+
   def test_error_rescue_returns_500
     # Force an unexpected error in run_checks by stubbing configuration
     Allgood.stub(:configuration, nil) do