Repository navigation
Playwright - add java sources to a captured Playwright trace - #1450
antonfilichkin wants to merge 2 commits into
Conversation
…spectJ frames from trace stacks Playwright's Tracing.StartOptions#setSources embeds a stack per action, but frame 0 is always synthetic AspectJ-weaving glue inserted by AllurePlaywrightAspect, not the real caller - making Trace Viewer's Source tab useless without this. - Add allure.playwright.trace.sources (default false) to opt into embedding sources in the trace. - Add TraceStackSourceTrimmer, which rewrites the trace.stacks entry in place: it resolves the first file-backed frame in each stack, falling back to stripping the leading run of recognizable AspectJ/Playwright weaving frames when no frame resolves. Fails open on any error, leaving the trace attached unchanged. - Declare the module's new compile-time-only dependency on Gson in module-info.java.
andrcuns
left a comment
There was a problem hiding this comment.
Thanks for the submission @antonfilichkin
My java is very rusty (almost non existent at this point 😅 ) but an agent review found a few issues, could you please check?
| if (!AllurePlaywrightConfig.shouldEmbedTraceSources()) { | ||
| return false; | ||
| } | ||
| if (Objects.isNull(System.getenv(PLAYWRIGHT_JAVA_SRC))) { |
There was a problem hiding this comment.
[P2] Do not disable valid per-instance Playwright source configuration
Playwright also accepts source roots through Playwright.create(new Playwright.CreateOptions().setEnv(Map.of("PLAYWRIGHT_JAVA_SRC", sourceRoot))); the variable does not have to exist in the JVM's process environment. With this supported configuration and allure.playwright.trace.sources=true, this check returns false, so startTracing() sets sources=false and skips trimming.
I reproduced the mismatch against Playwright 1.62.0: its real StackTraceCollector.createFromEnv() accepts the instance configuration, while the Allure helper passes StartOptions.sources=false. The new real-trace test uses this same setEnv() mechanism, but starts tracing directly rather than through AllurePlaywright.startTracing(), so it does not cover this failure.
Please let the opt-in property enable source embedding without requiring System.getenv() to contain the source roots, and cover the per-instance configuration through the public helper.
| ZipEntry entry; | ||
| while ((entry = zis.getNextEntry()) != null) { | ||
| final String name = entry.getName(); | ||
| final byte[] content = readAll(zis); |
There was a problem hiding this comment.
[P2] Stream unchanged entries instead of buffering every decompressed resource
This reads every ZIP entry fully into heap, including screenshots and captured network response bodies, even though only trace.stacks needs transformation. ByteArrayOutputStream grows an unbounded backing array and toByteArray() then allocates another full copy. The resources/ handling below does not skip this allocation.
I reproduced an uncaught OutOfMemoryError in TraceStackSourceTrimmer.readAll() using a 49,237-byte ZIP containing a 48 MiB resource and a valid trace.stacks entry, with a 64 MiB child-JVM heap. Neither this class nor DefaultTraceSession.stop() catches that error, so trace diagnostics can disrupt test execution instead of failing open and attaching the original archive.
Please copy unchanged entries with bounded memory, buffering only the stack JSON. Preserving their original compressed payloads would also avoid unnecessary decompression.
| if (stored) { | ||
| final CRC32 crc = new CRC32(); | ||
| crc.update(content); | ||
| outEntry.setMethod(ZipEntry.STORED); |
There was a problem hiding this comment.
[P2] Preserve compression for text resources
Every resources/ entry reaches this branch, but that directory contains compressible HTML, JavaScript, JSON response bodies, and embedded Java sources, not only already-compressed screenshots. Writing all of them as STORED can substantially increase the trace attachment size.
Running this trimmer on the PR's own trace-before.zip increased the archive from 17,968 to 28,554 bytes (about 59%). Its HTML resource alone expanded from 2,301 compressed bytes to 6,639 stored bytes. Larger or text-heavy traces will incur unnecessary attachment storage, transfer, and loading costs.
Please preserve unchanged entries' compressed payloads, or at least retain compression for compressible resources rather than treating the entire resources/ directory as incompressible binary data.
Context
Allows attaching java sources to Playwright traces.
Allure only configures Playwright traces to include screenshots and snapshots via
Playwright allows including java sources by setting PLAYWRIGHT_JAVA_SRC and configuring tracing via
The problem:
.zipbinaryThe idea is to add configurable option to allow Allure rewrite sources and remove it's own synthetic frames
Implementation idea:
allure.playwright.trace.sources(default false) to opt into embedding sources in the trace.TraceStackSourceTrimmer, which rewrites thetrace.stacksentry in place: it resolves the first file-backed frame in each stack, falling back to stripping the leading run of recognizable AspectJ/Playwright weaving frames when no frame resolves. Fails open on any error, leaving the trace attached unchanged. With some short-circuits to disable zip/json processing - I.e. binary blobs, sources path not set.Trace examples (both with sources)
npx playwright show-trace trace-after.ziptrace-before.zip
trace-after.zip
Checklist