Increase Playwright timeouts, robustify YAML editor interaction #422
Conversation
|
Example error messages and URLs to failing jobs:
|
| @@ -10,7 +10,7 @@ | |||
| import random | |||
|
|
|||
|
|
|||
| DEFAULT_TIMEOUT = 30_000 # time in ms | |||
| DEFAULT_TIMEOUT = 60_000 # time in ms | |||
There was a problem hiding this comment.
I increased this value because I saw "Timeout 30000ms exceeded" in the GitHub Action logs
| @@ -120,7 +120,7 @@ def _create_new_environment(page, screenshot=False): | |||
|
|
|||
|
|
|||
| def _existing_environment_interactions( | |||
| page, env_name, time_to_build_env=3 * 60 * 1000, screenshot=False | |||
| page, env_name, time_to_build_env=5 * 60 * 1000, screenshot=False | |||
There was a problem hiding this comment.
I increased this value because I also saw "Timeout 180000ms exceeded" in the GitHub Action logs
|
I can see new errors now - something like buttons not being enabled so perhaps the button behaviour is now different |
gabalafou
changed the title
| @@ -27,7 +27,7 @@ export const CodeEditor = ({ code, onChangeEditor }: ICodeEditor) => { | |||
| const convertToJSON = (e: string) => { | |||
| try { | |||
| setIsError(false); | |||
| onChangeEditor(parse(e)); | |||
| onChangeEditor(parse(e) || {}); | |||
There was a problem hiding this comment.
This is a lazy fix to prevent a runtime error that occurs if you clear the YAML editor. But a more rigorous fix would take a lot more time because the editor right now just doesn't have good validation.
| @@ -52,6 +52,7 @@ export const CodeEditor = ({ code, onChangeEditor }: ICodeEditor) => { | |||
| theme={isGrayscaleStyleType ? undefined : greenAccentTheme} | |||
| extensions={[StreamLanguage.define(yamlLanguage)]} | |||
| onChange={e => convertToJSON(e)} | |||
| data-testid="yaml-editor" | |||
There was a problem hiding this comment.
Should have done this a long time ago since it allows us to avoid the text matching (page.get_by_text) that you can find in the diff on the left in the Playwright test file. But I wonder if this test id data attribute should be "yaml-editor-container" or maybe "code-mirror-container" because, as you might notice in the changes to the Playwright test file in this PR, the locator for the actual textbox field looks like this:
page.get_by_test_id("yaml-editor").get_by_role("textbox")That's because the HTML structure of the CodeMirror React component is like this:
<div data-testid="yaml-editor">
<!-- stuff -->
<div contenteditable>
<!-- more stuff -->
Hoping this will fix failing jobs on GitHub.
Ideally, we would have shorter timeouts for local dev and longer timeouts for CI, but I'm not sure how to do that off the top of my head.