Added specType to the E2E test reports - #11
Conversation
added e2e test heatmap and coverage dashboard
E2e report ingestion
replaced logs with slog and added check in e2e migration file
removed duplicate table drop query
moved check constraint from the table
E2e spec type
E2e spec type
|
Warning Review limit reached
More reviews will be available in 58 minutes and 57 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?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 credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. 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, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughAdds a ChangesspecType end-to-end feature
Sequence Diagram(s)sequenceDiagram
participant Browser as Browser (e2eTest.html + e2e.js)
participant Handler as HTTP Handler (handlers.go)
participant UseCase as ListE2ERunsUseCase / GetE2EHeatmapUseCase
participant Repo as E2ETestRunRepository (Postgres)
participant DB as e2e_test_spec_results (spec_type column)
Browser->>Handler: GET /e2e/runs?specType=happyPath
Handler->>UseCase: Execute(ListE2ERunsInput{SpecType: "happyPath"})
UseCase->>UseCase: validate specType ∈ validSpecTypes
UseCase->>Repo: ListByProject(..., specType="happyPath", ...)
Repo->>DB: SELECT with EXISTS subquery on spec_type
DB-->>Repo: filtered run rows
Repo-->>UseCase: []E2ETestRun
UseCase-->>Handler: run list + failed specs (SpecType in FailedSpecResponse)
Handler-->>Browser: JSON response
Browser->>Browser: render specType column in failed-specs table
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (1)
internal/application/e2e_usecase.go (1)
122-127: ⚡ Quick winReplace package-level mutable
validSpecTypeswith an immutable validator helper.This introduces avoidable shared mutable state in the application package.
Refactor sketch
-var validSpecTypes = map[string]bool{ - "": true, - "happyPath": true, - "negativePath": true, - "setup": true, -} +func isValidSpecType(v string) bool { + switch v { + case "", "happyPath", "negativePath", "setup": + return true + default: + return false + } +}As per coding guidelines, "Avoid package-level mutable state."
🤖 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 `@internal/application/e2e_usecase.go` around lines 122 - 127, Replace the package-level mutable `validSpecTypes` map with a validator helper function that performs the same validation without introducing shared mutable state. Create a function that takes a spec type string as input and returns a boolean indicating whether it's valid (accepting empty string, "happyPath", "negativePath", and "setup"), then update all code locations that currently reference `validSpecTypes` to call this new validator helper instead of accessing the map directly.Source: Coding guidelines
🤖 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 `@cmd/coveragecli/main.go`:
- Around line 677-688: The switch statement that determines spec_type currently
falls back to using projectID in the default case, but projectID values are not
guaranteed to be valid spec types and can break the ingest process. Remove or
modify the default case in the switch statement (currently assigning spec_type =
projectID) so that it does not use projectID as a fallback value. Either remove
the default case entirely to leave spec_type unchanged when none of the
file-path patterns match (setup, happyPath, negativePath), or assign it to a
known valid default spec type instead of projectID.
In `@cmd/frontend/web/assets/e2e.js`:
- Around line 714-715: Multiple placeholder rows displayed in the E2E results
table are using colspan="4" when the table now has 5 columns, causing
misalignment. Find all placeholder HTML strings in the e2e.js file that render
messages like "No run selected" and "No failed specs" (including instances
around line 812), and update any colspan="4" attributes to colspan="5" to match
the corrected colspan="5" shown in the diff at line 714. Ensure all table
placeholders have consistent colspan="5" across all states.
In `@cmd/frontend/web/e2eTest.html`:
- Around line 109-113: Add a new option with value "setup" to both spec-type
filter dropdowns. The e2eSpecTypeFilter select element and the second spec-type
filter dropdown (noted as also applying to lines 190-194) both need to include
an option element with value="setup" and text content "setup" to allow users to
filter setup runs/specs from the UI. Insert this option between the existing
"All spec types" header option and the "happyPath" option in both select
elements.
In `@internal/application/e2e_usecase_test.go`:
- Around line 623-629: The validation test table for specType in the
e2e_usecase_test.go file is missing a test case for the setup specType, which is
a valid value according to the use-case contract. Add a new test case entry to
the table following the same structure as the existing empty specType case. This
new case should set SpecType to "setup" in the mutate function and have wantErr
set to false to ensure regressions for setup specType values are caught in
future changes.
In `@internal/application/e2e_usecase.go`:
- Around line 388-390: The validation of spec.SpecType against validSpecTypes is
checking the raw input without trimming whitespace, causing valid values with
leading or trailing spaces like " happyPath " to be rejected. Before the
validation check in the if condition that references
validSpecTypes[spec.SpecType], trim the spec.SpecType value to remove any
leading or trailing whitespace (use strings.TrimSpace or equivalent), then
update the validation check to use the trimmed value when looking it up in the
validSpecTypes map. This ensures that semantically valid spec type values are
accepted regardless of whitespace.
In `@migrations/004_add_spec_type.sql`:
- Line 4: The CHECK constraint in the spec_type column definition only allows
'setup', 'happyPath', and 'negativePath', but the ingest validation currently
accepts empty strings as valid specType values, creating a mismatch where
validation passes but database insertion fails. Decide whether empty strings
should be allowed: if yes, add an empty string option to the CHECK constraint
list (modify the ADD COLUMN statement to include '' in the allowed values); if
no, update the ingest validation logic that currently accepts empty specType to
reject it instead, ensuring both sides of the contract are aligned.
---
Nitpick comments:
In `@internal/application/e2e_usecase.go`:
- Around line 122-127: Replace the package-level mutable `validSpecTypes` map
with a validator helper function that performs the same validation without
introducing shared mutable state. Create a function that takes a spec type
string as input and returns a boolean indicating whether it's valid (accepting
empty string, "happyPath", "negativePath", and "setup"), then update all code
locations that currently reference `validSpecTypes` to call this new validator
helper instead of accessing the map directly.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 0284b9c0-950d-4188-b376-f47983f7fa71
📒 Files selected for processing (13)
cmd/coveragecli/main.gocmd/frontend/web/assets/e2e.jscmd/frontend/web/e2eTest.htmlinternal/adapters/http/handlers.gointernal/adapters/postgres/e2e_spec_result_repository.gointernal/adapters/postgres/e2e_test_run_repository.gointernal/application/e2e_usecase.gointernal/application/e2e_usecase_test.gointernal/application/integration_usecase.gointernal/application/mock_application.gointernal/application/ports.gointernal/domain/e2e.gomigrations/004_add_spec_type.sql
resolved pr comment - refactored spec type normalization and frontend
* added e2e heatmap and coverage dashboard * removed unused imports * added unit tests * fixed typo errors * replaced the dummy playwright report with minimal fields for test * replaced logs with slog and added check in e2e migration file * removed duplicate table drop query * moved check constraint from the table * added spec type to e2e tests report * added spec type filter in the dashboard and heatmap * updated the success ratio * refactor to save the spec type * updated failed spec query * resolved pr comment - refactored spec type normalization and frontend --------- Co-authored-by: rsultana1418 <rsultana@lodgelink.com>
Summary by CodeRabbit