test: introduce global setup #3544
Conversation
| * limitations under the License. | ||
| */ | ||
|
|
||
| matrix({ |
There was a problem hiding this comment.
this is a strange api. It can only be called once, and its mostly used for non-matrixed values.
There was a problem hiding this comment.
Yeah, let's iterate on it.
| @@ -31,10 +31,13 @@ type ItFunction<STATE> = ((name: string, inner: (state: STATE) => Promise<void>) | |||
| repeat(n: number): ItFunction<STATE>; | |||
| }; | |||
|
|
|||
| interface WorkerState { | |||
| interface FixtureParameters { | |||
There was a problem hiding this comment.
Somebody needs to put the matrix options in here.
There was a problem hiding this comment.
Not really, fixtures do that.
test/types.d.ts
Outdated
| } | ||
|
|
||
| interface FixtureState { | ||
| interface WorkerState extends FixtureParameters { |
There was a problem hiding this comment.
Can registerWorkerFixture override fixture parameters? If not, WorkerState shouldn't extend FixtureParameters.
There was a problem hiding this comment.
It can't, but they are defined on the same level. I'll remove inheritance!
| @@ -38,11 +38,9 @@ declare global { | |||
| defaultBrowserOptions: LaunchOptions; | |||
| golden: (path: string) => string; | |||
| playwright: typeof import('../index'); | |||
| browserType: BrowserType<Browser>; | |||
There was a problem hiding this comment.
This one is missing and lint complains.
| @@ -52,6 +50,9 @@ declare global { | |||
| } | |||
| interface FixtureParameters { | |||
| browserName: string; | |||
| headless: boolean; | |||
| wire: boolean; | |||
There was a problem hiding this comment.
Why do we need this? Can we use parameters object instead? Or options?
| @@ -24,7 +24,7 @@ import { Connection } from '../lib/rpc/client/connection'; | |||
| import { Transport } from '../lib/rpc/transport'; | |||
| import { setUnderTest } from '../lib/helper'; | |||
| import { installCoverageHooks } from './coverage'; | |||
| import { parameters, registerFixture, registerWorkerFixture, registerParameter } from './runner'; | |||
| import { parameters, registerFixture, registerWorkerFixture } from './runner'; | |||
There was a problem hiding this comment.
I think parameters and matrix should be named similarly, otherwise it's not clear parameters come from matrix.
There was a problem hiding this comment.
They aren't in any of the CIs, so let's follow up.
| matrix({ | ||
| 'browserName': process.env.BROWSER ? [process.env.BROWSER] : ['chromium', 'webkit', 'firefox'], | ||
| 'headless': [!!valueFromEnv('HEADLESS', true)], | ||
| 'wire': [!!process.env.PWWIRE], |
There was a problem hiding this comment.
Why don't we make this into actual parameter?
process.env.PWWIRE ? [true] : [true, false]
There was a problem hiding this comment.
Well, because you don't want to run both by default?
pavelfeldman
force-pushed
the
test25
branch
2 times, most recently
from
629186e
to
bdf4830
Compare
No description provided.