Add TESTINGBOT_BUILD env var and embedded build-report page#28
Merged
Conversation
Exposes a per-build identifier (TESTINGBOT_BUILD) from the freestyle wrapper and both pipeline steps, the TestingBot analog of Sauce Labs' SAUCE_BUILD_NAME. Tests that pass its value as their build capability are grouped under a new TestingBotBuildReportAction page, which iframes the build's TestingBot report (/mini/builds/<key>/<buildId>) authenticated with MD5(key:secret:buildId) computed on the controller so the raw secret is never persisted on the build. The action stores only Strings (verified to marshal cleanly under the JEP-200 class filter).
📝 WalkthroughWalkthroughAdds a Jenkins ChangesEmbedded TestingBot Build reports
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant JenkinsRun
participant TestingBotBuildReportAction
participant BuildEnvironment
participant TestingBotReport
JenkinsRun->>TestingBotBuildReportAction: attach credentials and run
TestingBotBuildReportAction->>TestingBotBuildReportAction: compute buildId and authHash
JenkinsRun->>BuildEnvironment: publish TESTINGBOT_BUILD
BuildEnvironment->>TestingBotReport: group sessions by build identifier
JenkinsRun->>TestingBotReport: load embedded report
TestingBotReport-->>JenkinsRun: render TestingBot Build page
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 “Embedded TestingBot Build report” heading in the README
to use the configured Markdown setext style instead of the current heading
syntax, preserving the existing heading text.
In `@src/main/java/testingbot/pipeline/TestingBotTunnelStep.java`:
- Around line 239-241: Ensure TestingBotBuildReportAction.attach rejects
credential mismatches on the same Run or replaces the existing action before
exporting TESTINGBOT_BUILD. Apply this consistently at TestingBotTunnelStep.java
lines 239-241 and TestingBotStep.java lines 91-93, preserving the existing
behavior when credentials match.
In `@src/main/java/testingbot/TestingBotBuildReportAction.java`:
- Around line 62-66: Update TestingBotBuildReportAction.attach to synchronize
the lookup-and-creation sequence so parallel calls cannot add duplicate actions;
then return the selected action’s persisted buildId rather than recomputing it
via buildIdFor(run). Preserve the existing action when one is already attached.
- Around line 53-55: Update buildIdFor to use an injective, URL-safe encoding of
the full job name and build number instead of replacing unsupported characters
with hyphens. Preserve deterministic output while ensuring distinct names such
as “qa smoke” and “qa-smoke” cannot produce the same identifier.
In `@src/main/resources/testingbot/TestingBotBuildReportAction/index.jelly`:
- Line 16: Add a descriptive title attribute to the iframe in the TestingBot
build report markup, identifying it as the embedded TestingBot report while
preserving the existing src and styling.
🪄 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: 9db17927-c2d2-4fbb-97a4-eb85fc7997de
📒 Files selected for processing (6)
README.mdsrc/main/java/testingbot/TestingBotBuildReportAction.javasrc/main/java/testingbot/TestingBotBuildWrapper.javasrc/main/java/testingbot/pipeline/TestingBotStep.javasrc/main/java/testingbot/pipeline/TestingBotTunnelStep.javasrc/main/resources/testingbot/TestingBotBuildReportAction/index.jelly
|
|
||
| A full example that you can use is available on our GitHub [Jenkins-Demo](https://github.com/testingbot/Jenkins-Demo) page. | ||
|
|
||
| ## Embedded TestingBot Build report |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Use the configured setext heading style.
Markdownlint expects a setext heading here.
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.
Suggested change
| ## Embedded TestingBot Build report | |
| Embedded TestingBot Build report | |
| -------------------------------- |
🧰 Tools
🪛 markdownlint-cli2 (0.23.0)
[warning] 57-57: 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 57, Update the “Embedded TestingBot Build report” heading
in the README to use the configured Markdown setext style instead of the current
heading syntax, preserving the existing heading text.
Source: Linters/SAST tools
Comment on lines
+239
to
+241
| // Expose the build id and add the embeddable build-report page so all sessions using | ||
| // build=$TESTINGBOT_BUILD are grouped and viewable inside Jenkins. | ||
| env.put(TestingBotBuildReportAction.TESTINGBOT_BUILD, TestingBotBuildReportAction.attach(run, tbCredentials)); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map the relevant files first.
ast-grep outline src/main/java/testingbot/pipeline/TestingBotStep.java --view expanded || true
ast-grep outline src/main/java/testingbot/pipeline/TestingBotTunnelStep.java --view expanded || true
ast-grep outline src/main/java/testingbot/TestingBotBuildReportAction.java --view expanded || true
# Show the relevant line ranges with line numbers.
printf '\n--- TestingBotStep.java ---\n'
sed -n '1,180p' src/main/java/testingbot/pipeline/TestingBotStep.java | cat -n
printf '\n--- TestingBotTunnelStep.java ---\n'
sed -n '200,280p' src/main/java/testingbot/pipeline/TestingBotTunnelStep.java | cat -n
printf '\n--- TestingBotBuildReportAction.java ---\n'
sed -n '1,260p' src/main/java/testingbot/TestingBotBuildReportAction.java | cat -nRepository: testingbot/TestingBot-Jenkins-Plugin
Length of output: 19661
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n--- usages of TestingBotBuildReportAction.attach ---\n'
rg -n "TestingBotBuildReportAction\.attach\(" src/main/java || true
printf '\n--- TestingBotBuildAction.java ---\n'
sed -n '1,260p' src/main/java/testingbot/TestingBotBuildAction.java | cat -n
printf '\n--- testingbot credential usages ---\n'
rg -n "getCredentialsId\\(|TestingBotCredentials\\.getCredentials|run\\.addAction\\(new TestingBotBuildAction|TestingBotBuildReportAction" src/main/java/testingbot -g '!**/target/**' || trueRepository: testingbot/TestingBot-Jenkins-Plugin
Length of output: 4928
Reject mixed TestingBot credentials on the same run. TestingBotBuildReportAction.attach keeps the first report action on the Run, so a later testingbot {} or testingbotTunnel {} block with different credentials will still export the original report URL/hash. Fail on credential mismatch or replace the attached action before setting TESTINGBOT_BUILD.
src/main/java/testingbot/pipeline/TestingBotStep.java#L91-L93src/main/java/testingbot/pipeline/TestingBotTunnelStep.java#L239-L241
📍 Affects 2 files
src/main/java/testingbot/pipeline/TestingBotTunnelStep.java#L239-L241(this comment)src/main/java/testingbot/pipeline/TestingBotStep.java#L91-L93
🤖 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/java/testingbot/pipeline/TestingBotTunnelStep.java` around lines 239
- 241, Ensure TestingBotBuildReportAction.attach rejects credential mismatches
on the same Run or replaces the existing action before exporting
TESTINGBOT_BUILD. Apply this consistently at TestingBotTunnelStep.java lines
239-241 and TestingBotStep.java lines 91-93, preserving the existing behavior
when credentials match.
Comment on lines
+53
to
+55
| public static String buildIdFor(Run<?, ?> run) { | ||
| return (run.getParent().getFullName() + "_" + run.getNumber()).replaceAll("[^A-Za-z0-9._-]", "-"); | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Make the build identifier collision-resistant.
Line 54 maps distinct job names such as qa smoke and qa-smoke to the same ID. With equal build numbers, their TestingBot sessions merge into one report and may cross job boundaries. Use an injective URL-safe encoding instead of replacing characters.
Proposed fix
+import java.util.Base64;
...
public static String buildIdFor(Run<?, ?> run) {
- return (run.getParent().getFullName() + "_" + run.getNumber()).replaceAll("[^A-Za-z0-9._-]", "-");
+ String rawId = run.getParent().getFullName() + "_" + run.getNumber();
+ return Base64.getUrlEncoder()
+ .withoutPadding()
+ .encodeToString(rawId.getBytes(StandardCharsets.UTF_8));
}🤖 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/java/testingbot/TestingBotBuildReportAction.java` around lines 53 -
55, Update buildIdFor to use an injective, URL-safe encoding of the full job
name and build number instead of replacing unsupported characters with hyphens.
Preserve deterministic output while ensuring distinct names such as “qa smoke”
and “qa-smoke” cannot produce the same identifier.
Comment on lines
+62
to
+66
| public static String attach(Run<?, ?> run, TestingBotCredentials credentials) { | ||
| if (run.getAction(TestingBotBuildReportAction.class) == null) { | ||
| run.addAction(create(run, credentials)); | ||
| } | ||
| return buildIdFor(run); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
Serialize action creation and return the persisted ID.
Lines 63-65 are a non-atomic check-then-add. Parallel Pipeline branches can attach duplicate actions with the same URL name. Synchronize lookup/creation (or use an atomic action API), then return the selected action’s stored buildId; recomputing it can diverge after a job rename.
🤖 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/java/testingbot/TestingBotBuildReportAction.java` around lines 62 -
66, Update TestingBotBuildReportAction.attach to synchronize the
lookup-and-creation sequence so parallel calls cannot add duplicate actions;
then return the selected action’s persisted buildId rather than recomputing it
via buildIdFor(run). Preserve the existing action when one is already attached.
| <a href="${it.reportUrl}&ref=jenkins" target="_blank">View on TestingBot</a> | ||
| <l:ajax> | ||
| <div> | ||
| <iframe src="${it.reportUrl}&ref=jenkins" style="width: 100%; min-height: 100vh; border: none"></iframe> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Give the embedded report an accessible name.
Screen-reader users cannot identify the iframe’s content. Add a descriptive title.
Proposed fix
-<iframe src="${it.reportUrl}&ref=jenkins" style="width: 100%; min-height: 100vh; border: none"></iframe>
+<iframe title="TestingBot Build report" src="${it.reportUrl}&ref=jenkins" style="width: 100%; min-height: 100vh; border: none"></iframe>📝 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.
Suggested change
| <iframe src="${it.reportUrl}&ref=jenkins" style="width: 100%; min-height: 100vh; border: none"></iframe> | |
| <iframe title="TestingBot Build report" src="${it.reportUrl}&ref=jenkins" style="width: 100%; min-height: 100vh; border: none"></iframe> |
🤖 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/TestingBotBuildReportAction/index.jelly` at
line 16, Add a descriptive title attribute to the iframe in the TestingBot build
report markup, identifying it as the embedded TestingBot report while preserving
the existing src and styling.
This was referenced Jul 16, 2026
…itle Addresses review findings on the build-report feature: - buildIdFor is now injective: when sanitizing the job name replaces characters it appends a short deterministic hash of the raw value, so distinct names like "qa smoke" and "qa-smoke" can no longer collapse to the same identifier (already-safe names are unchanged). - attach synchronizes on the run so concurrent steps in one build cannot add duplicate actions, returns the persisted build id, and replaces the action when a different TestingBot account (client key) is used later in the same build. - The embedded report iframe gets a descriptive title for accessibility.
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
Adds Sauce-Labs-style build grouping plus an embedded build report, closing two of the connected feature-parity gaps from the modernization review:
TESTINGBOT_BUILDenv var — a per-build identifier exposed by the freestyle Build Environment wrapper and both pipeline steps (testingbot { },testingbotTunnel { }). Tests pass its value as theirbuilddesired capability so every session from one Jenkins build is grouped under one TestingBot build. This is the TestingBot analog of Sauce Labs'SAUCE_BUILD_NAME.Embedded "TestingBot Build" page — a new
TestingBotBuildReportAction(RunAction2) added to each build that injects credentials. It iframes the build's report at/mini/builds/<clientKey>/<buildId>(the page added in the web app), so the full report — every session, status, video — is viewable without leaving Jenkins.Security / persistence notes
MD5(clientKey:clientSecret:buildId), computed once on the controller. Only the resulting hash is stored on the build — the raw secret is never persisted.Stringfields (clientKey, buildId, authHash). Verified to marshal cleanly under the JEP-200 class filter via a save + reload round-trip.buildIdis sanitized to[A-Za-z0-9._-], safe both as a capability value and as a URL path segment.Validation
mvn clean verify(SpotBugs enabled) — clean.hpi:runscript-console lifecycle test: action attaches, survivesbuild.save()+ reload (no JEP-200 marshal failure),getReportUrl()well-formed, and the auth hash matchesMD5(key:secret:buildId)— confirmed byte-identical to the web app'sDigest::MD5.hexdigest.<h1>TestingBot Build</h1>+ iframe pointing at the authenticated report URL withref=jenkins.Depends on
The web app route
GET /mini/builds/:client_key/:build_id(already added). The embed only works once that endpoint is deployed.Summary by CodeRabbit
New Features
TESTINGBOT_BUILDenvironment variable for grouping test sessions in a single report.Documentation
TESTINGBOT_BUILDas the test build capability.