feat: add ability to copy playwright config into checkly config #919
Conversation
Once creating the CLI it will check if there is a playwright.config file on the directory, then ask the user if he wants to copy it to the checkly config file
…mmand New feature that allows the user to sync (update or create) their playwright config with the checkly config
|
This pull request has been linked to Shortcut Story #18663: As a user, I would like my PW config to sync with the checkly config. |
ferrandiaz
force-pushed
the
ferran/sc-18663/playwright-checkly-config
branch
from
27bafa2
to
466c6b0
Compare
| return prompts({ | ||
| type: 'confirm', | ||
| name: 'shouldCopyPlaywrightConfig', | ||
| message: 'It looks like you have a playwright config, do you want to copy it?', |
There was a problem hiding this comment.
I think "copy it" is slightly vague. Maybe we can rephrase it a bit?
| message: 'It looks like you have a playwright config, do you want to copy it?', | |
| message: 'It looks like you have a Playwright config file. Would you like to copy your settings to your Checkly config file?', |
|
|
||
| export default class SyncPlaywright extends BaseCommand { | ||
| static hidden = true | ||
| static description = 'Sync playwright config' |
There was a problem hiding this comment.
I think Playwright and Checkly should be capitalized in the user facing messages.
Maybe we can make the help description more specific too in case users aren't familiar with the command. Maybe something like this?
| static description = 'Sync playwright config' | |
| static description = 'Copy Playwright config into the Checkly config file' |
| static description = 'Sync playwright config' | ||
|
|
||
| async run (): Promise<void> { | ||
| ux.action.start('Sync playwright config with checkly config', undefined, { stdout: true }) |
There was a problem hiding this comment.
| ux.action.start('Sync playwright config with checkly config', undefined, { stdout: true }) | |
| ux.action.start('Syncing Playwright config to the Checkly config file', undefined, { stdout: true }) |
There was a problem hiding this comment.
@clample Syncing or Synching? 🤔
There was a problem hiding this comment.
Hmm, I think Syncing is good. Apparently some British folks use Synching though. https://english.stackexchange.com/questions/681/synced-or-synched
|
|
||
| const checksAst = this.findPropertyByName(checklyAst, 'checks') | ||
| if (!checksAst) { | ||
| return this.handleError('Could not parse your checkly config file') |
There was a problem hiding this comment.
I think that users will assume that either their config is invalid or the sync-playwright command is broken, and get confused / try to contact support.
I think that we could make it more clear that this can happen in some cases and there's no need to worry. For example, maybe they're importing the config from another file or using a helper function. Maybe something like this?
| return this.handleError('Could not parse your checkly config file') | |
| return this.handleError('Unable to automatically sync your config file. This can happen if your Checkly config is built using helper functions or other JS/TS features. You can still manually set Playwright config values in your Checkly config: https://www.checklyhq.com/docs/cli/constructs-reference/#project') |
| this.reWriteChecklyConfigFile(checklyConfigData, configFile.fileName, dir) | ||
|
|
||
| ux.action.stop('✅ ') | ||
| this.log('Successfully sync') |
There was a problem hiding this comment.
| this.log('Successfully sync') | |
| this.log('Successfully updated Checkly config file') |
There was a problem hiding this comment.
@clample "...updated the Checkly config file"
|
Looks good to me 👍 I think that it could make sense to double check / tweak some of the user facing messages, though. |
I hereby confirm that I followed the code guidelines found at engineering guidelines
Affected Components
Notes for the Reviewer
Since create-cli and cli are 2 different workspaces I had to duplicate quite some code, if this extends more it would probably be wise to add a shared workspace or package between the 2 for the playwright config.
PR is separated in 2 commits.
checkly sync-playwrightcurrently hidden, if passed adds or updates the playwright config of the checkly config.