Improve Playwright integration #676
Conversation
github-actions
bot
added
theme:api
baev
added
type:improvement
Co-authored-by: Vladimir Semyonov <20096510+vovsemenv@users.noreply.github.com>
There was a problem hiding this comment.
✅ Tested locally. Works as before. @vovsemenv could you please check the PR's revision with your playwright examples?
Left some comments. Good to fix/discuss, but not necessary.
Great job! 👍
| } | ||
|
|
||
| writeCategoriesDefinitions(categories: Category[]): void { | ||
| writeJson("categories.json", "misc", categories); |
There was a problem hiding this comment.
Is this ok, that sort of the writer methods send data through ipc and another ones interacts with fs?
| allureTest.addParameter("Project", project.name); | ||
| } | ||
| if (project.repeatEach > 1) { | ||
| allureTest.addParameter("Repetition", `${test.repeatEachIndex + 1}`); |
There was a problem hiding this comment.
Is it reasonable to create enum with parameters types (like Labels or Statuses)?
| @@ -375,4 +375,16 @@ const appendStep = (parent: ExecutableItemWrapper, step: TestStep) => { | |||
| } | |||
| }; | |||
|
|
|||
| const getStatusDetails = (error: TestError): StatusDetails => { | |||
There was a problem hiding this comment.
I think, we can move the utilty to commons package. Looks pretty useful and we can use it in pair with the utility, which returns status by given error
There was a problem hiding this comment.
TestError is Playwright-specific. But anyway, the rework is planned to support multiple errors
| expect(results.tests).toEqual([ | ||
| expect.objectContaining({ | ||
| fullName: "a.test.ts#should be skipped 1", | ||
| status: "skipped", |
There was a problem hiding this comment.
Let's use Status enum instead of string here and next
| }); | ||
|
|
||
| test("should report stderr", async ({ runInlineTest }) => { | ||
| test.skip(true, "stderr is not reported by playwright for some reason"); |
There was a problem hiding this comment.
What's wrong with the test? Should we keep it skipped?
There was a problem hiding this comment.
I don't know. So I decided to simply ignore it at the moment and deal with that later. This PR is only the first one of many
the parameters (except included) to it.
parent (instead of using stdio)
Context
Checklist