-
-
Notifications
You must be signed in to change notification settings - Fork 28
[#2006] Added metadata to Behat screenshots. #2003
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughAdds Behat screenshot metadata configuration (info_types: url, feature, step, datetime), inserts assertions to verify screenshot metadata in a Bats workflow helper, and normalizes trailing newlines in Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Runner as Test Runner
participant Behat as Behat
participant Ext as BehatScreenshotExtension
participant FS as Filesystem
Runner->>Behat: Execute scenario
Behat->>Ext: Request screenshot with metadata
Note right of Ext #D3E4CD: Config: info_types = [url, feature, step, datetime]
Ext->>FS: Write screenshot file + embed metadata (HTML)
FS-->>Ext: Save confirmation
Ext-->>Behat: Return artifact path/details
Behat-->>Runner: Test result + screenshot artifact available
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: ASSERTIVE Plan: Pro ⛔ Files ignored due to path filters (3)
📒 Files selected for processing (4)
🧰 Additional context used🧠 Learnings (1)📚 Learning: 2025-08-08T12:02:24.652ZApplied to files:
🧬 Code graph analysis (1).vortex/tests/bats/_helper.workflow.bash (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (14)
🔇 Additional comments (3)
Pre-merge checks (1 passed, 2 warnings)❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing Touches
🧪 Generate unit tests
Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (3)
.vortex/installer/tests/Fixtures/install/_baseline/.eslintignoreis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/_baseline/.stylelintrc.jsis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/_baseline/behat.ymlis excluded by!.vortex/installer/tests/Fixtures/**
📒 Files selected for processing (3)
.eslintignore(1 hunks).stylelintrc.js(1 hunks)behat.yml(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (14)
- GitHub Check: build (1)
- GitHub Check: build (0)
- GitHub Check: vortex-test-workflow (3)
- GitHub Check: vortex-test-workflow (2)
- GitHub Check: vortex-test-workflow (0)
- GitHub Check: vortex-test-workflow (4)
- GitHub Check: vortex-test-workflow (1)
- GitHub Check: vortex-test-common
- GitHub Check: vortex-test-deployment (0)
- GitHub Check: vortex-test-deployment (1)
- GitHub Check: vortex-test-installer (8.4)
- GitHub Check: vortex-test-installer (8.3)
- GitHub Check: vortex-test-installer (8.2)
- GitHub Check: vortex-test-docs
🔇 Additional comments (4)
behat.yml (2)
90-94: LGTM: added screenshot metadata improves triage.Including url, feature, step, and datetime will make failures easier to diagnose.
90-94: No changes required:info_typessupported by the extension
DrevOps/BehatScreenshotExtension v2.1 (and since v1.0) definesinfo_typeswith default valuesurl,feature,step,datetime(github.com).stylelintrc.js (1)
39-39: LGTM: normalize trailing newline.No functional changes.
.eslintignore (1)
10-10: LGTM: add trailing newline.Ignore patterns unchanged.
| info_types: # Additional information to include in screenshots. | ||
| - url | ||
| - feature | ||
| - step | ||
| - datetime |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick (assertive)
Consider adding run-identifying context for parallel jobs.
Optional: if supported, include build ID/run number or profile name in metadata to disambiguate screenshots from parallel Behat profiles (p0/p1).
🤖 Prompt for AI Agents
In behat.yml around lines 90 to 94, the screenshot info_types list lacks
run-identifying context which makes screenshots ambiguous for parallel Behat
profiles; add keys for build/run identification (for example build_id,
run_number and/or profile) and populate them from your CI/env placeholders or
Behat parameters so each screenshot includes the CI build ID or run number and
the profile name; ensure the chosen keys are supported by your screenshot
reporter and that the CI pipeline exports the corresponding env vars or
parameters.
🧹 Nitpick (assertive)
Avoid leaking secrets via url metadata.
URLs may contain tokens or PII (e.g., access_token, csrf, session). If the extension supports redaction/whitelisting, enable it; otherwise, ensure tests don’t hit URLs with sensitive query params or add a pre-sanitization step before embedding.
🤖 Prompt for AI Agents
In behat.yml around lines 90–94, the config currently includes url in info_types
which can leak tokens/PII in screenshots; either remove "url" from info_types or
enable the extension's redaction/whitelist option, and if redaction isn't
available add a pre-sanitization step that strips sensitive query parameters
(e.g., access_token, csrf, session) or replaces the URL with a
sanitized/whitelisted host before embedding; implement one of these fixes and
ensure tests hitting external services use sanitized URLs or whitelisted
patterns only.
33c1bcf to
6a708c6
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
♻️ Duplicate comments (2)
behat.yml (2)
90-94: Add run/profile identifiers to screenshot metadata for parallel runs.Include CI build/run id and Behat profile to disambiguate artifacts in p0/p1 runs (if supported by the extension).
90-94: Sanitize URL metadata to avoid secret/PII leakage.If URLs may contain tokens (access_token, session, csrf), enable redaction/whitelisting in the extension or sanitize before embedding. Otherwise, drop "url" from info_types.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (3)
.vortex/installer/tests/Fixtures/install/_baseline/.eslintignoreis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/_baseline/.stylelintrc.jsis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/_baseline/behat.ymlis excluded by!.vortex/installer/tests/Fixtures/**
📒 Files selected for processing (4)
.eslintignore(1 hunks).stylelintrc.js(1 hunks).vortex/tests/bats/_helper.workflow.bash(1 hunks)behat.yml(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-08T12:02:24.652Z
Learnt from: AlexSkrypnyk
PR: drevops/vortex#1896
File: .vortex/tests/bats/unit/download-db-lagoon.bats:24-25
Timestamp: 2025-08-08T12:02:24.652Z
Learning: In .vortex/tests/bats/unit Bats tests using ../_helper.bash (run_steps), prefixing a STEPS entry with "- " denotes a negative assertion (the substring must NOT appear in output). Unprefixed entries are positive assertions. Example: "- Database dump refresh requested. Will create a new dump." asserts absence; "Database dump refresh requested. Will create a new dump." asserts presence.
Applied to files:
.vortex/tests/bats/_helper.workflow.bash
🧬 Code graph analysis (1)
.vortex/tests/bats/_helper.workflow.bash (1)
.vortex/tests/bats/_helper.bash (1)
substep(1293-1295)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (14)
- GitHub Check: build (1)
- GitHub Check: build (0)
- GitHub Check: vortex-test-workflow (1)
- GitHub Check: vortex-test-workflow (2)
- GitHub Check: vortex-test-workflow (3)
- GitHub Check: vortex-test-workflow (4)
- GitHub Check: vortex-test-workflow (0)
- GitHub Check: vortex-test-deployment (1)
- GitHub Check: vortex-test-deployment (0)
- GitHub Check: vortex-test-common
- GitHub Check: vortex-test-installer (8.3)
- GitHub Check: vortex-test-installer (8.4)
- GitHub Check: vortex-test-installer (8.2)
- GitHub Check: vortex-test-docs
🔇 Additional comments (3)
.eslintignore (1)
10-10: LGTM: newline normalization only.No functional change to ignore patterns.
.stylelintrc.js (1)
39-39: LGTM: trailing newline added.Config unchanged; formatting improvement only.
.vortex/tests/bats/_helper.workflow.bash (1)
672-678: Stabilize URL expectation.Your assertion expects a trailing slash “…8080/” while base_url in behat.yml is “http://nginx:8080”. Confirm the extension normalizes with “/” to avoid future flakiness.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #2003 +/- ##
===========================================
+ Coverage 77.99% 78.02% +0.02%
===========================================
Files 89 89
Lines 5536 5542 +6
Branches 44 44
===========================================
+ Hits 4318 4324 +6
Misses 1218 1218 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
6a708c6 to
d236f92
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (4)
behat.yml (2)
90-94: Consider adding run-identifying metadata (profile/build) for parallel jobs.If supported by the Screenshot extension, include profile and a CI run/build identifier to disambiguate artifacts across profiles/runs.
Does DrevOps\BehatScreenshotExtension support additional info_types such as scenario, profile, and CI run/build identifiers (e.g., run_number/build_id)? If yes, what are the exact keys?
90-92: Sanitize URL metadata to avoid leaking tokens/PII.Keeping "url" is useful but risky if query params contain secrets. Enable/implement redaction or restrict to origin/path if the extension allows it; otherwise, ensure tests only hit sanitized URLs.
Does DrevOps\BehatScreenshotExtension provide URL redaction/whitelisting for info_types:url? If so, how to configure it?.vortex/tests/bats/_helper.workflow.bash (2)
666-672: Good fix: assertions now run before cleanup.Block moved ahead of rm -rf; prevents false negatives.
668-671: Assert metadata in HTML, not PNG (binary grep is brittle).Switch targets to the deterministic HTML artifact.
Apply:
- assert_file_contains .logs/screenshots/behat-test-screenshot.png "Current URL: http://nginx:8080/" - assert_file_contains .logs/screenshots/behat-test-screenshot.png "Feature: Behat configuration" - assert_file_contains .logs/screenshots/behat-test-screenshot.png "Step: save screenshot with name" - assert_file_contains .logs/screenshots/behat-test-screenshot.png "Datetime:" + assert_file_contains .logs/screenshots/behat-test-screenshot.html "Current URL: http://nginx:8080/" + assert_file_contains .logs/screenshots/behat-test-screenshot.html "Feature: Behat configuration" + assert_file_contains .logs/screenshots/behat-test-screenshot.html "Step: save screenshot with name" + assert_file_contains .logs/screenshots/behat-test-screenshot.html "Datetime:"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (3)
.vortex/installer/tests/Fixtures/install/_baseline/.eslintignoreis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/_baseline/.stylelintrc.jsis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/_baseline/behat.ymlis excluded by!.vortex/installer/tests/Fixtures/**
📒 Files selected for processing (4)
.eslintignore(1 hunks).stylelintrc.js(1 hunks).vortex/tests/bats/_helper.workflow.bash(1 hunks)behat.yml(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-08T12:02:24.652Z
Learnt from: AlexSkrypnyk
PR: drevops/vortex#1896
File: .vortex/tests/bats/unit/download-db-lagoon.bats:24-25
Timestamp: 2025-08-08T12:02:24.652Z
Learning: In .vortex/tests/bats/unit Bats tests using ../_helper.bash (run_steps), prefixing a STEPS entry with "- " denotes a negative assertion (the substring must NOT appear in output). Unprefixed entries are positive assertions. Example: "- Database dump refresh requested. Will create a new dump." asserts absence; "Database dump refresh requested. Will create a new dump." asserts presence.
Applied to files:
.vortex/tests/bats/_helper.workflow.bash
🧬 Code graph analysis (1)
.vortex/tests/bats/_helper.workflow.bash (1)
.vortex/tests/bats/_helper.bash (1)
substep(1293-1295)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (14)
- GitHub Check: build (0)
- GitHub Check: build (1)
- GitHub Check: vortex-test-workflow (1)
- GitHub Check: vortex-test-workflow (3)
- GitHub Check: vortex-test-workflow (2)
- GitHub Check: vortex-test-workflow (4)
- GitHub Check: vortex-test-deployment (0)
- GitHub Check: vortex-test-deployment (1)
- GitHub Check: vortex-test-workflow (0)
- GitHub Check: vortex-test-common
- GitHub Check: vortex-test-installer (8.4)
- GitHub Check: vortex-test-installer (8.2)
- GitHub Check: vortex-test-installer (8.3)
- GitHub Check: vortex-test-docs
🔇 Additional comments (2)
.stylelintrc.js (1)
39-39: No-op formatting change — OK to merge.Trailing newline added; no behavioral impact.
.eslintignore (1)
10-10: No-op formatting change — OK to merge.Only end-of-file newline normalization for '*.min.css'.
|
|
||
| substep "Assert screenshot metadata is present" | ||
| assert_file_exists .logs/screenshots/behat-test-screenshot.html | ||
| assert_file_contains .logs/screenshots/behat-test-screenshot.png "Current URL: http://nginx:8080/" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick (assertive)
Avoid hardcoding base URL in assertions.
Consider reading base URL from behat.yml or an env var to future-proof tests across environments.
Would you like a small helper to parse base_url from behat.yml (e.g., with grep/awk) and use it in assertions?
d236f92 to
17ee726
Compare
Closes #2006
Summary by CodeRabbit
Tests
Chores