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

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 30, 2026

Review Change Stack
No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 62f18dfe-1ac6-41f0-b0f6-095b3208eb57

📥 Commits

Reviewing files that changed from the base of the PR and between 71a8b2e and 70e4331.

📒 Files selected for processing (3)
  • gateway/it/db_helpers.go
  • gateway/it/features/template-functions.feature
  • gateway/it/steps_template.go
🚧 Files skipped from review as they are similar to previous changes (3)
  • gateway/it/db_helpers.go
  • gateway/it/features/template-functions.feature
  • gateway/it/steps_template.go

📝 Walkthrough

Template Rendering Integration Test Coverage Extension

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

Key Changes

  • Test scenarios (gateway/it/features/template-functions.feature)

    • Added end-to-end scenarios for:
      • LlmProvider: places {{ secret }} in upstream auth.value; verifies it is persisted unrendered and resolved at runtime.
      • LlmProxy: places {{ secret }} in a set-headers policy header value; verifies it is persisted unrendered and resolved at runtime.
      • Mcp: places {{ env | default }} in the upstream URL; verifies it is persisted unrendered and resolves during MCP initialization and tool/call invocations.
    • Each scenario asserts the contract that API responses and DB storage preserve unrendered template expressions while runtime invocations receive resolved values. Scenarios include runtime calls and cleanup steps.
  • Generic DB assertion step (gateway/it/steps_template.go)

    • Replaced the RestApi-specific stored-configuration step with a generalized Gherkin step:
      "the stored (RestApi|LlmProvider|LlmProxy|Mcp) configuration for 'X' should contain:"
    • Introduced a kind→table mapping for RestApi, LlmProvider, LlmProxy, and Mcp and a unified handler storedConfigurationShouldContain that queries the DB and asserts the expected literal (supports JSON-escaped forms).
    • Removed the prior RestApi-only handler and added clearer error messages for unknown kinds and failure contexts.
  • Refactored DB helper (gateway/it/db_helpers.go)

    • Added GetStoredSourceConfigurationWithRetry(ctx, kind, table, handle) to generalize retrying reads of stored source configurations for any artifact kind/table.
    • GetStoredRestAPISourceConfigurationWithRetry now delegates to the new generic helper with RestApi-specific parameters.
    • The retry semantics are preserved; on success the stored configuration is returned, context cancellation returns ctx.Err(), and exhausting attempts returns a wrapped "stored configuration not found after attempts" error.

Overall, this PR broadens template-rendering IT coverage to additional artifact kinds and centralizes DB assertion and retry logic to support those scenarios.

Walkthrough

This pull request generalizes template configuration handling across multiple artifact kinds. It adds GetStoredSourceConfigurationWithRetry as a reusable bounded-retry DB helper and updates GetStoredRestAPISourceConfigurationWithRetry to delegate to it. Test step bindings are generalized to accept artifact kinds (RestApi, LlmProvider, LlmProxy, Mcp), map them to storage tables, and verify persisted source configuration via the new helper. Three end-to-end feature scenarios were added to validate that secret/env template expressions are persisted unrendered and resolved at runtime.

Sequence Diagram(s)

sequenceDiagram
  participant Client as Test Client
  participant Gateway as Gateway
  participant DB as Persisted Configuration
  participant Upstream as Upstream Service / Tool
  Client->>Gateway: runtime request (API or MCP)
  Gateway->>DB: read stored source configuration (unrendered templates)
  DB-->>Gateway: stored config (unrendered)
  Gateway->>Upstream: resolve templates (secret/env) -> send resolved headers/URL
  Upstream-->>Gateway: response
  Gateway-->>Client: response (assert runtime result)
Loading
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Description check ❓ Inconclusive The description covers Purpose, key implementation details (generic DB assertion, refactoring), and the three new scenarios. However, it omits several template sections (Goals, Approach, User stories, Documentation, Automation tests, Security checks, Samples, Related PRs, Test environment). Add missing template sections, especially Automation tests section detailing the three new end-to-end scenarios and coverage, and Security checks confirmation. Consider including Test environment details.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main objective: adding integration test coverage for template rendering across LlmProvider, LlmProxy, and 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.12.1)

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

Tip

💬 Introducing Slack Agent: The best way for teams to turn conversations into code.

Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.

  • Generate code and open pull requests
  • Plan features and break down work
  • Investigate incidents and troubleshoot customer tickets together
  • Automate recurring tasks and respond to alerts with triggers
  • Summarize progress and report instantly

Built for teams:

  • Shared memory across your entire org—no repeating context
  • Per-thread sandboxes to safely plan and execute work
  • Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get started


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.

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

Krishanx92
Krishanx92 previously approved these changes Apr 30, 2026
…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

Signed-off-by: Renuka Fernando <renukapiyumal@gmail.com>
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