Skip to content

Conversation

@O-sura
Copy link
Contributor

@O-sura O-sura commented Jan 23, 2026

Purpose

This PR adds the integration and unit tests for the analytics-header-filter policy

Summary by CodeRabbit

  • Tests
    • Added comprehensive unit tests and BDD scenarios for analytics header filtering covering request/response header handling, configuration parsing/validation and error responses, case-insensitive matching, empty-header semantics (allow/deny), readiness checks, and end-to-end validation; integrated the new scenarios into the BDD test suite.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 23, 2026

Walkthrough

Adds a Gherkin integration feature exercising the Analytics Header Filter policy, a comprehensive Go unit test suite for header parsing and OnRequest/OnResponse behavior, and registers the new feature in the BDD test suite.

Changes

Cohort / File(s) Summary
Integration Tests
gateway/it/features/analytics-header-filter.feature
New Gherkin feature (327 lines) with Background and multiple scenarios covering deployments with request/response header filters, header manipulation steps, readiness checks, negative config cases returning 400, case-insensitive matching, and empty-array semantics.
Unit Tests
gateway/policies/analytics-header-filter/v0.1.0/analyticsheaderfilter_test.go
New comprehensive unit tests (571 lines) covering GetPolicy, default modes, header-list parsing, operation parsing (allow/deny variants, casing/whitespace), full config parsing, and OnRequest/OnResponse flows that validate produced DropHeaderAction contents.
Test Suite Registration
gateway/it/suite_test.go
Adds the new feature file to the BDD TestFeatures list (single-line change).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐇 I nibble headers, soft and light,
Allow or deny in morning's sight,
Gherkin gardens, Go tests hum,
Carrots lined as checks become,
Analytics tidy — hop, I'm done.

🚥 Pre-merge checks | ✅ 1 | ❌ 2
❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Description check ⚠️ Warning The description is incomplete. While it states the purpose and includes a related issue link, it is missing most required sections: Goals, Approach, User stories, Documentation, Automation tests, Security checks, Samples, Related PRs, and Test environment. Complete the description by filling out the remaining template sections, including Goals, Approach, Automation tests (unit and integration coverage details), and Test environment information.
Docstring Coverage ⚠️ Warning Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: adding tests for the analytics-header-filter policy. It is concise, specific, and clearly identifies the primary purpose of the PR.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@O-sura O-sura force-pushed the analytics-header-filter branch from 7fa371c to 0727436 Compare January 23, 2026 04:28
@O-sura O-sura force-pushed the analytics-header-filter branch 2 times, most recently from 6588961 to 53477bd Compare January 23, 2026 06:31
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@gateway/it/features/analytics-header-filter.feature`:
- Around line 101-109: The readiness check uses the wrong endpoint: replace the
Gherkin step that waits for
"http://localhost:8080/analytics-response/v1.0/headers" with the corresponding
analytics-request readiness URL so it matches the scenario that posts to
"http://localhost:8080/analytics-request/v1.0/data"; update the step text (the
"I wait for the endpoint ..." line) to point to the analytics-request route and
ensure it uses the supported readiness method for that route so the wait aligns
with the POST target.

Comment on lines 101 to 109
Then the response should be successful
And the response should be valid JSON
And the JSON response field "status" should be "success"
And I wait for the endpoint "http://localhost:8080/analytics-response/v1.0/headers" to be ready

When I set header "Content-Type" to "application/json"
And I set header "User-Agent" to "test-client"
And I set header "Authorization" to "Bearer secret-token"
And I send a POST request to "http://localhost:8080/analytics-request/v1.0/data" with body:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Fix the readiness endpoint mismatch.

Line 104 waits on the analytics-response API, but this scenario deploys the analytics-request API. This will likely fail the readiness check or give a false signal. Update the URL to the correct API endpoint (and ensure the wait step aligns with the supported method for that route).

🛠️ Proposed fix
-    And I wait for the endpoint "http://localhost:8080/analytics-response/v1.0/headers" to be ready
+    And I wait for the endpoint "http://localhost:8080/analytics-request/v1.0/data" to be ready
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
Then the response should be successful
And the response should be valid JSON
And the JSON response field "status" should be "success"
And I wait for the endpoint "http://localhost:8080/analytics-response/v1.0/headers" to be ready
When I set header "Content-Type" to "application/json"
And I set header "User-Agent" to "test-client"
And I set header "Authorization" to "Bearer secret-token"
And I send a POST request to "http://localhost:8080/analytics-request/v1.0/data" with body:
Then the response should be successful
And the response should be valid JSON
And the JSON response field "status" should be "success"
And I wait for the endpoint "http://localhost:8080/analytics-request/v1.0/data" to be ready
When I set header "Content-Type" to "application/json"
And I set header "User-Agent" to "test-client"
And I set header "Authorization" to "Bearer secret-token"
And I send a POST request to "http://localhost:8080/analytics-request/v1.0/data" with body:
🤖 Prompt for AI Agents
In `@gateway/it/features/analytics-header-filter.feature` around lines 101 - 109,
The readiness check uses the wrong endpoint: replace the Gherkin step that waits
for "http://localhost:8080/analytics-response/v1.0/headers" with the
corresponding analytics-request readiness URL so it matches the scenario that
posts to "http://localhost:8080/analytics-request/v1.0/data"; update the step
text (the "I wait for the endpoint ..." line) to point to the analytics-request
route and ensure it uses the supported readiness method for that route so the
wait aligns with the POST target.

@O-sura O-sura force-pushed the analytics-header-filter branch from 53477bd to a7826f0 Compare January 23, 2026 06:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant