Establish readable E2E journey harness - #1635
Conversation
9ee3bec to
76c5ee4
Compare
There was a problem hiding this comment.
6 issues found across 187 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="e2e/console/webhook_rbac_test.go">
<violation number="1" location="e2e/console/webhook_rbac_test.go:148">
P3: Most subtests here create a webhook subscription but never tear it down (only the delete allow-cases remove their own), leaving orphaned subscriptions with unroutable endpoints behind on every run of the suite. In the persistent local/dev e2e database this accumulates rows and spawns extra bounded webhook-worker delivery attempts per run; consider deleting each created subscription at the end of its subtest (or relying on a documented org-level cleanup) and noting the cleanup convention in e2e.md.</violation>
</file>
<file name="contrib/claude/e2e-refactor.md">
<violation number="1" location="contrib/claude/e2e-refactor.md:1">
P3: This new migration-plan guide exists in the doc graph (e2e.md links to it) but is missing from the AGENTS.md guide index, so it won't surface as a discoverable subsystem guide. Consider adding a line to AGENTS.md pointing at contrib/claude/e2e-refactor.md alongside e2e.md.</violation>
</file>
<file name="e2e/internal/testutil/client.go">
<violation number="1" location="e2e/internal/testutil/client.go:841">
P3: The `return nil` after `t.Fatal("client is nil")` is unreachable dead code: `t.Fatal` never returns at runtime, and the compiler is already satisfied by the function's terminating `return &bound`. Dropping the line keeps behavior identical and removes the misleading implied nil path.</violation>
</file>
<file name="e2e/internal/testutil/mcp.go">
<violation number="1" location="e2e/internal/testutil/mcp.go:234">
P2: The new SSE decoder for the MCP Streamable HTTP transport returns the first single-line `data:` payload that parses as a JSON-RPC message with a non-nil id, without checking that the id matches the current request (always 1) and without supporting SSE events whose payload spans multiple `data:` lines. A large or multi-line MCP result can therefore be rejected as 'SSE response did not contain a JSON-RPC message' even though it's valid, making the harness brittle against the wire format it's supposed to mirror. Consider accumulating `data:` lines per event, forgiving continuation lines, and verifying the returned `id` matches the request before accepting the message.</violation>
</file>
<file name="e2e/console/control_applicability_helpers_test.go">
<violation number="1" location="e2e/console/control_applicability_helpers_test.go:195">
P3: The helper builds several GraphQL requests by embedding a bare `NODE` placeholder in the query string and splicing in a shared node-selection block with strings.ReplaceAll at runtime. The composed query is never visible verbatim in the source, indentation is lost, and any rename or edit of the marker will silently corrupt the request. Prefer inlining the node selection directly (as querySOAApplicabilityStatements/queryControlRelations already do) or, if deduplication is the goal, use a single shared const query per operation so the actual request text stays readable and statically visible.</violation>
</file>
<file name="pkg/server/api/console/v1/mailing_list_resolvers.go">
<violation number="1" location="pkg/server/api/console/v1/mailing_list_resolvers.go:173">
P2: This change makes the console require the dedicated `mailing-list-update:send` permission for sending an update, but the equivalent MCP tool (schema.resolvers.go:8041) still checks `mailing-list-update:update`. Both surfaces perform the same underlying operation, so the same responsible party may be allowed (or denied) to send depending on which API they use. If the new, dedicated `send` permission is the intended policy, the MCP `SendMailingListUpdateTool` should be aligned to `ActionMailingListUpdateSend` so both APIs enforce the same rule. Consider updating the MCP resolver in the same change to keep the two surfaces consistent.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| } `json:"createApplicabilityStatement"` | ||
| } | ||
|
|
||
| query := replaceApplicabilityStatementNodeSelection(mutation) |
There was a problem hiding this comment.
P3: The helper builds several GraphQL requests by embedding a bare NODE placeholder in the query string and splicing in a shared node-selection block with strings.ReplaceAll at runtime. The composed query is never visible verbatim in the source, indentation is lost, and any rename or edit of the marker will silently corrupt the request. Prefer inlining the node selection directly (as querySOAApplicabilityStatements/queryControlRelations already do) or, if deduplication is the goal, use a single shared const query per operation so the actual request text stays readable and statically visible.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At e2e/console/control_applicability_helpers_test.go, line 195:
<comment>The helper builds several GraphQL requests by embedding a bare `NODE` placeholder in the query string and splicing in a shared node-selection block with strings.ReplaceAll at runtime. The composed query is never visible verbatim in the source, indentation is lost, and any rename or edit of the marker will silently corrupt the request. Prefer inlining the node selection directly (as querySOAApplicabilityStatements/queryControlRelations already do) or, if deduplication is the goal, use a single shared const query per operation so the actual request text stays readable and statically visible.</comment>
<file context>
@@ -0,0 +1,792 @@
+ } `json:"createApplicabilityStatement"`
+ }
+
+ query := replaceApplicabilityStatementNodeSelection(mutation)
+
+ err := client.Execute(query, map[string]any{"input": input}, &result)
</file context>
92d9f0e to
d75fd2d
Compare
Keep product workflows fully end to end while making multi-actor scenarios read like user behavior. Record timed steps and redacted failure artifacts so local and CI failures identify the action that stopped and retain useful context. Improve GraphQL diagnostics and avoid instrumenting only the test driver in the normal E2E run. Application coverage remains available through the dedicated instrumented-server target. Signed-off-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Bryan FRIMIN <bryan@frimin.fr>
Apply the repository's multiline-call and whitespace rules throughout the new journey code. This keeps the reference test suitable for broader suite migration and resolves the lint findings from the first CI run. Signed-off-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Bryan FRIMIN <bryan@frimin.fr>
Create one real owner, admin, and viewer fixture for an organization and reuse it across independent framework permission cases. Declarative role matrices retain coverage while removing seventeen repeated onboarding flows. Signed-off-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Bryan FRIMIN <bryan@frimin.fr>
Define test shapes, fixture isolation rules, migration batches, and measurable acceptance criteria before applying the framework pilot across the full suite. This keeps performance work from weakening black-box coverage. Signed-off-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Bryan FRIMIN <bryan@frimin.fr>
Reuse one real organization role fixture in the measure, document, datum, audit, and processing-activity permission matrices. Bind shallow client copies to each leaf test so parallel setup failures retain precise attribution without repeating signup and invitation flows. Signed-off-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Bryan FRIMIN <bryan@frimin.fr>
Keep all permission cases in a compact resource and operation matrix while giving mutable cases independent fixtures. Parallel leaf execution preserves black-box coverage and removes serial API calls from the core authorization suite. Signed-off-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Bryan FRIMIN <bryan@frimin.fr>
Reuse one organization role fixture across third-party service operations and audit report uploads. Independent resources keep parallel cases isolated while strong forbidden assertions and typed responses preserve diagnostic coverage. Signed-off-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Bryan FRIMIN <bryan@frimin.fr>
Signed-off-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Bryan FRIMIN <bryan@frimin.fr>
Run CI against an instrumented probod process and publish text, profile, and HTML reports. The job summary now exposes total application statement coverage without counting the test driver itself. Signed-off-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Bryan FRIMIN <bryan@frimin.fr>
Separate role matrices and tenant-isolation scenarios from primary resource behavior while retaining one console test package. Concern-based filenames make fixture strategy and coverage easier to navigate without changing any test body or runtime semantics. Signed-off-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Bryan FRIMIN <bryan@frimin.fr>
Exercise subscription lifecycle, role permissions, signing-secret access, tenant isolation, and persisted third-party events through the Console API. Faster webhook polling keeps the worker assertion bounded in the parallel E2E suite. Signed-off-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Bryan FRIMIN <bryan@frimin.fr>
Signed-off-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Bryan FRIMIN <bryan@frimin.fr>
Signed-off-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Bryan FRIMIN <bryan@frimin.fr>
Signed-off-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Bryan FRIMIN <bryan@frimin.fr>
Signed-off-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Bryan FRIMIN <bryan@frimin.fr>
Signed-off-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Bryan FRIMIN <bryan@frimin.fr>
Signed-off-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Bryan FRIMIN <bryan@frimin.fr>
Signed-off-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Bryan FRIMIN <bryan@frimin.fr>
Signed-off-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Bryan FRIMIN <bryan@frimin.fr>
Signed-off-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Bryan FRIMIN <bryan@frimin.fr>
Signed-off-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Bryan FRIMIN <bryan@frimin.fr>
Signed-off-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Bryan FRIMIN <bryan@frimin.fr>
Signed-off-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Bryan FRIMIN <bryan@frimin.fr>
Signed-off-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Bryan FRIMIN <bryan@frimin.fr>
Signed-off-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Bryan FRIMIN <bryan@frimin.fr>
Signed-off-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Bryan FRIMIN <bryan@frimin.fr>
Signed-off-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Bryan FRIMIN <bryan@frimin.fr>
Signed-off-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Bryan FRIMIN <bryan@frimin.fr>
Signed-off-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Bryan FRIMIN <bryan@frimin.fr>
Signed-off-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Bryan FRIMIN <bryan@frimin.fr>
Signed-off-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Bryan FRIMIN <bryan@frimin.fr>
Signed-off-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Bryan FRIMIN <bryan@frimin.fr>
Signed-off-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Bryan FRIMIN <bryan@frimin.fr>
Signed-off-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Bryan FRIMIN <bryan@frimin.fr>
Signed-off-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Bryan FRIMIN <bryan@frimin.fr>
Signed-off-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Bryan FRIMIN <bryan@frimin.fr>
Signed-off-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Bryan FRIMIN <bryan@frimin.fr>
Signed-off-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Bryan FRIMIN <bryan@frimin.fr>
Signed-off-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Bryan FRIMIN <bryan@frimin.fr>
Signed-off-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Bryan FRIMIN <bryan@frimin.fr>
Signed-off-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Bryan FRIMIN <bryan@frimin.fr>
Signed-off-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Bryan FRIMIN <bryan@frimin.fr>
Signed-off-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Bryan FRIMIN <bryan@frimin.fr>
Signed-off-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Bryan FRIMIN <bryan@frimin.fr>
d75fd2d to
09abdcc
Compare
Summary
Testing
make go-fmtgo test -race ./e2e/internal/journey ./e2e/internal/testutilgo vet ./e2e/internal/journey ./e2e/internal/testutilgo test -c -o /tmp/probo-console-e2e.test ./e2e/consolemake -n test-e2eThe live E2E scenario was not run because Docker is unavailable in the agent environment.
Summary by cubic
Adds a readable E2E journey harness with named actors and timed steps, reorganizes console tests by concern, and expands RBAC and tenant-isolation coverage. CI splits fast and coverage runs, race-tests the harness packages, and keeps Trust E2E outside the coverage gate.
Tests
+13044-6440e2e/internal/journey; migrate email-verification flow to named actors with timed steps and redacted failure artifacts.*_rbac_test.goand*_tenant_test.go; removerbac_test.go.owner/admin/vieweractors across RBAC matrices; parallelize leaf cases and isolate mutable fixtures.Agents
+215-2e2e-refactor.mdwith test shapes and isolation rules; extende2e.mdwith journey guidance and fast-path vs coverage flags.make.md(Trust and MCP noted).Other
+90-15test-e2eand instrumentedtest-e2e-coverage; race-tests./e2e/internal/...; coverage runs MCP scenarios, decodes event-stream responses, publishes JUnit and core/per-package coverage, and measures Trust E2E serially (excluded from the coverage gate).GNUmakefileaddsE2E_TEST_FLAGS,E2E_BINARY,E2E_TRUST_JUNIT_FILE, andE2E_CORE_COVER_PKGS; routes tests throughgotestsum..gitignoreignorese2e-artifacts/and E2E coverage files.Written for commit 09abdcc. Summary will update on new commits.