Show inline session media (thumbnail + expandable) on the build page#31
Merged
Conversation
The build summary and the TestingBot embed page listed each session as a plain text link. Each session now shows a preview thumbnail and expands in place (native details/summary) to the full mini view (video, screenshots, logs). TestingBotBuildObject gains a thumbUrl derived from the session's first available thumbnail at build completion (Strings only, JEP-200-safe; no extra network I/O since the test details are already fetched). The embedded iframe uses loading=lazy so collapsed rows stay lightweight and the media loads only when a session is expanded. Sessions without assets degrade to just the expand control.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughTestingBot session data now includes thumbnail URLs. Build and embed pages render sessions as expandable details with status icons, direct TestingBot links, and lazily loaded embedded media. The README documents the new session presentation. ChangesSession Media UI
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
actor User
participant Jenkins as Jenkins session view
participant TestingBot
User->>Jenkins: Expand a session
Jenkins->>TestingBot: Request mini view with sessionId and authHash
TestingBot-->>Jenkins: Display session video, screenshots, and logs
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@README.md`:
- Line 57: Update the README description of TestingBot session media to avoid
guaranteeing that expansion triggers loading. Either implement deferred iframe
creation when the session details open, or soften the wording to state that
media “loads lazily,” while preserving the existing preview and in-place
expansion behavior.
In `@src/main/java/testingbot/TestingBotBuildObject.java`:
- Around line 40-41: Update getThumbUrl() in TestingBotBuildObject to return an
empty string when thumbUrl is null, while preserving the existing value for
non-null thumbnails.
In `@src/main/resources/testingbot/TestingBotBuildSummary/summary.jelly`:
- Around line 14-25: The pass/fail SVG status indicators lack accessible names.
In the j:choose branches of
src/main/resources/testingbot/TestingBotBuildSummary/summary.jelly lines 14-25
and src/main/resources/testingbot/TestingBotTestEmbed/index.jelly lines 18-29,
add concise accessible text or aria-label values identifying the passed and
failed states for each SVG.
- Around line 30-33: The TestingBot iframes must not fetch until their rows
open. In TestingBotBuildSummary/summary.jelly and
TestingBotTestEmbed/index.jelly, replace the iframe src with data-src and add
first-open handling that copies data-src to src when the containing row is
expanded; update README.md to retain the lazy-loading behavior claim only once.
- Around line 12-13: Add the Jelly escape-by-default directive at the top of
summary.jelly so dynamic PCDATA values rendered in the cs.testName and
cs.environmentName spans are escaped. Keep the existing markup and variable
rendering unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: db471789-1f5a-4d37-9073-2945b15f0657
📒 Files selected for processing (4)
README.mdsrc/main/java/testingbot/TestingBotBuildObject.javasrc/main/resources/testingbot/TestingBotBuildSummary/summary.jellysrc/main/resources/testingbot/TestingBotTestEmbed/index.jelly
Comment on lines
+30
to
+33
| <!-- loading="lazy" defers the embed until the row is expanded (the iframe is | ||
| display:none inside a closed details), so collapsed rows stay lightweight. --> | ||
| <iframe src="https://testingbot.com/mini/?id=${cs.sessionId}&auth=${cs.authHash}&ref=jenkins" | ||
| loading="lazy" style="width: 100%; min-height: 600px; border: none"></iframe> |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map the relevant files first
printf '\n== Files ==\n'
git ls-files | rg '^(src/main/resources/testingbot/TestingBot(BuildSummary|TestEmbed)/|README\.md)'
printf '\n== Outline: summary.jelly ==\n'
ast-grep outline src/main/resources/testingbot/TestingBotBuildSummary/summary.jelly --view expanded || true
printf '\n== Outline: index.jelly ==\n'
ast-grep outline src/main/resources/testingbot/TestingBotTestEmbed/index.jelly --view expanded || true
printf '\n== Read summary.jelly around the reported lines ==\n'
sed -n '1,220p' src/main/resources/testingbot/TestingBotBuildSummary/summary.jelly | nl -ba | sed -n '1,120p'
printf '\n== Read index.jelly around the reported lines ==\n'
sed -n '1,220p' src/main/resources/testingbot/TestingBotTestEmbed/index.jelly | nl -ba | sed -n '1,140p'
printf '\n== Search for expansion/loading logic or data-src usage ==\n'
rg -n "data-src|loading=|details|toggle|expand|iframe" src/main/resources src/main/java README.mdRepository: testingbot/TestingBot-Jenkins-Plugin
Length of output: 566
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# If the first search suggests client-side behavior, inspect any associated JS/Stapler resources in this plugin.
git ls-files | rg '(^src/main/(resources|webapp|javascript|js|java)/|package\.json$|pom\.xml$)'
printf '\n== Search for TestingBot-related client code ==\n'
rg -n "TestingBot|details|toggle|iframe|loading=\"lazy\"|data-src" src/main/resources src/main/java src/main/webapp src/main/js src/main/javascript 2>/dev/null || trueRepository: testingbot/TestingBot-Jenkins-Plugin
Length of output: 28441
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check whether README wording aligns with the current UI markup
sed -n '45,70p' README.md | nl -ba
printf '\n== Search for the exact README claim elsewhere ==\n'
rg -n "preview thumbnail|expands in place|Collapsed sessions stay lightweight|media only loads" .Repository: testingbot/TestingBot-Jenkins-Plugin
Length of output: 214
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "## summary.jelly"
sed -n '1,120p' src/main/resources/testingbot/TestingBotBuildSummary/summary.jelly | nl -ba
echo
echo "## index.jelly"
sed -n '1,140p' src/main/resources/testingbot/TestingBotTestEmbed/index.jelly | nl -ba
echo
echo "## README.md"
sed -n '50,62p' README.md | nl -ba
echo
echo "## TestingBot-related code search"
rg -n "TestingBot|loading=\"lazy\"|data-src|details|toggle|expand|iframe" src/main/resources src/main/java README.md || trueRepository: testingbot/TestingBot-Jenkins-Plugin
Length of output: 231
🌐 Web query:
HTMLIFrameElement loading lazy is a hint not a guarantee iframe inside closed details display:none loads when expanded
💡 Result:
The loading="lazy" attribute on an iframe is a hint to the browser to defer loading until the element is near the viewport, not a guarantee that it will behave in a specific way relative to CSS visibility [1][2]. When an iframe with loading="lazy" is placed inside a closed
Details
element (which applies display: none to its contents), the browser's behavior is inconsistent and generally unreliable for triggering a load upon expansion [3][4]. 1. Why it often fails: The native lazy-loading mechanism for iframes typically relies on an Intersection Observer [5]. When an element is inside a closed element, it has no layout dimensions (it is essentially display: none) [4]. Consequently, it cannot intersect with the viewport. Even when the
element is expanded, browsers may not always re-evaluate the lazy-loading trigger for that specific iframe, causing it to remain in a non-loaded state [3][4]. 2. Not a Guarantee: As noted, loading="lazy" is a hint [2]. Browsers prioritize performance, and their heuristics for when to load these elements can vary across implementations and versions [2][3]. 3. Recommended Workaround: If you need to ensure an iframe loads only when a
element is opened, you should not rely on the loading="lazy" attribute alone [4]. Instead, use a JavaScript-based approach: - Keep the iframe's src attribute empty (or set to a placeholder) initially. - Add an event listener to the
element for the toggle event. - When the
element opens, set the src attribute of the iframe via JavaScript to trigger the load [6][5]. This provides a deterministic way to load the content only when the user interacts with the UI [6].
Citations:
- 1: https://developer.mozilla.org/en-US/docs/Web/HTML/Reference/Elements/iframe
- 2: https://web.dev/articles/iframe-lazy-loading
- 3: https://bugzilla.mozilla.org/show_bug.cgi?id=1717264
- 4: https://stackoverflow.com/questions/70634302/html-combining-lazy-loading-summary
- 5: Frame with
[loading="lazy"]cannot load when styled withdisplay: contentshotwired/turbo#625 - 6: Lazyload iFrame within a collapsed Boostrap element verlok/vanilla-lazyload#465
Defer the TestingBot iframe until the row opens. loading="lazy" is only a hint, so the embed can still start fetching before expansion. Move the URL to data-src and set src on first open in:
src/main/resources/testingbot/TestingBotBuildSummary/summary.jellysrc/main/resources/testingbot/TestingBotTestEmbed/index.jelly
Keep the README claim only once that behavior is enforced.
📍 Affects 3 files
src/main/resources/testingbot/TestingBotBuildSummary/summary.jelly#L30-L33(this comment)src/main/resources/testingbot/TestingBotTestEmbed/index.jelly#L31-L35README.md#L57-L57
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/main/resources/testingbot/TestingBotBuildSummary/summary.jelly` around
lines 30 - 33, The TestingBot iframes must not fetch until their rows open. In
TestingBotBuildSummary/summary.jelly and TestingBotTestEmbed/index.jelly,
replace the iframe src with data-src and add first-open handling that copies
data-src to src when the containing row is expanded; update README.md to retain
the lazy-loading behavior claim only once.
…ten README - Add the escape-by-default directive to summary.jelly so dynamic values (cs.testName, cs.environmentName) are HTML-escaped, matching index.jelly and preventing markup in a session/test name from injecting into the build page. - Give the pass/fail status SVGs an accessible name (role=img + aria-label Passed/Failed) in both summary.jelly and the embed index.jelly. - Soften the README wording from "only loads when expanded" to "loads lazily", since loading=lazy defers but does not strictly guarantee load-on- expand across browsers.
# Conflicts: # README.md
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/resources/testingbot/TestingBotBuildSummary/summary.jelly (1)
33-34: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAdd a descriptive
titleto every TestingBot iframe.
src/main/resources/testingbot/TestingBotBuildSummary/summary.jellysrc/main/resources/testingbot/TestingBotTestEmbed/index.jellysrc/main/resources/testingbot/TestingBotTestEmbed/show.jellysrc/main/resources/testingbot/TestingBotReport/index.jellyUse a meaningful label like
TestingBot session mediaso screen readers can identify the embed.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/resources/testingbot/TestingBotBuildSummary/summary.jelly` around lines 33 - 34, Add a descriptive title such as “TestingBot session media” to every TestingBot iframe in src/main/resources/testingbot/TestingBotBuildSummary/summary.jelly lines 33-34 and src/main/resources/testingbot/TestingBotTestEmbed/index.jelly lines 34-35; apply the same title to the iframes in TestingBotTestEmbed/show.jelly and TestingBotReport/index.jelly.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@README.md`:
- Line 57: Update the README wording around TestingBot session previews to
reflect that TestingBotBuildSummary only displays the first session ID per JUnit
case; either narrow the claim to the displayed session or change the
session-handling implementation to collect and render all IDs, keeping the
documentation and behavior consistent.
---
Outside diff comments:
In `@src/main/resources/testingbot/TestingBotBuildSummary/summary.jelly`:
- Around line 33-34: Add a descriptive title such as “TestingBot session media”
to every TestingBot iframe in
src/main/resources/testingbot/TestingBotBuildSummary/summary.jelly lines 33-34
and src/main/resources/testingbot/TestingBotTestEmbed/index.jelly lines 34-35;
apply the same title to the iframes in TestingBotTestEmbed/show.jelly and
TestingBotReport/index.jelly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 2c23eddd-517a-499a-98a1-a5f3a8af75b1
📒 Files selected for processing (3)
README.mdsrc/main/resources/testingbot/TestingBotBuildSummary/summary.jellysrc/main/resources/testingbot/TestingBotTestEmbed/index.jelly
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@README.md`:
- Line 63: Update the TestingBot Build description in the README to document the
complete lazy-loaded embedded session experience: include thumbnail previews,
screenshots, and logs in addition to status and video, and state that the media
loads lazily.
- Line 59: Update the “Embedded TestingBot Build report” heading in README.md to
use the configured setext Markdown syntax instead of an ATX heading, preserving
the heading text and level.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@README.md`:
- Line 63: Update the TestingBot Build description in the README to document the
complete lazy-loaded embedded session experience: include thumbnail previews,
screenshots, and logs in addition to status and video, and state that the media
loads lazily.
- Line 59: Update the “Embedded TestingBot Build report” heading in README.md to
use the configured setext Markdown syntax instead of an ATX heading, preserving
the heading text and level.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: f94556b4-058d-463e-ac4d-e3d3e9dd3ffe
📒 Files selected for processing (1)
README.md
🛑 Comments failed to post (2)
README.md (2)
59-59: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Use the configured Markdown heading style.
markdownlint-cli2reports that this heading should use setext syntax instead of an ATX heading.Proposed fix
-## Embedded TestingBot Build report +Embedded TestingBot Build report +--------------------------------📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.Embedded TestingBot Build report --------------------------------🧰 Tools
🪛 markdownlint-cli2 (0.23.0)
[warning] 59-59: Heading style
Expected: setext; Actual: atx(MD003, heading-style)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@README.md` at line 59, Update the “Embedded TestingBot Build report” heading in README.md to use the configured setext Markdown syntax instead of an ATX heading, preserving the heading text and level.Source: Linters/SAST tools
63-63: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Document the complete lazy-loaded media experience.
This sentence currently mentions only status and video, but the UI also provides thumbnail previews plus screenshots and logs in the embedded session view. Update it to describe the behavior promised by this PR, including that the media loads lazily.
Proposed wording
-Set your test's `build` desired capability to `$TESTINGBOT_BUILD` so all sessions from a single Jenkins build are grouped under one TestingBot build. The **TestingBot Build** link on the build page then embeds that build's TestingBot report (every session, with status and video) directly inside Jenkins, without leaving the CI UI. +Set your test's `build` desired capability to `$TESTINGBOT_BUILD` so all sessions from a single Jenkins build are grouped under one TestingBot build. The **TestingBot Build** link on the build page then embeds that build's TestingBot report directly inside Jenkins, with thumbnail previews and lazily loaded video, screenshots, and logs for the displayed sessions.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.Set your test's `build` desired capability to `$TESTINGBOT_BUILD` so all sessions from a single Jenkins build are grouped under one TestingBot build. The **TestingBot Build** link on the build page then embeds that build's TestingBot report directly inside Jenkins, with thumbnail previews and lazily loaded video, screenshots, and logs for the displayed sessions.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@README.md` at line 63, Update the TestingBot Build description in the README to document the complete lazy-loaded embedded session experience: include thumbnail previews, screenshots, and logs in addition to status and video, and state that the media loads lazily.
- TestingBotBuildObject.getThumbUrl() returns "" when thumbUrl is null, so build records persisted before the field existed (deserialized as null) don't render a broken thumbnail img. - README now states the build page shows each JUnit test's session (the first one when a test records several), matching TestingBotBuildSummary which keys on sessionIds.get(0) per case.
jochen-testingbot
added a commit
that referenced
this pull request
Jul 16, 2026
PRs #29 and #30 were merged into the build-report-embed branch, but that branch had already been squash-merged to master as #28 beforehand, so their code never reached master. This brings the stranded changes forward, on top of the current master (which already has #28 and the #31 inline-media feature): - #29: run the freestyle tunnel on the build's agent (TunnelManager.startOnChannel/ stopOnChannel + shared registry; wrapper + testingbotTunnel step delegate to it). - #30: TestingBotChecksPublisher (testingbotChecks) publishing results as a GitHub check, plus the checks-api + display-url-api dependencies and docs. No behavior change versus the already-reviewed #29/#30; only rebased onto master. The inline-media (#31) files are left untouched.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Shows TestingBot session media inline on the build page, instead of just text links.
Change
The build summary panel and the TestingBot embed page previously listed each session as a plain link. Each session now renders as a native
<details>disclosure: a preview thumbnail + test name + environment + pass/fail by default, expanding in place to the fullmini/view (video screencast, screenshots, logs).TestingBotBuildObjectgains athumbUrl, derived from the session's first available thumbnail (getThumbs()) at build completion — Strings only (JEP-200-safe), and no extra network I/O since the test details are already fetched by the summary listener.<iframe>usesloading="lazy", so a collapsed session (the iframe isdisplay:noneinside a closed<details>) doesn't load — the media loads only when the row is expanded. Keeps builds with many sessions light.Validation
mvn -DskipTests verify(SpotBugs on) — clean.hpi:runscript console + page fetch:thumbUrlpopulates fromgetThumbs()and survives asave()+ reload (JEP-200 marshaling clean); a null-asset session yields an emptythumbUrl.testingbot-embedpage each render one<details>per session, the thumbnail<img>only for the session with assets, bothmini/iframes withloading="lazy", and the no-asset session shows just the expand control.Note
Independent of #28/#29/#30 — based on
master.Summary by CodeRabbit