feat(junit): Implement automatic saving of traces and screenshots via fixtures

[uchagani]

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 outputDir is specified in the Options, the default output path will be projectDir/test-results/fully.qualified.ClassName.testMethodName-browserName/trace.zip. If an outputDir is specified in the Options then the artifacts will go in userSpecifiedOutputDir/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 a BrowserContext has, the 1 will be incremented per Page.

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.

@Transient annotation was created because the new tests should not be run by themselves and only run through the EngineTestKit. 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.

[uchagani]

I don't think the failures here are related to the PR. could we try re-running?

[yury-s]

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.

[uchagani]

@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.

[uchagani]

@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

[uchagani]

will work on test failures this evening

[uchagani]

I'm sorry for the delay life got in the way recently. Hopefully will get to this this weekend

[yury-s]

No problem at all, hope everything is alright.

[uchagani]

@yury-s was wondering if it's okay to land this feature without tests. The JUnit Engine Test Kit isn't playing well. I have confirmed that the issue is not in the feature but how the engine test kit runs the tests.

[yury-s]

@yury-s was wondering if it's okay to land this feature without tests. The JUnit Engine Test Kit isn't playing well. I have confirmed that the issue is not in the feature but how the engine test kit runs the tests.

Yes, if it proves too difficult to test it, we can land it without tests. Would be nice to have some description of the problem here, so that later on if we wonder why it's difficult to write such tests we wouldn't need to repeat all the debugging and investigation that you've already done.

[uchagani]

@yury-s was wondering if it's okay to land this feature without tests. The JUnit Engine Test Kit isn't playing well. I have confirmed that the issue is not in the feature but how the engine test kit runs the tests.

Yes, if it proves too difficult to test it, we can land it without tests. Would be nice to have some description of the problem here, so that later on if we wonder why it's difficult to write such tests we wouldn't need to repeat all the debugging and investigation that you've already done.

The main problem that I was seeing was that, when running trace tests through the engine test kit, the Playwright object was getting closed even though the tests were running. Some how I think the thread the engine test kit runs the test on and the actual @Test annotated tests are conflicting in some way. I think it may be a bug in the engine test kit and the usage of ExtensionContext.Store.CloseableResource

I need to create a simpler example of this and post it to the JUnit github page to see if I can get some help troubleshooting it.

[uchagani]

looking into failures

[uchagani]

@yury-s All tests should be passing now