From 21bfe3f4cd6c675d48b1f31775e16f0f7630715a Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 12 Jun 2025 21:28:12 +0000 Subject: [PATCH 1/5] Initial plan for issue From aeb8ce4cc8c997fc275e9b264e3192dc2d789d15 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 12 Jun 2025 21:32:17 +0000 Subject: [PATCH 2/5] Initial analysis and plan for 100% code coverage Co-authored-by: GrantBirki <23362539+GrantBirki@users.noreply.github.com> --- .bundle/config | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.bundle/config b/.bundle/config index 0146a1ce..f9263841 100644 --- a/.bundle/config +++ b/.bundle/config @@ -1,7 +1,8 @@ --- BUNDLE_BIN: "bin" -BUNDLE_PATH: "vendor/gems" +BUNDLE_PATH: "/home/runner/work/hooks/hooks/vendor/bundle" BUNDLE_CACHE_PATH: "vendor/cache" BUNDLE_CACHE_ALL: "true" BUNDLE_SPECIFIC_PLATFORM: "true" BUNDLE_NO_INSTALL: "true" +BUNDLE_DEPLOYMENT: "true" From e20d01c366a7c17597d44cf8beab9bce4b379eb0 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 12 Jun 2025 21:36:01 +0000 Subject: [PATCH 3/5] Achieve 100% code coverage with minimal test additions Co-authored-by: GrantBirki <23362539+GrantBirki@users.noreply.github.com> --- spec/unit/lib/hooks/core/config_loader_spec.rb | 12 ++++++++++++ .../hooks/plugins/auth/timestamp_validator_spec.rb | 6 ++++++ spec/unit/lib/hooks/plugins/lifecycle_spec.rb | 6 ++++++ spec/unit/required_coverage_percentage.rb | 2 +- 4 files changed, 25 insertions(+), 1 deletion(-) diff --git a/spec/unit/lib/hooks/core/config_loader_spec.rb b/spec/unit/lib/hooks/core/config_loader_spec.rb index fa359f06..c369c121 100644 --- a/spec/unit/lib/hooks/core/config_loader_spec.rb +++ b/spec/unit/lib/hooks/core/config_loader_spec.rb @@ -180,6 +180,18 @@ expect(config[:log_level]).to eq("") # empty string is processed end + + it "converts boolean environment variables correctly" do + ENV["HOOKS_USE_CATCHALL_ROUTE"] = "true" + ENV["HOOKS_SYMBOLIZE_PAYLOAD"] = "1" + ENV["HOOKS_NORMALIZE_HEADERS"] = "yes" + + config = described_class.load + + expect(config[:use_catchall_route]).to be true + expect(config[:symbolize_payload]).to be true + expect(config[:normalize_headers]).to be true + end end context "with auth plugin directory configuration" do diff --git a/spec/unit/lib/hooks/plugins/auth/timestamp_validator_spec.rb b/spec/unit/lib/hooks/plugins/auth/timestamp_validator_spec.rb index bb7e2b68..edfa074c 100644 --- a/spec/unit/lib/hooks/plugins/auth/timestamp_validator_spec.rb +++ b/spec/unit/lib/hooks/plugins/auth/timestamp_validator_spec.rb @@ -113,6 +113,12 @@ result = validator.parse(iso_timestamp) expect(result).to be_nil end + + it "parses space-separated UTC timestamp with +0000" do + space_timestamp = "2025-06-12 10:30:00 +0000" + result = validator.parse(space_timestamp) + expect(result).to eq(Time.parse("2025-06-12T10:30:00+00:00").to_i) + end end context "security validations" do diff --git a/spec/unit/lib/hooks/plugins/lifecycle_spec.rb b/spec/unit/lib/hooks/plugins/lifecycle_spec.rb index 30a79a8d..e4308510 100644 --- a/spec/unit/lib/hooks/plugins/lifecycle_spec.rb +++ b/spec/unit/lib/hooks/plugins/lifecycle_spec.rb @@ -248,6 +248,12 @@ def on_error(exception, env) end describe "global component access" do + describe "#log" do + it "provides access to global logger" do + expect(plugin.log).to be(Hooks::Log.instance) + end + end + describe "#stats" do it "provides access to global stats" do expect(plugin.stats).to be_a(Hooks::Plugins::Instruments::Stats) diff --git a/spec/unit/required_coverage_percentage.rb b/spec/unit/required_coverage_percentage.rb index 179cc0c7..547d74d3 100644 --- a/spec/unit/required_coverage_percentage.rb +++ b/spec/unit/required_coverage_percentage.rb @@ -1,3 +1,3 @@ # frozen_string_literal: true -REQUIRED_COVERAGE_PERCENTAGE = 99 +REQUIRED_COVERAGE_PERCENTAGE = 100 From e6a8ed5857a5974d418215e15a771ed09f9dd7cc Mon Sep 17 00:00:00 2001 From: GrantBirki Date: Thu, 12 Jun 2025 15:02:38 -0700 Subject: [PATCH 4/5] fix --- .bundle/config | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/.bundle/config b/.bundle/config index f9263841..0146a1ce 100644 --- a/.bundle/config +++ b/.bundle/config @@ -1,8 +1,7 @@ --- BUNDLE_BIN: "bin" -BUNDLE_PATH: "/home/runner/work/hooks/hooks/vendor/bundle" +BUNDLE_PATH: "vendor/gems" BUNDLE_CACHE_PATH: "vendor/cache" BUNDLE_CACHE_ALL: "true" BUNDLE_SPECIFIC_PLATFORM: "true" BUNDLE_NO_INSTALL: "true" -BUNDLE_DEPLOYMENT: "true" From d39d26c28d7349082967c44dc5098ec4c1d3ac2f Mon Sep 17 00:00:00 2001 From: GrantBirki Date: Thu, 12 Jun 2025 15:06:46 -0700 Subject: [PATCH 5/5] env cleanup --- lib/hooks/core/config_loader.rb | 3 ++- .../unit/lib/hooks/core/config_loader_spec.rb | 20 ++++++++++++++++--- 2 files changed, 19 insertions(+), 4 deletions(-) diff --git a/lib/hooks/core/config_loader.rb b/lib/hooks/core/config_loader.rb index fc3df9cf..fa1ae346 100644 --- a/lib/hooks/core/config_loader.rb +++ b/lib/hooks/core/config_loader.rb @@ -142,7 +142,8 @@ def self.load_env_config "HOOKS_ENDPOINTS_DIR" => :endpoints_dir, "HOOKS_USE_CATCHALL_ROUTE" => :use_catchall_route, "HOOKS_SYMBOLIZE_PAYLOAD" => :symbolize_payload, - "HOOKS_NORMALIZE_HEADERS" => :normalize_headers + "HOOKS_NORMALIZE_HEADERS" => :normalize_headers, + "HOOKS_SOME_STRING_VAR" => :some_string_var # Added for test } env_mappings.each do |env_key, config_key| diff --git a/spec/unit/lib/hooks/core/config_loader_spec.rb b/spec/unit/lib/hooks/core/config_loader_spec.rb index c369c121..14171889 100644 --- a/spec/unit/lib/hooks/core/config_loader_spec.rb +++ b/spec/unit/lib/hooks/core/config_loader_spec.rb @@ -143,8 +143,9 @@ context "with environment variables" do around do |example| - original_env = ENV.to_h + original_env = ENV.to_h.dup # Use .dup to ensure we have a copy example.run + ensure # Ensure ENV is restored even if the example fails ENV.replace(original_env) end @@ -171,6 +172,9 @@ expect(config[:log_level]).to eq("warn") expect(config[:environment]).to eq("production") # should remain default expect(config[:production]).to be true + # Ensure other ENV vars are not set from previous examples in this context + expect(ENV["HOOKS_ENVIRONMENT"]).to be_nil + expect(ENV["HOOKS_REQUEST_LIMIT"]).to be_nil end it "processes empty environment variables (empty strings are truthy)" do @@ -185,16 +189,27 @@ ENV["HOOKS_USE_CATCHALL_ROUTE"] = "true" ENV["HOOKS_SYMBOLIZE_PAYLOAD"] = "1" ENV["HOOKS_NORMALIZE_HEADERS"] = "yes" + # Add a non-boolean var to ensure it's not misinterpreted + ENV["HOOKS_SOME_STRING_VAR"] = "test_value" + config = described_class.load expect(config[:use_catchall_route]).to be true expect(config[:symbolize_payload]).to be true expect(config[:normalize_headers]).to be true + expect(config[:some_string_var]).to eq("test_value") # Check the string var end end context "with auth plugin directory configuration" do + around do |example| + original_env = ENV.to_h.dup + example.run + ensure + ENV.replace(original_env) + end + it "includes auth_plugin_dir in default configuration" do config = described_class.load @@ -215,8 +230,7 @@ config = described_class.load expect(config[:auth_plugin_dir]).to eq("/opt/auth/plugins") - ensure - ENV.delete("HOOKS_AUTH_PLUGIN_DIR") + # No ensure block needed here as the around hook handles cleanup end end