Accept hyphens in TestingBotSessionID so modern session ids resolve#39
Merged
Conversation
Modern TestingBot / W3C WebDriver session ids are hyphenated (e.g. b0dbacacfa63-342d529ca760-...). The extraction pattern was TestingBotSessionID=([A-Za-z0-9]+), which stopped at the first hyphen and captured only the leading segment. That truncated id 404s against the TestingBot REST API (the full id maps to an internal test id) and, because getAuthenticationHash() then hashes the wrong string, the embedded mini iframe and share links break too. Net effect: every embedded report, Build page and GitHub-checks summary silently failed for real sessions. Widen the character class to [A-Za-z0-9_-]+ and add TestingBotSessionIdParsingTest covering hyphenated, plain, multiple and no-match inputs.
|
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 (1)
📝 WalkthroughWalkthroughThe session ID regex now accepts letters, digits, underscores, and hyphens. New tests verify hyphenated IDs, plain IDs, multiple tokens, and text without session ID tokens. ChangesSession ID parsing
Estimated code review effort: 1 (Trivial) | ~5 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@src/test/java/testingbot/TestingBotSessionIdParsingTest.java`:
- Around line 31-36: Add an underscore-containing session ID such as abc_def-123
to capturesMultipleSessionIds and update the assertion to verify the complete
value is captured alongside the existing IDs.
🪄 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: 64de2a0f-97c6-4590-a104-c1b48cc9a49d
📒 Files selected for processing (2)
src/main/java/testingbot/TestingBotReportFactory.javasrc/test/java/testingbot/TestingBotSessionIdParsingTest.java
The pattern allows [A-Za-z0-9_-]+; add an underscore-containing id (abc_def-123) to capturesMultipleSessionIds so the underscore branch of the character class is exercised alongside the hyphenated ids.
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
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.
Problem
Modern TestingBot / W3C WebDriver session ids are hyphenated, e.g.
b0dbacacfa63-342d529ca760-2f981532f469-....TestingBotReportFactoryextracted them withTestingBotSessionID=([A-Za-z0-9]+), which stops at the first hyphen and captures only the leading segment.Verified against the live API during an end-to-end test: the full id resolves (
/tests/<full>→ an internal test id) while the truncated segment 404s. BecausegetAuthenticationHash()then hashes the truncated id, the embeddedmini/iframe and the share links break as well. Net effect: embedded reports, the TestingBot Build page and the GitHub-checks summary silently failed for every real session.Fix
Widen the character class to
([A-Za-z0-9_-]+). Single extraction point; no other id parsing exists.Adds
TestingBotSessionIdParsingTest(pure unit test on the staticcollectSessionIDs) covering hyphenated, plain-hex, multiple and no-match inputs.Summary by CodeRabbit
Bug Fixes
Tests