chore: migrate Trace Viewer tests to use real Trace viewer#1830
chore: migrate Trace Viewer tests to use real Trace viewer#1830
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR migrates Trace Viewer tests from a JSON-based approach to using the actual Trace Viewer web application for validation. The change improves test coverage by testing the real UI components instead of just parsing trace files.
- Replaced JSON parsing and verification of trace events with actual Trace Viewer UI interactions
- Added a new
TraceViewerPagehelper class to encapsulate Trace Viewer UI operations - Enhanced the test server to serve static files from the filesystem to support hosting the Trace Viewer
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| TestTracing.java | Migrated all trace verification logic from JSON parsing to Trace Viewer UI testing, added TraceViewerPage helper class |
| Server.java | Added static file serving capability to support hosting Trace Viewer from filesystem |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
playwright/src/test/java/com/microsoft/playwright/TestTracing.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
underneath we do something similar but via getResourceAsStream which got introduced here - do you remember why?
There was a problem hiding this comment.
This is the standard way to read resources in a maven package (they move from the source into the test package artifact). I'd keep using it with the custom base dir too, can we do that?
There was a problem hiding this comment.
I think there is no way of accessing the trace-viewer html source code via the tests/resource directory so I introduced another resourceLoader concept - are you fine with that?
There was a problem hiding this comment.
Yes. I'd pass the resource provide in server the constructor.
d28e6f8 to
409fa48
Compare
There was a problem hiding this comment.
This is the standard way to read resources in a maven package (they move from the source into the test package artifact). I'd keep using it with the custom base dir too, can we do that?
playwright/src/test/java/com/microsoft/playwright/TestTracing.java
Outdated
Show resolved
Hide resolved
409fa48 to
73dc2d6
Compare
73dc2d6 to
ed032cf
Compare
yury-s
left a comment
There was a problem hiding this comment.
Looks good, but please revert changes in DriverJar.java
driver-bundle/src/main/java/com/microsoft/playwright/impl/driver/jar/DriverJar.java
Outdated
Show resolved
Hide resolved
driver-bundle/src/main/java/com/microsoft/playwright/impl/driver/jar/DriverJar.java
Show resolved
Hide resolved
playwright/src/test/java/com/microsoft/playwright/TraceViewerFixture.java
Outdated
Show resolved
Hide resolved
playwright/src/test/java/com/microsoft/playwright/TraceViewerPage.java
Outdated
Show resolved
Hide resolved
010e782 to
21caa82
Compare
Fixes #1829