Skip to content

Comments

Fix test failure#949

Merged
Krishanx92 merged 2 commits intowso2:mainfrom
Krishanx92:report
Feb 5, 2026
Merged

Fix test failure#949
Krishanx92 merged 2 commits intowso2:mainfrom
Krishanx92:report

Conversation

@Krishanx92
Copy link
Contributor

@Krishanx92 Krishanx92 commented Feb 5, 2026

Purpose

Explain why this feature or fix is required. Describe the underlying problems, issues, or needs driving this feature/fix and include links to related issues in the following format: Resolves issue1, issue2, etc.

Goals

Describe what solutions this feature or fix introduces to address the problems outlined above.

Approach

Describe how you are implementing the solutions. Include an animated GIF or screenshot if the change affects the UI. Include a link to a Markdown file or Google doc if the feature write-up is too long to paste here.

User stories

Summary of user stories addressed by this change>

Documentation

Link(s) to product documentation that addresses the changes of this PR. If no doc impact, enter “N/A” plus brief explanation of why there’s no doc impact

Automation tests

  • Unit tests

    Code coverage information

  • Integration tests

    Details about the test cases and coverage

Security checks

Samples

Provide high-level details about the samples related to this feature

Related PRs

List any other related PRs

Test environment

List all JDK versions, operating systems, databases, and browser/versions on which this feature/fix was tested

Summary by CodeRabbit

  • Tests
    • Extended test coverage to exercise a new boolean flag that influences policy execution flows.
    • Updated test fixtures and names to align with revised API request naming (Generation → Creation) and related fields.
    • Enabled Lua-based request-transformation in relevant test scenarios to better mirror runtime behavior.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 5, 2026

Walkthrough

Extended two ChainExecutor methods (ExecuteRequestPolicies, ExecuteResponsePolicies) to accept a trailing boolean flag; updated tests and related call sites to pass the new flag. Also adjusted API key test types/names (Generation→Creation) and wired a Lua script path into several translator tests.

Changes

Cohort / File(s) Summary
Chain executor tests
gateway/policy-engine/internal/executor/chain_test.go
Added a trailing bool parameter to ExecuteRequestPolicies and ExecuteResponsePolicies calls in tests to match updated signatures; propagated flag values across test cases.
API key tests (rename/refactor)
gateway/gateway-controller/pkg/utils/api_key_test.go
Renamed test types/functions from Generation→Creation (APIKeyGenerationRequestAPIKeyCreationRequest, generateAPIKeyFromRequestcreateAPIKeyFromRequest) and updated related field/constant names and test name.
Translator tests (Lua wiring)
gateway/gateway-controller/pkg/xds/translator_test.go
Set translator.routerConfig.LuaScriptPath = "../../lua/request_transformation.lua" in three test setups to enable Lua-based request transformation in listener/policy-engine tests.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 I jumped into tests with a curious flag,
Two methods grew tails and hopped in the flag,
Names changed their hats from Generation to Create,
A Lua path twitched its whiskers and joined the gate,
Tests hum a tune — a tiny change, big delight. 🥕

🚥 Pre-merge checks | ❌ 3
❌ Failed checks (2 warnings, 1 inconclusive)
Check name Status Explanation Resolution
Description check ⚠️ Warning The pull request description is entirely unfilled—it contains only the template structure with placeholder text and no actual content addressing Purpose, Goals, Approach, or other required sections. Fill in all required sections: explain why the fixes are needed, describe the solutions (method signature changes, API renaming, Lua path config), detail the implementation approach, confirm test coverage, and verify security checks.
Docstring Coverage ⚠️ Warning Docstring coverage is 26.32% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ❓ Inconclusive The title 'Fix test failure' is vague and generic, using non-descriptive language that doesn't convey meaningful information about the actual changes (method signature updates, API renaming, Lua script path configuration). Use a more specific title that describes the primary change, such as 'Update method signatures and add Lua script path to test configurations' or 'Refactor API key types and extend executor method signatures'.

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

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

@Krishanx92 Krishanx92 merged commit 8b71bad into wso2:main Feb 5, 2026
4 checks passed
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.

2 participants