internal(browser): Create test from recording#1170
internal(browser): Create test from recording#1170going-confetti wants to merge 6 commits intomainfrom
Conversation
4079210 to
d935b37
Compare
|
|
||
| const locatorOptions = { locator: LocatorOptionsSchema } | ||
|
|
||
| const BrowserTestActionSchema = z.discriminatedUnion('method', [ |
There was a problem hiding this comment.
I think having two different schemas might lead to confusion. Maybe we could have %PREFIX%RunnerActionSchema and %PREFIX%TestActionSchema?
…er-test-from-recording
| .map(([type, label]) => ( | ||
| <RadioGroup.Item value={type} key={type}> | ||
| {label} | ||
| <Text |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Empty optional selectors picked over valid CSS fallback
- Added shared non-empty string checks for label/alt/placeholder/title (and testId) in both locator value construction and locator-type selection so empty optional selectors now fall back to CSS.
Or push these changes by commenting:
@cursor push 6f89a95a1a
Preview (6f89a95a1a)
diff --git a/src/codegen/browser/convertEventsToActions.ts b/src/codegen/browser/convertEventsToActions.ts
--- a/src/codegen/browser/convertEventsToActions.ts
+++ b/src/codegen/browser/convertEventsToActions.ts
@@ -6,31 +6,35 @@
} from '@/schemas/recording'
import { exhaustive } from '@/utils/typescript'
+function hasNonEmptyValue(value: string | undefined): value is string {
+ return value !== undefined && value.trim() !== ''
+}
+
function toLocatorOptions(selector: ElementSelector): LocatorOptions {
const values: LocatorOptions['values'] = {}
values.css = { type: 'css', selector: selector.css }
- if (selector.testId !== undefined && selector.testId.trim() !== '') {
+ if (hasNonEmptyValue(selector.testId)) {
values.testid = { type: 'testid', testId: selector.testId }
}
- if (selector.alt !== undefined) {
+ if (hasNonEmptyValue(selector.alt)) {
values.alt = { type: 'alt', text: selector.alt }
}
- if (selector.label !== undefined) {
+ if (hasNonEmptyValue(selector.label)) {
values.label = { type: 'label', label: selector.label }
}
- if (selector.placeholder !== undefined) {
+ if (hasNonEmptyValue(selector.placeholder)) {
values.placeholder = {
type: 'placeholder',
placeholder: selector.placeholder,
}
}
- if (selector.title !== undefined) {
+ if (hasNonEmptyValue(selector.title)) {
values.title = { type: 'title', title: selector.title }
}
@@ -53,12 +57,11 @@
selector: ElementSelector
): LocatorOptions['current'] {
if (selector.role !== undefined) return 'role'
- if (selector.label !== undefined) return 'label'
- if (selector.alt !== undefined) return 'alt'
- if (selector.placeholder !== undefined) return 'placeholder'
- if (selector.title !== undefined) return 'title'
- if (selector.testId !== undefined && selector.testId.trim() !== '')
- return 'testid'
+ if (hasNonEmptyValue(selector.label)) return 'label'
+ if (hasNonEmptyValue(selector.alt)) return 'alt'
+ if (hasNonEmptyValue(selector.placeholder)) return 'placeholder'
+ if (hasNonEmptyValue(selector.title)) return 'title'
+ if (hasNonEmptyValue(selector.testId)) return 'testid'
return 'css'
}You can send follow-ups to the cloud agent here.
Reviewed by Cursor Bugbot for commit d127928. Configure here.
| if (selector.testId !== undefined && selector.testId.trim() !== '') | ||
| return 'testid' | ||
| return 'css' | ||
| } |
There was a problem hiding this comment.
Empty optional selectors picked over valid CSS fallback
Medium Severity
pickBestLocatorType checks !== undefined for label, alt, placeholder, and title, but unlike testId, it doesn't guard against empty strings. If a recorded element has e.g. label: "", pickBestLocatorType returns 'label' over a valid css selector. The matching toLocatorOptions also adds these empty values. This results in the generated action defaulting to an empty, invalid locator when a usable CSS fallback exists. The testId field correctly applies selector.testId.trim() !== '', but the other optional fields lack the same guard.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit d127928. Configure here.




Description
Browser testoption to theCreate testmenu (hidden behind a feature toggle) - when selected, the browser events are converted into browser actions that are then saved to a.k6bfileNote
Assertions are not supported yet. Also some of the actions (most notably,
Click) don't yet support the options that can be recorded in their event counterparts.How to Test
Important
If you have any browser tests you created before, delete them as they will no longer work
For testing, just try creating tests out of recordings with all kinds of browser events.
Checklist
Related PR(s)/Issue(s)
Note
Medium Risk
Changes the persisted browser test schema and creation IPC contract, which can break existing
.k6bfiles and requires correct migration/compat handling. Also introduces new event→action conversion logic that may miss/approximate some recorded options.Overview
Adds a new (feature-flagged) “Browser test” create flow from the recording preview: recorded
BrowserEvents are converted toBrowserTestActions, saved into a new.k6bfile, and the editor navigates directly to the created test.Updates the browser test file format so locator-based actions store
LocatorOptions(current locator + all configured locator variants) and refactors the editor to use these schema types directly (simplifying action adapters and save/dirty tracking). The locator picker UI now visually distinguishes configured locator types from empty ones.Reviewed by Cursor Bugbot for commit d127928. Bugbot is set up for automated code reviews on this repo. Configure here.