api-test: Go conformance harness (mock + sunbird) — closes #2188, #2189, #2209 - #2230
Open
nandhu-kumar wants to merge 1 commit into
Open
api-test: Go conformance harness (mock + sunbird) — closes #2188, #2189, #2209#2230nandhu-kumar wants to merge 1 commit into
nandhu-kumar wants to merge 1 commit into
Conversation
|
Warning Review limit reached
Next review available in: 27 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (42)
WalkthroughThe PR adds a Go-based API test harness with conformance, BDD, and E2E surfaces; shared configuration, authentication drivers, result models, HTML reporting, Docker Compose packaging, and CI build/publish jobs. ChangesAPI test harness
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Harness
participant eSignet
participant Keycloak
participant Userinfo
Harness->>eSignet: Register throwaway OAuth client
Harness->>eSignet: Start authorize flow
eSignet->>Keycloak: Request client-credentials token
eSignet-->>Harness: Return authorization redirect
Harness->>eSignet: Exchange code with PKCE and private_key_jwt
Harness->>Userinfo: Request claims
Userinfo-->>Harness: Return JSON or signed JWS claims
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related issues
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (3 warnings)
✅ Passed checks (2 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop-go #2230 +/- ##
=============================================
Coverage ? 81.28%
=============================================
Files ? 59
Lines ? 4376
Branches ? 0
=============================================
Hits ? 3557
Misses ? 625
Partials ? 194
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
nandhu-kumar
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 21
🤖 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 @.github/workflows/push-trigger.yml:
- Around line 127-141: Update build_go_apitest to grant the job only read-level
repository contents permission, and configure its actions/checkout step to avoid
persisting the GITHUB_TOKEN in .git/config. Leave the existing Go build and vet
steps unchanged.
In `@api-test/bdd/engine_test.go`:
- Around line 30-43: The tag assignment in the Keycloak-secret branch currently
overwrites the earlier `@flow-authz-neg` tag; update this branch to preserve
previously accumulated tags while adding `@flow-execute` and `@client-mgmt`. Keep
the existing conditional addition of `@client-mgmt-pms` unchanged so all
applicable tags run together.
In `@api-test/bdd/features/client-mgmt/create-client.feature`:
- Line 20: Replace the misleading existence assertion for the "response" JSON
path in the rejected create scenario with an explicit null assertion. Reuse an
existing null-check step if available; otherwise add a dedicated null-value step
in the BDD step definitions and use it here, rather than asserting path
existence.
In `@api-test/bdd/features/client-mgmt/get-update-client.feature`:
- Around line 13-14: Remove the exact errorMessage assertion from the scenario
in get-update-client.feature, keeping the stable errors.0.errorCode assertion
for validating the client-not-found response.
In `@api-test/bdd/steps.go`:
- Around line 207-227: Update state.record to redact all credential-bearing
headers on both request and response sides: Authorization, Cookie,
X-API-KEY/Api-Key, and Set-Cookie, while preserving other header values. Apply
the same redaction before assigning ReqHeaders and RespHeaders so session tokens
cannot enter published reports.
- Around line 133-145: Update the TLS configurations used by httpClient and
noFollowClient to set a secure minimum TLS version and enable InsecureSkipVerify
only when an explicit environment flag opts in, defaulting to certificate
verification so CI and real deployments fail closed. Reuse the resulting TLS
configuration consistently for both clients while preserving noFollowClient’s
redirect behavior.
In `@api-test/cmd/consolidate/main.go`:
- Around line 44-81: Update the source-loading flow in the main consolidation
function to stamp missing surfaces immediately after each load: use the
conformance surface for loadConformance results and the appropriate BDD or E2E
surface for each loadEnvelope result. Add and reuse a stampSurface helper for
this defaulting, then remove the global loop’s SurfaceConformance assignment
while preserving plugin stamping.
In `@api-test/cmd/e2e/main.go`:
- Around line 41-45: Make TLS verification fail closed in main.go by setting
tlsVerify from BDD_TLS_VERIFY so only the explicit value "false" disables it;
add tls.VersionTLS12 as MinVersion in the tls.Config; and explicitly set
BDD_TLS_VERIFY to ${BDD_TLS_VERIFY:-true} in api-test/docker-compose.yml under
the harness environment.
- Around line 200-206: Update writeEnvelope to create the parent directory of
path before calling os.WriteFile, using filepath.Dir and ensuring directory
creation succeeds; preserve the existing JSON marshaling and file-writing
behavior.
In `@api-test/Dockerfile`:
- Around line 26-31: Update the builder-bdd Dockerfile step invoking go test -c
to target the bdd root package explicitly instead of ./..., while preserving the
existing output path and compilation flags.
In `@api-test/internal/config/config.go`:
- Around line 297-299: Update Redacted() to mask or omit all sensitive report
configuration fields before json.MarshalIndent, including otp.value,
individual_id, recipient email, name, and date of birth, while retaining the
existing conformance token and Esignet password masking. Ensure the sanitized
clone is the payload persisted by the report sidecar.
- Around line 22-25: Update api-test/internal/config/config.go and
api-test/config.example.json so TLS verification defaults to enabled when
tls_verify is omitted, with disabling available only through an explicit false
value. Parse CONFORMANCE_TLS_VERIFY strictly, reject malformed values, and
ensure the resulting setting remains enforced by both the conformance client and
eSignet driver.
In `@api-test/internal/e2e/e2e.go`:
- Around line 323-350: Update createClientViaClientMgmt and createClientViaPMS
to explicitly reject any non-2xx HTTP status before treating the response as
successful. Preserve the existing firstErrorCode checks, then return a
descriptive registration error containing the HTTP status when the status is
outside the success range; in createClientViaPMS, perform this check before
parsing the response clientId.
In `@api-test/internal/e2e/helpers.go`:
- Around line 158-164: Update snippet to truncate at most 300 bytes without
splitting a UTF-8 rune: when the input exceeds n, back off from the byte
boundary to the last complete rune before appending the ellipsis. Preserve the
existing behavior for inputs within the limit.
In `@api-test/internal/esignet/answers.go`:
- Around line 41-73: Add direct table-driven unit tests for AuthFactorTokens and
IDTypeTokens, covering normalized aliases, canonical token lists, empty inputs,
and unknown values. Assert exact returned slices and preserve the functions’
case-insensitive, whitespace-trimming behavior; keep the tests independent of
driver or integration flows.
In `@api-test/internal/esignet/driver.go`:
- Around line 142-191: Extend the redaction flow used by redactCallBodies() and
headerStr() to cover PII fields such as individualId, full name, DOB, email, and
phone in request/response bodies, and mask cookies plus headers or Set-Cookie
values containing session, cookie, auth, jot, oidc, or bearer-like credentials.
Preserve existing password, otp, assertion, token, secret, and Authorization
redaction while ensuring retained HTTPCall artifacts never expose these values.
In `@api-test/internal/orchestrator/run.go`:
- Around line 261-264: Update the stale comment in driveOne to state that
deliver and other suite-plumbing HTTP calls are discarded by runModule, not
merged into the final report. Keep the existing runModule behavior around
o.client.TakeCalls() unchanged, and apply the same wording correction to both
referenced comment locations.
In `@api-test/internal/report/html_test.go`:
- Around line 100-125: Add table-driven cases to TestRedactCallBodies covering a
form-encoded client_secret body and a URL containing code=, asserting sensitive
values are replaced with ***redacted*** while preserving the existing JSON and
immutability checks. Update redactCallBodies and its underlying redaction logic
to handle these non-JSON inputs consistently.
In `@api-test/internal/report/html.go`:
- Around line 383-426: Extend the redaction flow around redactBody and
redactCallBodies to cover form-encoded bodies and URL query parameters, not only
JSON. Add the sensitive keys code, client_assertion, and code_verifier,
implement form parsing/redaction and URL query redaction using the existing
sensitive-key matching helper, and apply redactURL to captured call URLs in both
redactCallBodies and toCallViews so HTML and sidecar output match.
In `@api-test/internal/result/result.go`:
- Around line 119-121: Update the duplicate-call matching logic around the
Repeat increment to collapse requests only when all semantically relevant HTTP
metadata matches, including authorization and other request headers, cookies,
labels, and response headers, in addition to method, URL, status, and bodies.
Keep different calls as separate entries so each retains its own metadata and
repeat count.
In `@api-test/run-all.sh`:
- Around line 33-34: Guard the directory change in run-all.sh by making the cd
command failure terminate the script, while preserving the existing set -uo
pipefail behavior and subsequent execution from the script directory.
🪄 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 UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 6bf13be0-09ff-488d-86b3-4d760077c91d
⛔ Files ignored due to path filters (1)
api-test/bdd/go.sumis excluded by!**/*.sum
📒 Files selected for processing (41)
.github/workflows/push-trigger.ymlapi-test/.dockerignoreapi-test/.env.exampleapi-test/.gitignoreapi-test/Dockerfileapi-test/README.mdapi-test/bdd/engine_test.goapi-test/bdd/features/client-mgmt/create-client-validation.featureapi-test/bdd/features/client-mgmt/create-client.featureapi-test/bdd/features/client-mgmt/create-update-client-pms.featureapi-test/bdd/features/client-mgmt/create-update-client-positive.featureapi-test/bdd/features/client-mgmt/get-update-client.featureapi-test/bdd/features/flow-execute/authorize-negative.featureapi-test/bdd/features/flow-execute/meta.featureapi-test/bdd/features/flow-execute/negative.featureapi-test/bdd/go.modapi-test/bdd/steps.goapi-test/cmd/conformance/main.goapi-test/cmd/consolidate/main.goapi-test/cmd/e2e/main.goapi-test/config.example.jsonapi-test/docker-compose.ymlapi-test/e2e-scenarios-sunbird.jsonapi-test/e2e-scenarios.jsonapi-test/go.modapi-test/internal/config/config.goapi-test/internal/conformance/client.goapi-test/internal/e2e/crypto.goapi-test/internal/e2e/e2e.goapi-test/internal/e2e/e2e_test.goapi-test/internal/e2e/helpers.goapi-test/internal/esignet/answers.goapi-test/internal/esignet/driver.goapi-test/internal/esignet/driver_test.goapi-test/internal/httpx/httpx.goapi-test/internal/orchestrator/run.goapi-test/internal/report/html.goapi-test/internal/report/html_test.goapi-test/internal/result/result.goapi-test/profiles/oidcc-test-plan.smoke.jsonapi-test/run-all.sh
nandhu-kumar
force-pushed
the
api-test-harness
branch
from
July 27, 2026 10:49
9d19c95 to
9ee8985
Compare
There was a problem hiding this comment.
Actionable comments posted: 25
🤖 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 @.github/workflows/push-trigger.yml:
- Line 132: Pin the third-party actions referenced in the workflow, including
actions/checkout and actions/setup-go, to their trusted full commit SHAs instead
of mutable version tags. Update each corresponding uses entry while preserving
the existing action versions and workflow behavior.
- Around line 127-145: Add a timeout-minutes setting to the build_go_apitest
job, using the repository’s established CI timeout convention if available, so
hung go build or go vet commands terminate promptly. Leave the existing
checkout, Go setup, and validation steps unchanged.
In `@api-test/.env.example`:
- Around line 41-43: Update the SUITE_IMAGE_TAG default in the environment
example from latest to the tested conformance suite release tag 5.1.41, keeping
the smoke profile and suite image versions synchronized for reproducible runs.
In `@api-test/bdd/engine_test.go`:
- Around line 29-34: Update the flow tag-selection logic in the test setup
around the `@flow-authz-neg` gate to append the established ENV_NOT_READY fallback
row when FLOW_CLIENT_ID is unset, matching the existing client-mgmt gate
behavior around KEYCLOAK_CLIENT_SECRET. Preserve `@flow-authz-neg` execution when
FLOW_CLIENT_ID is configured, and ensure the consolidated report explicitly
records the skipped authorize-negative coverage.
In `@api-test/cmd/consolidate/main.go`:
- Around line 72-74: Update the fatal error message in the rows-empty check to
include the valid -e2e source alongside -bdd and -conformance, while preserving
the existing consolidation flow.
- Around line 104-115: Move the loadEnvelope-specific documentation paragraph so
it directly precedes the loadEnvelope declaration, and leave stampSurface
documented only by its own surface-defaulting comment. Ensure godoc associates
each comment with the correct function.
In `@api-test/cmd/e2e/main.go`:
- Around line 141-154: Update fetchDiscovery and the underlying httpGet flow to
validate the HTTP response status before unmarshalling the body, returning an
error that includes the non-success status for 404/502 responses. Preserve JSON
parsing for successful responses and ensure the same status handling applies to
the additional call site noted in the comment.
- Around line 66-68: Update the IndividualID initialization in the Esignet
configuration to default INDIVIDUAL_ID to an empty value instead of a real phone
number, while preserving the existing environment-variable override and other
identity defaults.
In `@api-test/docker-compose.yml`:
- Around line 80-81: Update the CONFORMANCE_TLS_VERIFY setting in the compose
environment to be environment-driven with the existing local
self-signed-certificate behavior as its default, allowing remote or shared
CONFORMANCE_BASE_URL overrides to enable certificate validation.
In `@api-test/Dockerfile`:
- Around line 55-58: The Docker setup does not ensure non-root user apitest can
write to the bind-mounted out directory. Update the compose configuration or
documented run instructions around USER apitest:apitest, ./out, and RUN_UID so
the container user can be overridden or the host directory ownership is prepared
before report generation.
In `@api-test/internal/config/config.go`:
- Around line 247-252: Update the configuration validation flow in the method
containing the Run.Profile switch to validate poll_interval_seconds and timeout
after defaults are applied, rejecting zero or negative values with configuration
errors before polling uses them. Preserve the existing smoke/full profile
validation and successful return behavior.
In `@api-test/internal/conformance/client.go`:
- Around line 281-314: Set HTTPCall.Seq in Client.doAbs when constructing the
call, using the next sequence value based on the current length of c.calls
(len(c.calls)+1), matching the sequencing behavior in session.do. Ensure this
applies to both successful requests and request/transport error paths.
In `@api-test/internal/e2e/crypto.go`:
- Around line 170-187: Update fetchJWKS to validate the HTTP response status
after c.Get and before decoding the body, returning a descriptive
endpoint-unreachable error for non-success responses. Preserve the existing JSON
parsing and key-return behavior for successful responses so verifyJWS can
distinguish endpoint failures from signature-verification failures.
- Around line 189-202: Update rsaPubFromJWK to reject missing or invalid
modulus/exponent bytes before constructing the key, and validate the decoded
exponent fits safely in int without truncation on any supported architecture.
Return an explicit error for absent values or oversized exponents; only
construct rsa.PublicKey after these checks.
In `@api-test/internal/e2e/e2e.go`:
- Around line 104-112: Update Runner.httpClient to set tls.Config.MinVersion
explicitly to TLS 1.2 alongside the existing InsecureSkipVerify configuration,
preserving the current toggle-controlled certificate verification behavior.
- Around line 478-487: Update the userinfo request flow around r.do and
parseUserinfo to retain and validate the returned HTTP status before parsing the
response body. Treat non-success statuses as userinfo request failures,
returning an error that includes the HTTP status and avoids claim assertions or
parsing of error bodies; preserve the existing parsing path for successful
responses.
In `@api-test/internal/esignet/driver_test.go`:
- Around line 56-68: Extend TestResolveInputs to cover the resolveInputs
fallback that invokes d.otp when no explicit "otp" answer is present. Add a
minimal fake OTP provider and assertions for both successful generated OTP
resolution and the provider-error path, while preserving the existing
explicit-answer and missing-input checks.
In `@api-test/internal/esignet/driver.go`:
- Around line 382-450: Update selectAction to implement the documented
navigation fallback: when nonNav is empty, choose a navigation candidate as the
final resort instead of returning ambiguity. Preserve the existing
non-navigation selection behavior, and provide a distinct error for multiple
non-navigation candidates that continues to direct callers to
esignet.auth_factor.
In `@api-test/internal/httpx/httpx.go`:
- Around line 15-20: The shared TLS configuration in NewClient must enforce TLS
1.2 or newer, and fetchJWKS must reuse that client instead of constructing a
duplicate inline client. Update api-test/internal/httpx/httpx.go lines 15-20 to
set MinVersion to tls.VersionTLS12; update api-test/internal/e2e/crypto.go lines
170-187 to call httpx.NewClient(tlsVerify, 15*time.Second) and remove the local
client/TLS configuration.
In `@api-test/internal/orchestrator/run.go`:
- Around line 324-396: Update loadSmokeProfile to resolve the smoke profile
through the configured project/root directory or executable-relative base
instead of the current-working-directory-relative filepath.Join("profiles",
...). Preserve the existing profile filename, file-reading, parsing, and
validation behavior while removing the requirement that callers launch from
api-test/.
In `@api-test/internal/report/html_test.go`:
- Around line 127-149: Add fragment and cookie redaction cases to
TestRedactCallBodiesFormAndURL, covering sensitive values in URL fragments and
Cookie headers while asserting non-sensitive values remain. Align the cases with
the redaction behavior implemented in redactCallBodies and the relevant html.go
redaction paths.
In `@api-test/internal/report/html.go`:
- Around line 266-286: Update cookie handling in toCallViews and
redactCallBodies so ReqCookies and RespCookies pass through a shared
redactCookies helper before being stored or serialized. Preserve cookie names
while masking every value, including empty or whitespace-separated inputs, and
keep the redacted HTML and sidecar JSON representations consistent.
- Around line 426-451: Update redactURL to split the URL at both '?' and '#',
redact query and fragment components independently using a shared helper such as
redactQuery, and reassemble them while preserving their delimiters and unchanged
portions. Ensure sensitive keys, including code and token parameters in either
component, are masked without allowing fragment values to remain embedded in or
bypass the query redaction.
In `@api-test/run-all.sh`:
- Around line 85-108: Track whether each selected surface fails when invoking
run_bdd or run_e2e, rather than swallowing non-zero statuses in the current ||
echo handlers. Ensure a crashed or failed surface is recorded even when its
envelope is absent, and propagate the recorded failure after run_consolidate so
CI exits non-zero while preserving consolidation and existing scenario-failure
messaging.
- Around line 57-69: Validate SUITE_WAIT_SECONDS in wait_for_suite before
performing arithmetic or the numeric comparison, treating non-integer values
such as “90s” as invalid and applying the existing safe wait behavior instead of
calculating a garbage deadline. Preserve the zero-value early return and normal
polling behavior for valid non-negative integers.
🪄 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 UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 7554cd6e-961d-482f-b8f2-ac42cc1d77b7
⛔ Files ignored due to path filters (1)
api-test/bdd/go.sumis excluded by!**/*.sum
📒 Files selected for processing (41)
.github/workflows/push-trigger.ymlapi-test/.dockerignoreapi-test/.env.exampleapi-test/.gitignoreapi-test/Dockerfileapi-test/README.mdapi-test/bdd/engine_test.goapi-test/bdd/features/client-mgmt/create-client-validation.featureapi-test/bdd/features/client-mgmt/create-client.featureapi-test/bdd/features/client-mgmt/create-update-client-pms.featureapi-test/bdd/features/client-mgmt/create-update-client-positive.featureapi-test/bdd/features/client-mgmt/get-update-client.featureapi-test/bdd/features/flow-execute/authorize-negative.featureapi-test/bdd/features/flow-execute/meta.featureapi-test/bdd/features/flow-execute/negative.featureapi-test/bdd/go.modapi-test/bdd/steps.goapi-test/cmd/conformance/main.goapi-test/cmd/consolidate/main.goapi-test/cmd/e2e/main.goapi-test/config.example.jsonapi-test/docker-compose.ymlapi-test/e2e-scenarios-sunbird.jsonapi-test/e2e-scenarios.jsonapi-test/go.modapi-test/internal/config/config.goapi-test/internal/conformance/client.goapi-test/internal/e2e/crypto.goapi-test/internal/e2e/e2e.goapi-test/internal/e2e/e2e_test.goapi-test/internal/e2e/helpers.goapi-test/internal/esignet/answers.goapi-test/internal/esignet/driver.goapi-test/internal/esignet/driver_test.goapi-test/internal/httpx/httpx.goapi-test/internal/orchestrator/run.goapi-test/internal/report/html.goapi-test/internal/report/html_test.goapi-test/internal/result/result.goapi-test/profiles/oidcc-test-plan.smoke.jsonapi-test/run-all.sh
This was referenced Jul 27, 2026
Open
nandhu-kumar
force-pushed
the
api-test-harness
branch
from
July 27, 2026 13:58
9ee8985 to
e75fd52
Compare
There was a problem hiding this comment.
Actionable comments posted: 12
🤖 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 `@api-test/bdd/features/client-mgmt/create-update-client-positive.feature`:
- Around line 120-183: Refresh the request timestamp before every client update
in both feature scenarios. In
api-test/bdd/features/client-mgmt/create-update-client-positive.feature lines
120-183, invoke “Given a fresh request timestamp” before the deactivate and
reactivate PUT requests; in
api-test/bdd/features/client-mgmt/create-update-client-pms.feature lines 97-155,
invoke it before the update, deactivate, and reactivate PUT requests, while
preserving each existing request body and assertions.
In `@api-test/bdd/go.mod`:
- Around line 3-8: Align the Go toolchain declaration in the BDD module with the
version used by its Docker build stage. Update the go directive in
api-test/bdd/go.mod from 1.25.5 to Go 1.26, preserving the existing dependency
requirements.
In `@api-test/cmd/e2e/main.go`:
- Around line 173-197: Update keycloakToken to validate resp.StatusCode before
decoding the response body, matching the status-first handling in
fetchDiscovery. For non-2xx responses, return an error that includes the HTTP
status and relevant response details; only decode the access-token payload for
successful responses.
In `@api-test/docker-compose.yml`:
- Around line 80-83: Make CONFORMANCE_BASE_URL environment-overridable in the
Docker Compose configuration, using the existing localhost.emobix.co.uk URL as
its default. Keep CONFORMANCE_TLS_VERIFY’s behavior and accompanying guidance
aligned with this override.
In `@api-test/e2e-scenarios.json`:
- Around line 50-55: Remove the hardcoded username and password override from
the “password positive: login succeeds with configured password” scenario so it
uses the base answers generated by esignet.BuildAnswers from
config.Esignet.Credentials. Ensure the test setup reads the required values from
ESIGNET_USERNAME and ESIGNET_PASSWORD environment variables instead.
In `@api-test/internal/conformance/client.go`:
- Around line 297-300: Update the request-header capture in doAbs so the
Authorization header is redacted before assigning the cloned headers to
call.ReqHeaders. Preserve the outgoing request’s Bearer token while ensuring
retained call traces contain no suite token, reusing the existing
header-redaction convention if available.
In `@api-test/internal/e2e/e2e.go`:
- Around line 138-142: Update the response capture flow to assign
rec.RespHeaders using the shared httpx.CloneHeader helper instead of directly
converting resp.Header, and add the corresponding httpx import. Keep the
existing status, body, call-record append, and return behavior unchanged.
In `@api-test/internal/e2e/helpers.go`:
- Around line 32-39: Update parseUserinfo and the assertClaims evaluation flow
so a failed verifyJWS result for userinfo becomes a failed scenario assertion,
rather than only recording _jws_verified and _jws_error claims. Ensure
successful verification passes, failures propagate the verification error into
the scenario verdict, and retain the existing claim metadata for diagnostics.
In `@api-test/internal/esignet/driver_test.go`:
- Around line 16-27: Add a table-driven test case for selectAction covering a
step whose available actions consist only of login_id_* navigation tabs, and
assert the expected stage-5 fallback action and non-ambiguous result. Place it
alongside the existing action-selection cases in the test table.
In `@api-test/internal/orchestrator/run.go`:
- Around line 245-253: Update the final GetInfo handling in the orchestrator run
flow so a successful re-fetch only replaces the loop-captured status and result
when the returned fields are present and valid. Preserve previously captured
verdict values when the response contains an empty Result (and corresponding
partial status data), while retaining the existing error fallback behavior.
In `@api-test/internal/result/result.go`:
- Around line 101-107: Implement credential redaction before report
serialization by adding a helper such as redactCallCredentials that deep-copies
each HTTPCall, masks Authorization, Proxy-Authorization, Cookie, and Set-Cookie
header values case-insensitively, and masks ReqCookies and RespCookies. Ensure
report sidecar persistence and HTML rendering consume the redacted copies while
leaving the original HTTPCall data unchanged.
In `@api-test/run-all.sh`:
- Around line 84-93: Update the conformance branch around run_conformance to
verify that a report envelope was produced before consolidation, matching the
existing run_bdd/run_e2e checks. When no conformance report exists, mark the
surface as failed and preserve the failure status so a requested but crashed
conformance run cannot exit successfully or be silently omitted.
🪄 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 UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: da4d3696-482c-4d1c-81a6-00966d7963ec
⛔ Files ignored due to path filters (1)
api-test/bdd/go.sumis excluded by!**/*.sum
📒 Files selected for processing (42)
.github/workflows/push-trigger.ymlapi-test/.dockerignoreapi-test/.env.exampleapi-test/.gitignoreapi-test/Dockerfileapi-test/README.mdapi-test/bdd/engine_test.goapi-test/bdd/features/client-mgmt/create-client-validation.featureapi-test/bdd/features/client-mgmt/create-client.featureapi-test/bdd/features/client-mgmt/create-update-client-pms.featureapi-test/bdd/features/client-mgmt/create-update-client-positive.featureapi-test/bdd/features/client-mgmt/get-update-client.featureapi-test/bdd/features/flow-execute/authorize-negative.featureapi-test/bdd/features/flow-execute/meta.featureapi-test/bdd/features/flow-execute/negative.featureapi-test/bdd/go.modapi-test/bdd/steps.goapi-test/cmd/conformance/main.goapi-test/cmd/consolidate/main.goapi-test/cmd/e2e/main.goapi-test/config.example.jsonapi-test/docker-compose.ymlapi-test/e2e-scenarios-sunbird.jsonapi-test/e2e-scenarios.jsonapi-test/go.modapi-test/internal/config/config.goapi-test/internal/config/config_test.goapi-test/internal/conformance/client.goapi-test/internal/e2e/crypto.goapi-test/internal/e2e/e2e.goapi-test/internal/e2e/e2e_test.goapi-test/internal/e2e/helpers.goapi-test/internal/esignet/answers.goapi-test/internal/esignet/driver.goapi-test/internal/esignet/driver_test.goapi-test/internal/httpx/httpx.goapi-test/internal/orchestrator/run.goapi-test/internal/report/html.goapi-test/internal/report/html_test.goapi-test/internal/result/result.goapi-test/profiles/oidcc-test-plan.smoke.jsonapi-test/run-all.sh
nandhu-kumar
force-pushed
the
api-test-harness
branch
from
July 27, 2026 18:07
e75fd52 to
41283cd
Compare
There was a problem hiding this comment.
Actionable comments posted: 7
♻️ Duplicate comments (1)
api-test/internal/result/result.go (1)
110-132: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
CollapseCallsstill merges non-identical HTTP calls — prior fix not present in this snapshot.A previous review flagged this exact comparison (missing
Label,ReqHeaders,ReqCookies,RespHeaders,RespCookies) and was marked "✅ Addressed in commit 9ee8985," but the code here is identical to the pre-fix version. Calls that differ only by auth header, cookie, or label are still collapsed into one row with an inflatedRepeatcount, hiding the discrepancy from anyone using the report to audit the eSignet-thunder wire trace — this weakens MOSIP conformance debugging evidence, since the "collapsed" row misrepresents what actually happened on the wire.🐛 Proposed fix
+import "reflect" - if p.Method == c.Method && p.URL == c.URL && p.Status == c.Status && - p.ReqBody == c.ReqBody && p.RespBody == c.RespBody { + if p.Label == c.Label && p.Method == c.Method && p.URL == c.URL && + p.Status == c.Status && p.ReqBody == c.ReqBody && + p.RespBody == c.RespBody && p.ReqCookies == c.ReqCookies && + p.RespCookies == c.RespCookies && + reflect.DeepEqual(p.ReqHeaders, c.ReqHeaders) && + reflect.DeepEqual(p.RespHeaders, c.RespHeaders) {Please confirm whether commit 9ee8985 actually landed this change or was reverted/lost in a rebase.
🤖 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 `@api-test/internal/result/result.go` around lines 110 - 132, Update CollapseCalls so calls are merged only when all identity fields match, including Label, ReqHeaders, ReqCookies, RespHeaders, and RespCookies in addition to the existing method, URL, status, and body comparisons. Preserve the current sorting, Repeat counting, and Seq renumbering behavior.
🤖 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 `@api-test/cmd/e2e/main.go`:
- Around line 105-126: Validate the directly constructed e2e configuration
before loading or executing the spec, ensuring OTP_SOURCE=dynamic is rejected
with a clear configuration error. Update the entrypoint’s config setup near the
existing INDIVIDUAL_ID handling and invoke the available config validation
method, failing fast through logger.Fatalf; preserve static OTP behavior and
avoid relying on the later nil otpProvider path.
In `@api-test/internal/config/config.go`:
- Around line 149-163: Update applyEnv() to map the ID_TYPE environment variable
into c.Esignet.Identity.IDType alongside the existing INDIVIDUAL_ID mapping,
preserving the configured login-id-type preference for downstream
esignet.IDTypeTokens processing.
In `@api-test/internal/report/html_test.go`:
- Around line 100-125: Add a regression test alongside TestRedactCallBodies
named TestRedactLogItemDetails that constructs a result with a RESPONSE LogItem
whose response_body detail contains an access_token, then verifies
redactCallBodies removes the token from LogItems[].Details[].Value. Use the
existing redaction marker or equivalent assertion, and confirm the original
input remains unaffected if consistent with the surrounding tests.
In `@api-test/internal/report/html.go`:
- Around line 442-465: Extend redactCallBodies in
api-test/internal/report/html.go to redact suite LogItems alongside Calls: apply
redactBody to each LogItems[].Msg and Details[].Value, masking values outright
when their detail key matches containsSensitiveKey or isSensitiveParam, while
preserving the existing redaction flow. Add a regression test in
api-test/internal/report/html_test.go covering a response_body detail containing
access_token and assert the token is absent after redactCallBodies.
- Around line 633-648: Update redactSensitiveMode so any scalar value under a
sensitive key is replaced with "***redacted***", regardless of whether it is a
string; retain the existing non-empty string behavior and request identity-key
handling, while continuing recursive traversal for nested maps and arrays.
In `@api-test/README.md`:
- Around line 345-356: Update the redaction coverage description in the README
to include code_verifier and refresh_token, along with request and response
cookies, matching the behavior implemented by sensitiveParams and html.go. Keep
the existing distinctions about request-only identity redaction and readable
userinfo claims unchanged.
In `@api-test/run-all.sh`:
- Around line 7-15: Update the header documentation in run-all.sh to include the
e2e surface alongside conformance and bdd, and document that the SURFACES
default includes all three values, matching the script’s actual behavior.
---
Duplicate comments:
In `@api-test/internal/result/result.go`:
- Around line 110-132: Update CollapseCalls so calls are merged only when all
identity fields match, including Label, ReqHeaders, ReqCookies, RespHeaders, and
RespCookies in addition to the existing method, URL, status, and body
comparisons. Preserve the current sorting, Repeat counting, and Seq renumbering
behavior.
🪄 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 UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: d7f6d498-4b24-44ba-a244-b395539865bf
⛔ Files ignored due to path filters (1)
api-test/bdd/go.sumis excluded by!**/*.sum
📒 Files selected for processing (42)
.github/workflows/push-trigger.ymlapi-test/.dockerignoreapi-test/.env.exampleapi-test/.gitignoreapi-test/Dockerfileapi-test/README.mdapi-test/bdd/engine_test.goapi-test/bdd/features/client-mgmt/create-client-validation.featureapi-test/bdd/features/client-mgmt/create-client.featureapi-test/bdd/features/client-mgmt/create-update-client-pms.featureapi-test/bdd/features/client-mgmt/create-update-client-positive.featureapi-test/bdd/features/client-mgmt/get-update-client.featureapi-test/bdd/features/flow-execute/authorize-negative.featureapi-test/bdd/features/flow-execute/meta.featureapi-test/bdd/features/flow-execute/negative.featureapi-test/bdd/go.modapi-test/bdd/steps.goapi-test/cmd/conformance/main.goapi-test/cmd/consolidate/main.goapi-test/cmd/e2e/main.goapi-test/config.example.jsonapi-test/docker-compose.ymlapi-test/e2e-scenarios-sunbird.jsonapi-test/e2e-scenarios.jsonapi-test/go.modapi-test/internal/config/config.goapi-test/internal/config/config_test.goapi-test/internal/conformance/client.goapi-test/internal/e2e/crypto.goapi-test/internal/e2e/e2e.goapi-test/internal/e2e/e2e_test.goapi-test/internal/e2e/helpers.goapi-test/internal/esignet/answers.goapi-test/internal/esignet/driver.goapi-test/internal/esignet/driver_test.goapi-test/internal/httpx/httpx.goapi-test/internal/orchestrator/run.goapi-test/internal/report/html.goapi-test/internal/report/html_test.goapi-test/internal/result/result.goapi-test/profiles/oidcc-test-plan.smoke.jsonapi-test/run-all.sh
Replaces the removed Java api-test rig with a stdlib-only Go harness driving four surfaces via run-all.sh: conformance (OpenID Conformance Suite orchestration + eSignet flow driver), bdd (nested godog module), e2e, and consolidate (one HTML report). Scope is mock and sunbird, both exercised end-to-end. The mosipid plugin needs PMS client registration and a dynamic OTP source that are still unverified, so AUTHN_PROVIDER=mosip and OTP_SOURCE=dynamic are rejected by config validation; internal/wsotp and e2e-scenarios-mosip.json land with that follow-up. Signed-off-by: Nandhukumar <nandhukumare@gmail.com>
nandhu-kumar
force-pushed
the
api-test-harness
branch
from
July 27, 2026 18:35
41283cd to
b3a3816
Compare
mohanachandran-s
approved these changes
Aug 3, 2026
4 tasks
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
4 participants
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.
Part of #2120. Closes #2188, closes #2189, closes #2209.
Replaces the removed Java api-test rig with a stdlib-only Go harness driving four surfaces via run-all.sh: conformance (OpenID Conformance Suite orchestration + eSignet flow driver), bdd (nested godog module), e2e, and consolidate (one HTML report).
Scope is mock and sunbird, both exercised end-to-end, plus the Docker Compose + CI setup to run the harness with one command. The mosipid plugin needs PMS client registration and a dynamic OTP source that are still unverified, so
AUTHN_PROVIDER=mosipandOTP_SOURCE=dynamicare rejected by config validation;internal/wsotpande2e-scenarios-mosip.jsonland in a follow-up PR (tracked in #2190).Sub-issues closed by this PR
Summary by CodeRabbit
run-allscript to run selected surfaces and consolidate results.