-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ref(e2e-tests): Make startCommand
optional in shared playwright config
#12842
Conversation
@@ -1,79 +1,8 @@ | |||
import type { PlaywrightTestConfig } from '@playwright/test'; | |||
import { devices } from '@playwright/test'; | |||
import { getPlaywrightConfig } from '@sentry-internal/test-utils'; | |||
|
|||
// Fix urls not resolving to localhost on Node v17+ | |||
// See: https://github.com/axios/axios/issues/3821#issuecomment-1413727575 | |||
import { setDefaultResultOrder } from 'dns'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can also get rid of these lines here!
@@ -5,23 +5,4 @@ import { getPlaywrightConfig } from '@sentry-internal/test-utils'; | |||
import { setDefaultResultOrder } from 'dns'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can get rid of this too!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
even better!
size-limit report 📦
|
Small refactor:
Not all our e2e tests require a
startCommand
as we sometimes directly invoke an application from within tests. For example in our AWS lambda e2e tests. This PR makes thestartCommand
option optional so that we can avoid a larger override of the entirewebServer
playwright config object.