RI-8190 E2E coverage for prod vs non-prod modes#5984
Merged
Conversation
Add Playwright E2E coverage for the per-database environment classification (Production / Development / Unspecified) and the type-to-confirm modal flow, all gated on the dev-prodMode feature flag. Specs under tests/e2e-playwright/tests/main/databases/environment/ cover: - PROD badge / DEV label rendering in DB list + instance header - Browser write confirmations (no-input variant per RI-8201) - Bulk delete (still requires typing the DB name) - CLI FLUSHDB and Workbench dangerous batches (new title + ACL tip) - Profiler confirmation popover + always-on advisory - Tutorial Run button disabled on Production Infrastructure: new TypeToConfirmModal page object + fixture, Environment enum in e2e types, optional environment in createDatabase API helper, selectEnvironment helper on AddDatabaseDialog. TEST_PLAN.md updated with section 1.9. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
🛡️ Jit Security Scan Results
✅ No security findings were detected in this PR
Security scan by Jit
|
The pre-commit hook is misconfigured for worktree commits (it runs lint-staged against the main checkout's tests/e2e-playwright via core.hooksPath), so prettier didn't normalize these files locally and CI's format:check rejected them. No behavioural changes — pure whitespace. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The PROD/DEV badge introduced by RI-8181 is rendered inside the same `td:nth-child(2)` cell as the database alias, so the cell's full text becomes "PROD <alias>" (or "DEV <alias>"). The previous getRow regex was anchored at start and end of the cell text — once a badge appeared the filter no longer matched, and tests that exercised expectDatabaseVisible on a Production/Development DB timed out. Switch the locator from `td:nth-child(2)` (cell-level text) to `td:nth-child(2) p` (the database-alias paragraph) so the anchored regex applies only to the name paragraph, regardless of any sibling badge paragraph in the same cell. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
KrumTy
commented
May 29, 2026
|
|
||
| constructor(page: Page) { | ||
| this.page = page; | ||
| this.title = page.getByTestId('type-to-confirm-modal-title'); |
Contributor
Author
There was a problem hiding this comment.
it's a generic, reusable modal, that's used with various props based on case; hence it's easier to manage it via test ids in e2e
…PLAN entries
Reshape per review feedback:
* Replace dedicated "1.9 Environment Modes" section in TEST_PLAN with
entries distributed across existing feature sections (1.2 Database
List, 2.4 Key Details - String, 2.11 Bulk Actions, 3.1 Command
Execution, 3.3 Tutorials, 3.4 Profiler, 4.1 CLI Panel).
* Use `test.describe('Production DB' | 'Development DB' | 'Unspecified DB')`
groups; tests start with `should ...` and don't repeat the environment
in the test name.
* Consolidate 14 -> 9 tests, dropping coverage already exercised by unit
tests: per-key-type Browser write variants (TTL, hash) since the
rename test already proves the provider+modal+useProductionWriteConfirmation
wiring; modal copy assertions (title, ACL tip) which live in the
modal's own spec; "mistyped name keeps Confirm disabled" since that's
modal-internal logic; Unspecified-only variants of Profiler/Tutorials
which are just the default no-op path.
* Split profiler-tutorials.spec.ts into profiler.spec.ts and
tutorials.spec.ts so each spec aligns 1:1 with its TEST_PLAN section.
* Strip ticket-id references from comments, test names, page-object
docstrings, and TEST_PLAN rows.
* Simplify AddDatabaseDialog.selectEnvironment to derive the option
label directly from the Environment enum value.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
… areas Move each spec out of tests/main/databases/environment/ and into the folder of the feature it exercises, matching the new TEST_PLAN distribution: databases/list/environment-badge.spec.ts (was connection-form) browser/key-details/environment-gating.spec.ts (was browser-confirmations) browser/bulk-actions/environment-gating.spec.ts (was bulk-delete) cli/dangerous-commands.spec.ts (was cli-dangerous) workbench/dangerous-commands.spec.ts (was workbench-dangerous) workbench/profiler.spec.ts (was profiler) insights/tutorials.spec.ts (was tutorials) Pure file moves; no content changes. Removes the now-empty environment/ directory. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit a0e8be7. Configure here.
…ped DB name Address PR feedback: exercise the newly-defined `TypeToConfirmModal.expectConfirmDisabledWhen` helper inside the bulk-delete spec, where the typed-input variant of the modal lives. The extra assertion proves the integration end-to-end (form value → modal state → Confirm button enable/disable), complementing the unit-test coverage on the modal in isolation. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
valkirilov
reviewed
May 29, 2026
| | 🔲 | main | Confirm deletion failures surfaced in summary log | | ||
| | 🔲 | main | Confirm performance when deleting thousands of keys | | ||
| | 🔲 | main | Confirm performance when bulk uploading large datasets (>10K keys) | | ||
| | ✅ | main | Production DB > should require typing the database name to bulk-delete | |
Member
There was a problem hiding this comment.
note: Make sure to run this test in an isolated database instance, so it doesn’t conflict with other tests that depend on the same data, which will be deleted.
Contributor
Author
There was a problem hiding this comment.
it creates keys with a keyPrefix, then bulk deletes only them
lets see if this ever collides with another test first?
If we have a test that deliberately wants to read/mutate all data in the database and does not want to rely on predixes, then maybe we can spin an instance for this test alone
Member
There was a problem hiding this comment.
I think I saw the FLUSHALL command, and it can cause flakiness with other tests (because they run in parallel). At least, it was happening with some of the test suites previously, but if it's not the case, then okay 👌
| await browserPage.goto(database.id); | ||
| await browserPage.openProfiler(); | ||
|
|
||
| await page.getByTestId('start-monitor').click(); |
Member
There was a problem hiding this comment.
nit: is there a way to use a better visual selector, instead of the IDs
| | 🔲 | main | History limited to 30 commands (oldest replaced by newest) | | ||
| | 🔲 | main | Quick-access to command history with Up Arrow | | ||
| | 🔲 | main | Use Non-Redis Editor with Shift+Space | | ||
| | ✅ | main | Production DB > should require type-to-confirm modal for dangerous workbench batches | |
Member
There was a problem hiding this comment.
note: same as https://github.com/redis/RedisInsight/pull/5984/changes#r3324636891, since the test contains FLUSHALL in the list of dangerous commands we execute.
…modal descriptions Address review feedback: * Extract a ProfilerPanel page object (pages/profiler/ProfilerPanel.ts) exposing Start / Stop controls, the production confirmation popover, the advisory banner, and the visible "Profiler is started." indicator. Locators prefer accessible names (getByRole / getByText); testids are retained only where the element has no stable semantic anchor (the popover buttons and the advisory banner). * Wire ProfilerPanel as a Playwright fixture. * Replace direct page.getByTestId(...) calls in profiler.spec.ts with the new page object, and extend the test to verify that confirming the production popover actually starts the profiler — not just that cancelling closes the popover. * In cli/dangerous-commands.spec.ts and workbench/dangerous-commands.spec.ts assert that the type-to-confirm modal description lists the dangerous command (FLUSHDB) and the target database name, matching the copy rendered by CliBodyWrapper / WBViewWrapper. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…rial The CLI and Workbench dangerous-command specs both confirm `FLUSHDB` to verify the production write-confirmation flow end-to-end. Pointed at the shared standalone Redis (port 8100), that wipe would clobber every other parallel spec running concurrently on the same instance. Switch both to `StandaloneEmptyConfigFactory` (dedicated port 8105 Redis) and tag the describes `@serial` so they execute in the chromium-serial project — single-worker, no concurrent file — alongside the existing vector-search specs that already share this pattern. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
valkirilov
approved these changes
Jun 1, 2026
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
2 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.
What
Adds Playwright E2E coverage for prod vs non-prod modes. All scenarios run with
dev-prodModeenabled viatest.use({ featureFlags: ... }).Specs are co-located with the feature area each scenario exercises:
databases/list/environment-badge.spec.ts— Production / Development / Unspecified DBs render the right badge (or no badge) in the list and the instance header.browser/key-details/environment-gating.spec.ts— Browser writes on a Production DB open the modal (no typing needed); writes on a Development DB bypass it entirely (per-connection gating).browser/bulk-actions/environment-gating.spec.ts— Bulk delete on a Production DB keeps the typed-input variant; mistyped name keeps Confirm disabled.cli/dangerous-commands.spec.ts—FLUSHDBon a Production DB opens the modal; cancel preserves keys, confirm runs (verified against the real Redis backend).workbench/dangerous-commands.spec.ts— Workbench batch containing a dangerous command opens the modal mid-batch; same cancel/confirm contract.workbench/profiler.spec.ts— Starting the Profiler on a Production DB requires a confirmation popover.insights/tutorials.spec.ts— Tutorial Run buttons are disabled on a Production DB.Modal copy, per-component wiring, and per-action confirmation strings are covered by unit tests and were deliberately not duplicated here.
Infrastructure
TypeToConfirmModalpage object + fixture exposing bothconfirm()(input variant) andconfirmWithoutInput()(Browser variant).Environmentenum ine2eSrc/types; optionalenvironmentflows throughApiHelper.createDatabase.selectEnvironment()helper onAddDatabaseDialog;fillFormhonours the new field.DatabaseList.getRowswitched from a cell-level locator to the inner name<p>so the environment badge in the same cell doesn't break the lookup.Testing
@e2e-testslabel).yarn dev:api+yarn dev:ui, thencd tests/e2e-playwright && npx playwright test --project=chromium tests/main/browser/bulk-actions tests/main/browser/key-details tests/main/cli tests/main/workbench tests/main/insights/tutorials.spec.ts tests/main/databases/list/environment-badge.spec.tsagainst the docker-compose RTE.Note
Low Risk
Changes are limited to E2E tests, fixtures, and TEST_PLAN; no application runtime logic is modified in this diff.
Overview
Adds Playwright E2E coverage for prod vs non-prod behavior with
dev-prodModeenabled, plus shared test infrastructure so scenarios can set Production / Development / Unspecified on connections and drive the UI consistently.New specs exercise PROD/DEV badges in the database list and instance header, type-to-confirm on production for browser writes (rename vs dev bypass on edit), bulk delete (typed DB name), CLI and Workbench dangerous commands (
FLUSHDB, cancel/confirm against Redis), profiler start confirmation, and disabled tutorial Run on production.Supporting changes:
TypeToConfirmModalandProfilerPanelpage objects wired through fixtures;Environmenton API/UI helpers;DatabaseList.getRowtargets the name cell’s inner<p>so badges don’t break row lookup; TEST_PLAN marks the new cases as implemented.Reviewed by Cursor Bugbot for commit 8f71359. Bugbot is set up for automated code reviews on this repo. Configure here.