Port 5 router cookie tests from openshift-tests-private#31157
Open
dgoodwin wants to merge 3 commits into
Open
Conversation
Port high-pass-rate router cookie tests covering HAProxy sticky sessions, cookie naming, and SameSite attribute behavior for edge and reencrypt routes. Tests ported: - OCP-11903: haproxy cookies based sticky session for unsecure routes (98.0%) - OCP-12566: Cookie name should not use openshift prefix (98.9%) - OCP-15872: can set cookie name for unsecure routes by annotation (98.9%) - OCP-35547: cookie-same-site annotation for edge routes (99.2%) - OCP-35548: cookie-same-site annotation for Reencrypt routes (98.7%) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
|
Pipeline controller notification For optional jobs, comment This repository is configured in: automatic mode |
WalkthroughAdds end-to-end Ginkgo/Gomega tests for router OTP cookie behavior (sticky sessions, cookie naming, custom names, SameSite) plus helper utilities and test manifests to exercise HAProxy config, routes, and backend traffic. ChangesRouter OTP Cookie Testing
Sequence DiagramsequenceDiagram
participant Test as Test Suite
participant API as Kubernetes API
participant Router as HAProxy Pod
participant Backend as Web Server Pod
participant Client as curl
Test->>API: create IngressController, Deployments, Services, Routes
API-->>Test: resource created / admitted
Test->>Router: exec into router pod, fetch haproxy config
Router-->>Test: haproxy config text
Test->>Client: curl route (no cookie) -> responses from Backend A/B
Client-->>Test: Set-Cookie header (OTP cookie)
Test->>Client: subsequent curls with cookie -> backend sticky selection
Test->>API: patch route annotations (disable cookies / set cookie name / set SameSite)
API-->>Router: rollout config changes
Test->>Router: wait for new replica/pod ready, fetch haproxy config
Router-->>Test: updated haproxy config
Test->>Client: curl route, validate Set-Cookie header and backend distribution
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 7 | ❌ 5❌ Failed checks (5 warnings)
✅ Passed checks (7 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Comment |
openshift-ci
Bot
added
the
approved
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (6)
test/extended/router/util_otp.go (3)
69-77: 💤 Low valuePer-call random seeding is inefficient but acceptable for test code.
Creating a new
rand.Sourceseeded withtime.Now().UnixNano()on each call is inefficient and could theoretically produce duplicates if called rapidly. For test temp filenames this is acceptable, but a package-level seeded source would be cleaner.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/extended/router/util_otp.go` around lines 69 - 77, getRandomString currently creates a new rand.Source seeded with time.Now().UnixNano() on every call which is inefficient and can collide under rapid calls; change this to use a package-level seeded *rand.Rand or rand.Source (e.g. a var seeded in init or at declaration) and have getRandomString reuse that global RNG instead of calling rand.New(rand.NewSource(...)) each time to improve efficiency and uniqueness while keeping the same logic.
39-46: ⚖️ Poor tradeoffString-based node counting may miscount if node names contain status keywords.
The function counts occurrences of "Ready", "SchedulingDisabled", and "NotReady" in the raw output. If a node name happens to contain these substrings (e.g.,
node-NotReady-01), the count will be incorrect. Consider usingkubectl get nodes -o jsonpathto extract status conditions more reliably.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/extended/router/util_otp.go` around lines 39 - 46, The exactNodeDetails function currently counts status keywords in the raw kubectl output which can be miscounted if node names include those substrings; update exactNodeDetails to call oc.AsAdmin().WithoutNamespace().Run("get").Args("nodes","--selector=node-role.kubernetes.io/worker=,kubernetes.io/os=linux","-o","jsonpath={range .items[*]}{.metadata.name}:{range .status.conditions[*]}{.type}={.status};{end}\n{end}").Output() (or equivalent json/jsonpath) and parse each node's status conditions to compute nodeCount from actual Condition type/status pairs instead of string-counting "Ready"/"NotReady"/"SchedulingDisabled", keeping the returned values nodeCount and the raw or formatted node details for logging.
446-450: 💤 Low valueSed command may fail if replacement strings contain the pipe delimiter.
The
sedcommand uses|as a delimiter. IftoBeReplacedornewContentcontain|, the command will fail or produce unexpected results. While current callers use safe strings like"replicas: 1", this could be fragile for future use.♻️ Safer alternative using Go's file manipulation
func updateFilebySedCmd(file, toBeReplaced, newContent string) { - sedCmd := fmt.Sprintf(`sed -i'' -e 's|%s|%s|g' %s`, toBeReplaced, newContent, file) - _, err := exec.Command("bash", "-c", sedCmd).Output() + content, err := os.ReadFile(file) + o.Expect(err).NotTo(o.HaveOccurred()) + newData := strings.ReplaceAll(string(content), toBeReplaced, newContent) + err = os.WriteFile(file, []byte(newData), 0644) o.Expect(err).NotTo(o.HaveOccurred()) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/extended/router/util_otp.go` around lines 446 - 450, The helper updateFilebySedCmd is brittle because it shells out to sed with '|' delimiters; replace it with pure-Go file manipulation: read the file (os.ReadFile), perform a safe replacement (strings.ReplaceAll on the file content using the provided toBeReplaced and newContent values, or use regexp.QuoteMeta+ReplaceAllString for regex needs), then write the updated content back (os.WriteFile) preserving original file permissions (os.Stat to get FileMode); return or fail on any IO errors instead of invoking bash/exec.Command. Ensure the function name updateFilebySedCmd is updated to use this approach and that error assertions (o.Expect) still validate the operation.test/extended/router/cookies_otp.go (1)
130-135: ⚡ Quick winPrefer
os.ReadFileover shell command for reading local files.Using
exec.Command("bash", "-c", "cat ...")to read a file is unnecessary when Go'sos.ReadFileis available. This also addresses the static analysis hint about shell command injection.♻️ Suggested fix
- cmd := fmt.Sprintf(`cat %s`, cookie) - cookiesOutput, err := exec.Command("bash", "-c", cmd).Output() + cookiesOutput, err := os.ReadFile(cookie) o.Expect(err).NotTo(o.HaveOccurred())🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/extended/router/cookies_otp.go` around lines 130 - 135, Replace the shell cat invocation using exec.Command("bash", "-c", cmd) with a direct file read via os.ReadFile(cookie): call data, err := os.ReadFile(cookie) and check err with o.Expect(err).NotTo(o.HaveOccurred()), then use string(data) in place of string(cookiesOutput) for the subsequent assertions (o.Expect(string(data)).NotTo(o.ContainSubstring("OPENSHIFT")) and o.Expect(string(data)).NotTo(o.MatchRegexp(...))). Remove cmd and cookiesOutput variables and import "os" if not already imported.test/extended/testdata/router/web-server-signed-deploy.yaml (1)
34-64: 💤 Low valueConsider adding security context for consistency.
Same as
web-server-deploy.yaml, this deployment lacks pod and containersecurityContextsettings. For consistency withtest-client-pod.yaml, consider adding security context settings.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/extended/testdata/router/web-server-signed-deploy.yaml` around lines 34 - 64, The Deployment "web-server-deploy" is missing pod- and container-level securityContext entries; add a pod securityContext under spec.template.spec (e.g., runAsNonRoot: true, runAsUser: <non-root uid>, fsGroup: <gid>) and add a container securityContext on the "nginx" container (e.g., allowPrivilegeEscalation: false, readOnlyRootFilesystem: true, runAsUser matching pod UID) to match the settings used in test-client-pod.yaml for consistency and stricter security.test/extended/testdata/router/web-server-deploy.yaml (1)
19-29: 💤 Low valueConsider adding security context for consistency with other test fixtures.
The deployment lacks pod and container
securityContextsettings. Thetest-client-pod.yamlin this PR properly setsrunAsNonRoot,seccompProfile, andallowPrivilegeEscalation: false. Adding similar settings here would align with security best practices and ensure consistency across test fixtures.♻️ Suggested security context addition
spec: + securityContext: + runAsNonRoot: true + seccompProfile: + type: RuntimeDefault containers: - name: nginx image: quay.io/openshifttest/nginx-alpine@sha256:cee6930776b92dc1e93b73f9e5965925d49cff3d2e91e1d071c2f0ff72cbca29 + securityContext: + allowPrivilegeEscalation: false + capabilities: + drop: + - ALL ports:🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/extended/testdata/router/web-server-deploy.yaml` around lines 19 - 29, Add consistent pod- and container-level securityContext entries to the Deployment spec: under the Pod spec (same level as containers) add securityContext with runAsNonRoot: true and a seccompProfile (type: RuntimeDefault), and for the nginx container (name: "nginx") add a container-level securityContext with allowPrivilegeEscalation: false and runAsNonRoot: true; ensure field names match the existing test-client-pod.yaml pattern so the fixture aligns with other tests.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test/extended/router/cookies_otp.go`:
- Line 49: The test mutates a shared fixture by calling updateFilebySedCmd on
testPodSvc which can cause flakiness; instead create a temp copy of the fixture
or avoid in-place edits (e.g., use oc process with parameter overrides).
Implement by reading the file referenced by testPodSvc, writing a modified copy
to a temporary file (os.CreateTemp / os.WriteFile), replace "replicas: 1" →
"replicas: 2" in that temp content, use the temp file path for subsequent calls
instead of testPodSvc, and ensure the temp file is removed in a defer;
alternatively refactor the apply step to call oc process with parameter
overrides rather than mutating the file.
---
Nitpick comments:
In `@test/extended/router/cookies_otp.go`:
- Around line 130-135: Replace the shell cat invocation using
exec.Command("bash", "-c", cmd) with a direct file read via os.ReadFile(cookie):
call data, err := os.ReadFile(cookie) and check err with
o.Expect(err).NotTo(o.HaveOccurred()), then use string(data) in place of
string(cookiesOutput) for the subsequent assertions
(o.Expect(string(data)).NotTo(o.ContainSubstring("OPENSHIFT")) and
o.Expect(string(data)).NotTo(o.MatchRegexp(...))). Remove cmd and cookiesOutput
variables and import "os" if not already imported.
In `@test/extended/router/util_otp.go`:
- Around line 69-77: getRandomString currently creates a new rand.Source seeded
with time.Now().UnixNano() on every call which is inefficient and can collide
under rapid calls; change this to use a package-level seeded *rand.Rand or
rand.Source (e.g. a var seeded in init or at declaration) and have
getRandomString reuse that global RNG instead of calling
rand.New(rand.NewSource(...)) each time to improve efficiency and uniqueness
while keeping the same logic.
- Around line 39-46: The exactNodeDetails function currently counts status
keywords in the raw kubectl output which can be miscounted if node names include
those substrings; update exactNodeDetails to call
oc.AsAdmin().WithoutNamespace().Run("get").Args("nodes","--selector=node-role.kubernetes.io/worker=,kubernetes.io/os=linux","-o","jsonpath={range
.items[*]}{.metadata.name}:{range
.status.conditions[*]}{.type}={.status};{end}\n{end}").Output() (or equivalent
json/jsonpath) and parse each node's status conditions to compute nodeCount from
actual Condition type/status pairs instead of string-counting
"Ready"/"NotReady"/"SchedulingDisabled", keeping the returned values nodeCount
and the raw or formatted node details for logging.
- Around line 446-450: The helper updateFilebySedCmd is brittle because it
shells out to sed with '|' delimiters; replace it with pure-Go file
manipulation: read the file (os.ReadFile), perform a safe replacement
(strings.ReplaceAll on the file content using the provided toBeReplaced and
newContent values, or use regexp.QuoteMeta+ReplaceAllString for regex needs),
then write the updated content back (os.WriteFile) preserving original file
permissions (os.Stat to get FileMode); return or fail on any IO errors instead
of invoking bash/exec.Command. Ensure the function name updateFilebySedCmd is
updated to use this approach and that error assertions (o.Expect) still validate
the operation.
In `@test/extended/testdata/router/web-server-deploy.yaml`:
- Around line 19-29: Add consistent pod- and container-level securityContext
entries to the Deployment spec: under the Pod spec (same level as containers)
add securityContext with runAsNonRoot: true and a seccompProfile (type:
RuntimeDefault), and for the nginx container (name: "nginx") add a
container-level securityContext with allowPrivilegeEscalation: false and
runAsNonRoot: true; ensure field names match the existing test-client-pod.yaml
pattern so the fixture aligns with other tests.
In `@test/extended/testdata/router/web-server-signed-deploy.yaml`:
- Around line 34-64: The Deployment "web-server-deploy" is missing pod- and
container-level securityContext entries; add a pod securityContext under
spec.template.spec (e.g., runAsNonRoot: true, runAsUser: <non-root uid>,
fsGroup: <gid>) and add a container securityContext on the "nginx" container
(e.g., allowPrivilegeEscalation: false, readOnlyRootFilesystem: true, runAsUser
matching pod UID) to match the settings used in test-client-pod.yaml for
consistency and stricter security.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: db47178e-63d2-4739-901c-bf3babc8eb71
📒 Files selected for processing (6)
test/extended/router/cookies_otp.gotest/extended/router/util_otp.gotest/extended/testdata/router/ingresscontroller-np.yamltest/extended/testdata/router/test-client-pod.yamltest/extended/testdata/router/web-server-deploy.yamltest/extended/testdata/router/web-server-signed-deploy.yaml
| ensureRouterDeployGenerationIs(oc, ingctrl.name, "1") | ||
|
|
||
| g.By("2.0: Prepare file for testing") | ||
| updateFilebySedCmd(testPodSvc, "replicas: 1", "replicas: 2") |
There was a problem hiding this comment.
Mutating shared fixture files can cause test flakiness or pollution.
updateFilebySedCmd modifies web-server-deploy.yaml on disk. Multiple tests (OCP-11903, OCP-12566, OCP-15872) call this on the same file. If tests run in parallel or the file is already modified from a previous run, the sed replacement becomes a no-op (the pattern won't match). Consider either:
- Creating a temporary copy of the fixture per test, or
- Using
oc processwith parameter overrides instead of file mutation
🛠️ Suggested approach: copy fixture to temp file
// Create a temp copy of the fixture
tmpFile, err := os.CreateTemp("", "web-server-deploy-*.yaml")
o.Expect(err).NotTo(o.HaveOccurred())
defer os.Remove(tmpFile.Name())
content, err := os.ReadFile(testPodSvc)
o.Expect(err).NotTo(o.HaveOccurred())
modifiedContent := strings.ReplaceAll(string(content), "replicas: 1", "replicas: 2")
err = os.WriteFile(tmpFile.Name(), []byte(modifiedContent), 0644)
o.Expect(err).NotTo(o.HaveOccurred())
// Use tmpFile.Name() instead of testPodSvc🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@test/extended/router/cookies_otp.go` at line 49, The test mutates a shared
fixture by calling updateFilebySedCmd on testPodSvc which can cause flakiness;
instead create a temp copy of the fixture or avoid in-place edits (e.g., use oc
process with parameter overrides). Implement by reading the file referenced by
testPodSvc, writing a modified copy to a temporary file (os.CreateTemp /
os.WriteFile), replace "replicas: 1" → "replicas: 2" in that temp content, use
the temp file path for subsequent calls instead of testPodSvc, and ensure the
temp file is removed in a defer; alternatively refactor the apply step to call
oc process with parameter overrides rather than mutating the file.
Contributor
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: dgoodwin The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
test/extended/router/cookies_otp.go (1)
49-49:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAvoid mutating shared fixture files in-place across specs.
Line 49, Line 117, and Line 151 mutate
test/extended/testdata/router/web-server-deploy.yamldirectly. This can leak state between specs/retries and cause flaky behavior when runs overlap. Please use a per-spec temp copy (or parameterized processing) instead of editing the shared fixture.Suggested fix pattern (temp copy per spec)
+content, err := os.ReadFile(testPodSvc) +o.Expect(err).NotTo(o.HaveOccurred()) + +tmp, err := os.CreateTemp("", "web-server-deploy-*.yaml") +o.Expect(err).NotTo(o.HaveOccurred()) +defer os.Remove(tmp.Name()) + +patched := strings.ReplaceAll(string(content), "replicas: 1", "replicas: 2") +err = os.WriteFile(tmp.Name(), []byte(patched), 0644) +o.Expect(err).NotTo(o.HaveOccurred()) + - updateFilebySedCmd(testPodSvc, "replicas: 1", "replicas: 2") - srvPodList := createResourceFromWebServer(oc, ns, testPodSvc, srvrcInfo) + srvPodList := createResourceFromWebServer(oc, ns, tmp.Name(), srvrcInfo)Also applies to: 117-117, 151-151
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/extended/router/cookies_otp.go` at line 49, The test is mutating a shared fixture in-place via updateFilebySedCmd in cookies_otp.go; instead, copy the fixture to a per-spec temp file and run updateFilebySedCmd against that temp copy (createTempFile + io.Copy or os.ReadFile/os.WriteFile), pass the temp file path into any Deploy/Apply helper used in the spec, and remove the temp file in a defer cleanup so each spec operates on an isolated copy (update references to updateFilebySedCmd calls at lines around where it's used and ensure any helpers that load the fixture accept the temp path).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test/extended/router/cookies_otp.go`:
- Around line 131-133: Replace the shell-backed cat call that builds cmd and
runs exec.Command("bash", "-c", cmd) with a direct file read using
os.ReadFile(cookie); assign the returned []byte to cookiesOutput and handle the
error the same way (o.Expect(err).NotTo(o.HaveOccurred())). Update the
references to cmd and exec.Command to use os.ReadFile(cookie) so file contents
are read safely without spawning a shell.
---
Duplicate comments:
In `@test/extended/router/cookies_otp.go`:
- Line 49: The test is mutating a shared fixture in-place via updateFilebySedCmd
in cookies_otp.go; instead, copy the fixture to a per-spec temp file and run
updateFilebySedCmd against that temp copy (createTempFile + io.Copy or
os.ReadFile/os.WriteFile), pass the temp file path into any Deploy/Apply helper
used in the spec, and remove the temp file in a defer cleanup so each spec
operates on an isolated copy (update references to updateFilebySedCmd calls at
lines around where it's used and ensure any helpers that load the fixture accept
the temp path).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 0afd1ffe-a225-47bb-b998-9443ba5cb0cf
📒 Files selected for processing (2)
test/extended/router/cookies_otp.gotest/extended/testdata/bindata.go
✅ Files skipped from review due to trivial changes (1)
- test/extended/testdata/bindata.go
Comment on lines
+131
to
+133
| cmd := fmt.Sprintf(`cat %s`, cookie) | ||
| cookiesOutput, err := exec.Command("bash", "-c", cmd).Output() | ||
| o.Expect(err).NotTo(o.HaveOccurred()) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify there are no remaining shell-wrapped exec.Command usages in router extended tests
rg -nP 'exec\.Command\("bash",\s*"-c"' --type go test/extended/routerRepository: openshift/origin
Length of output: 362
🏁 Script executed:
#!/bin/bash
# Get the context around lines 131-133 in cookies_otp.go
sed -n '125,140p' test/extended/router/cookies_otp.go | cat -nRepository: openshift/origin
Length of output: 1059
🏁 Script executed:
#!/bin/bash
# Check the util_otp.go instances to see what they're doing
echo "=== util_otp.go:391 ==="
sed -n '385,395p' test/extended/router/util_otp.go | cat -n
echo ""
echo "=== util_otp.go:448 ==="
sed -n '442,452p' test/extended/router/util_otp.go | cat -nRepository: openshift/origin
Length of output: 907
🏁 Script executed:
#!/bin/bash
# Verify what imports are available in cookies_otp.go
head -30 test/extended/router/cookies_otp.go | cat -nRepository: openshift/origin
Length of output: 1332
Replace shell-wrapped cat with os.ReadFile() for cleaner, safer file reading.
Line 132 uses exec.Command("bash", "-c", cmd) to read a file with cat. Use os.ReadFile(cookie) instead, which returns []byte and error matching the existing error handling pattern.
Safer direct read
- cmd := fmt.Sprintf(`cat %s`, cookie)
- cookiesOutput, err := exec.Command("bash", "-c", cmd).Output()
+ cookiesOutput, err := os.ReadFile(cookie)
o.Expect(err).NotTo(o.HaveOccurred())🧰 Tools
🪛 OpenGrep (1.20.0)
[ERROR] 132-132: Dynamic command passed to exec.Command with a shell invocation. Pass arguments directly to exec.Command without a shell wrapper.
(coderabbit.command-injection.go-exec-command)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@test/extended/router/cookies_otp.go` around lines 131 - 133, Replace the
shell-backed cat call that builds cmd and runs exec.Command("bash", "-c", cmd)
with a direct file read using os.ReadFile(cookie); assign the returned []byte to
cookiesOutput and handle the error the same way
(o.Expect(err).NotTo(o.HaveOccurred())). Update the references to cmd and
exec.Command to use os.ReadFile(cookie) so file contents are read safely without
spawning a shell.
Contributor
|
@dgoodwin: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Testing a test porting agent, please be patient.
Agent will respond to coderabbit feedback, ensure the tests appear to be running and passing, then respond to human feedback.
Human feedback can request improvements or to skip porting a test entirely.
Summary
util_otp.goand 4 fixture YAML filesTest plan
go build ./test/extended/router/passes (verified locally)go vet ./test/extended/router/passes (verified locally)[OTP]tag🤖 Generated with Claude Code
Summary by CodeRabbit