feat(junit): Implement automatic saving of traces and screenshots via fixtures #1560
Conversation
uchagani
changed the title
|
I don't think the failures here are related to the PR. could we try re-running? |
Restarted. But it looks like at least some of them are in the new tests. |
playwright/src/main/java/com/microsoft/playwright/junit/Options.java
Outdated
Show resolved
Hide resolved
playwright/src/main/java/com/microsoft/playwright/impl/junit/BrowserContextExtension.java
Outdated
Show resolved
Hide resolved
playwright/src/main/java/com/microsoft/playwright/impl/junit/BrowserContextExtension.java
Outdated
Show resolved
Hide resolved
playwright/src/main/java/com/microsoft/playwright/impl/junit/BrowserContextExtension.java
Show resolved
Hide resolved
playwright/src/test/java/com/microsoft/playwright/junit/TestTracesAndScreenshots.java
Outdated
Show resolved
Hide resolved
playwright/src/test/java/com/microsoft/playwright/junit/TestScreenshotOn.java
Outdated
Show resolved
Hide resolved
playwright/src/test/java/com/microsoft/playwright/junit/Transient.java
Outdated
Show resolved
Hide resolved
playwright/src/test/java/com/microsoft/playwright/junit/TestTracesAndScreenshots.java
Show resolved
Hide resolved
playwright/src/test/java/com/microsoft/playwright/junit/TestTracesAndScreenshots.java
Outdated
Show resolved
Hide resolved
|
@yury-s the threading issues, I believe, are resulting from tests running via the engine kit and the normal way at the same time. I believe the threading issues are due to use using threadlocal in our extensions. Somewhere, playwright is getting closed by the ExtensionContext but there are still tests running on that thread. We should eventually move away from using threadlocal and store objects in the ExtensionContext (this is what Junit recommends). However, I'm not sure how we can scope to threads while doing that. We can scope to method or class easily but we want to reuse playwright and keep it open across test classes. I don't think users will be affected by this but will verify by running the whole suite except engine kit tests. |
|
@yury-s i believe i have addressed all the comments. I also verified that the threading issues are not user-facing and should not affect users. After this PR, I can start looking into how we can store playwright fixtures in the ExtensionContext and scope them to specific threads |
|
will work on test failures this evening |
Resolves #1526. This PR also adds saving screenshots.
I tried to keep the API of the fixture-generated artifacts as close to node as possible.
Traces:
By default, if no
outputDiris specified in theOptions, the default output path will beprojectDir/test-results/fully.qualified.ClassName.testMethodName-browserName/trace.zip. If anoutputDiris specified in theOptionsthen the artifacts will go inuserSpecifiedOutputDir/fully.qualified.ClassName.testMethodName-browserName/trace.zip.Screenshots:
Screenshots follow the same nomenclature as the traces with the exception that screenshot files are named
test-finished-1.png. Depending on the number of pages aBrowserContexthas, the1will be incremented perPage.I added tests to test this feature. Since traces and screenshots are created after the test is finished, I needed to add a new test dependency (
junit-platform-testkit). This is JUnit's preferred way to test extensions.@Transientannotation was created because the new tests should not be run by themselves and only run through theEngineTestKit. I updated the GitHub actions to exclude these tests.There is some weird threading issues happening when the new tests run with the existing suite so I added a new step in the CI actions to run the new tests separately.