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.
|
|
||
| // Resources from "src/test/resources/" are copied to "resources/" directory in the jar. | ||
| // If static files directory is set, serve from filesystem first | ||
| if (staticFilesDirectory != null) { |
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.
mxschmitt
force-pushed
the
trace-viewer
branch
from
d28e6f8 to
409fa48
Compare
|
|
||
| // Resources from "src/test/resources/" are copied to "resources/" directory in the jar. | ||
| // If static files directory is set, serve from filesystem first | ||
| if (staticFilesDirectory != null) { |
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?
mxschmitt
force-pushed
the
trace-viewer
branch
from
409fa48 to
73dc2d6
Compare
mxschmitt
force-pushed
the
trace-viewer
branch
from
73dc2d6 to
ed032cf
Compare
yury-s
left a comment
yury-s
left a comment
There was a problem hiding this comment.
Looks good, but please revert changes in DriverJar.java
mxschmitt
force-pushed
the
trace-viewer
branch
from
010e782 to
21caa82
Compare
Fixes #1829