Skip to content

test(it): add template rendering IT coverage for LlmProvider, LlmProxy, Mcp#1829

Open
renuka-fernando wants to merge 1 commit intowso2:mainfrom
renuka-fernando:main
Open

test(it): add template rendering IT coverage for LlmProvider, LlmProxy, Mcp#1829
renuka-fernando wants to merge 1 commit intowso2:mainfrom
renuka-fernando:main

Conversation

@renuka-fernando
Copy link
Copy Markdown
Contributor

Purpose

Extend template-rendering integration tests beyond RestApi to cover LlmProvider, LlmProxy, and Mcp. Each scenario validates the render contract: API responses and DB storage preserve unrendered template expressions ({{ secret }}, {{ env }}, {{ default }}), while runtime invocation receives resolved values.

  • Add generic DB assertion step the stored <Kind> configuration for "X" should contain: backed by kind→table mapping in steps_template.go
  • Refactor GetStoredRestAPISourceConfigurationWithRetry to delegate to new GetStoredSourceConfigurationWithRetry(ctx, kind, table, handle)
  • LlmProvider scenario: {{ secret }} in upstream auth header; verify echoed value
  • LlmProxy scenario: {{ secret }} in set-headers policy; verify echoed header
  • Mcp scenario: {{ env | default }} in upstream URL; verify tools/call invocation

…y, Mcp

Extend template-rendering integration tests beyond RestApi to cover LlmProvider,
LlmProxy, and Mcp. Each scenario validates the render contract: API responses and
DB storage preserve unrendered template expressions ({{ secret }}, {{ env }},
{{ default }}), while runtime invocation receives resolved values.

- Add generic DB assertion step `the stored <Kind> configuration for "X" should contain:`
  backed by kind→table mapping in steps_template.go
- Refactor GetStoredRestAPISourceConfigurationWithRetry to delegate to new
  GetStoredSourceConfigurationWithRetry(ctx, kind, table, handle)
- LlmProvider scenario: {{ secret }} in upstream auth header; verify echoed value
- LlmProxy scenario: {{ secret }} in set-headers policy; verify echoed header
- Mcp scenario: {{ env | default }} in upstream URL; verify tools/call invocation
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 30, 2026

📝 Walkthrough

Template Rendering Integration Test Coverage Extension

This PR extends template-rendering integration test coverage to include LlmProvider, LlmProxy, and Mcp artifact types, complementing existing RestApi test coverage.

Key Changes

Test Scenarios (template-functions.feature)

  • Added three new end-to-end scenarios validating template resolution behavior across artifact types:
    • LlmProvider: verifies {{ secret }} expressions in upstream auth headers
    • LlmProxy: verifies {{ secret }} expressions in set-headers policy values
    • Mcp: verifies {{ env | default }} expressions in upstream URLs
  • Each scenario validates the same contract: API responses and stored configurations preserve unrendered template expressions, while runtime invocations receive resolved values

Generic DB Assertion Step (steps_template.go)

  • Generalized the Gherkin step "the stored configuration for 'X' should contain" to support multiple artifact kinds via kind→table mapping
  • Dispatches to storedConfigurationShouldContain(kind, handle, literal) with support for RestApi, LlmProvider, LlmProxy, and Mcp
  • Improved error messaging to report the specific artifact kind being validated

Refactored DB Helper (db_helpers.go)

  • Introduced GetStoredSourceConfigurationWithRetry(ctx, kind, table, handle) as a generic helper for querying stored configurations
  • Refactored existing GetStoredRestAPISourceConfigurationWithRetry to delegate to the new generic helper with RestApi-specific parameters
  • Maintains original retry behavior and error handling

Walkthrough

This pull request generalizes template configuration handling across multiple artifact kinds. The database helper function GetStoredSourceConfigurationWithRetry is introduced as a reusable retry mechanism, replacing RestAPI-specific logic in the existing GetStoredRestAPISourceConfigurationWithRetry function. Test steps are updated to validate stored configuration for multiple artifact types (RestApi, LlmProvider, LlmProxy, Mcp) using a unified storedConfigurationShouldContain method that dynamically maps artifact kinds to their storage tables. New test scenarios verify that template expressions in secret and env functions are persisted unrendered but resolved at runtime across different artifact contexts.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Description check ❓ Inconclusive The description adequately covers purpose, goals, and approach. However, several required template sections (User stories, Documentation, Automation tests, Security checks, Samples, Related PRs, Test environment) are missing or incomplete. Complete missing sections from the template: provide user stories, documentation links, test coverage details, security verification checklist results, and test environment specifications.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: adding template rendering integration test coverage for three new artifact kinds (LlmProvider, LlmProxy, Mcp).
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 golangci-lint (2.11.4)

level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain modules listed in go.work or their selected dependencies"


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
Review rate limit: 0/1 reviews remaining, refill in 60 minutes.

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

Copy link
Copy Markdown
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.

🧹 Nitpick comments (1)
gateway/it/steps_template.go (1)

48-59: ⚡ Quick win

Keep supported artifact kinds in one source of truth.

The supported-kind list is currently repeated in the step regex, kindTables, and the error string. Consider deriving the regex and supported-kinds message from kindTables to avoid drift when adding/removing kinds.

Also applies to: 101-104

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@gateway/it/steps_template.go` around lines 48 - 59, The step definition
hardcodes the supported kinds in the regex and error messages; instead derive
them from the kindTables map so there is a single source of truth. Change the
ctx.Step registration that invokes storedConfigurationShouldContain to build its
allowed-kind pattern from the keys of kindTables (e.g., join keys with |) or
make the regex accept any kind and validate inside
storedConfigurationShouldContain against kindTables; also replace any error
message that lists supported kinds to generate that message from kindTables keys
so additions/removals only need to change kindTables.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@gateway/it/steps_template.go`:
- Around line 48-59: The step definition hardcodes the supported kinds in the
regex and error messages; instead derive them from the kindTables map so there
is a single source of truth. Change the ctx.Step registration that invokes
storedConfigurationShouldContain to build its allowed-kind pattern from the keys
of kindTables (e.g., join keys with |) or make the regex accept any kind and
validate inside storedConfigurationShouldContain against kindTables; also
replace any error message that lists supported kinds to generate that message
from kindTables keys so additions/removals only need to change kindTables.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 2d2a91c6-1e6e-4471-851f-5e12c0fcdb0d

📥 Commits

Reviewing files that changed from the base of the PR and between e4fc7ab and 9f813f6.

📒 Files selected for processing (3)
  • gateway/it/db_helpers.go
  • gateway/it/features/template-functions.feature
  • gateway/it/steps_template.go

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