Skip to content

Corrected documentation and comment drift, replaced expectation-free mocks with stubs, and trimmed the distribution archive. - #306

Merged
AlexSkrypnyk merged 4 commits into
mainfrom
feature/polish-260916-1728
Sep 16, 2026
Merged

AlexSkrypnyk merged 4 commits into
mainfrom
feature/polish-260916-1728

Conversation

@AlexSkrypnyk

@AlexSkrypnyk AlexSkrypnyk commented Sep 16, 2026 •

Copy link
Copy Markdown
Member

Summary

README.md documents BEHAT_SCREENSHOT_TOKEN_HOST and the {feature} token, states that the {step_name} token strips double quotes, and shows the literal nl2br()-generated markup for the info block; the comments on ScreenshotContext::captureScreenshot() and BehatCliTrait::behatCliIsDebug() now describe what the code actually does; and beforeScenarioUpdateBaseUrl() in the FeatureContextTest heredoc template returns early instead of nesting its body inside an if.

BEHAT_SCREENSHOT_TOKEN_HOST had no documentation at all, {feature} resolved through Tokenizer::replaceFeatureToken() identically to {feature_file} with nothing in the table or the tests recording it, the {step_name} row omitted the double-quote removal Tokenizer::replaceStepToken() performs, the info-block example showed <br/> and a standalone <hr/> line rather than the <br /> and same-line <hr/> that nl2br() actually emits, the driver comment in captureScreenshot() referenced a Goutte-shipped-with-Behat example this repo replaced with behat/mink-browserkit-driver, the docblock on behatCliIsDebug() claimed any non-empty string enables debug output when call sites use plain truthiness so BEHAT_CLI_DEBUG="0" disables it, and AnimationAssemblyProfileTest::createBeforeScenarioScope() duplicated BehatScopeTrait::createBeforeScenarioScope() byte-for-byte while silently shadowing it.

PHPUnit now doubles collaborators with createStub() instead of createMock() in BehatScopeTrait, ScreenshotContextTest, ScreenshotContextInfoTest, ScreenshotContextResizeTest and ScreenshotContextAnimationTest wherever a test configures no expectation, dropping the notice count from 27 to 14 with the assertion count unchanged; .gitattributes adds .claude, CLAUDE.md and logo.png to export-ignore so the distribution archive holds only src/, the four OSS docs, behat.dist.php and composer.json; no src/ behaviour changes anywhere - the sole src/ edit is the comment in captureScreenshot().

Before / After

BEFORE - git archive (dist package)
├── src/
├── README.md
├── LICENSE
├── CONTRIBUTING.md
├── SECURITY.md
├── composer.json
├── behat.dist.php
├── .claude/
├── CLAUDE.md
└── logo.png

AFTER - git archive (dist package)
├── src/
├── README.md
├── LICENSE
├── CONTRIBUTING.md
├── SECURITY.md
├── composer.json
└── behat.dist.php
BEFORE - filename token table
{feature_file}  -> my_example        documented
{feature}       -> my_example        undocumented, untested

AFTER - filename token table
{feature_file}  -> my_example        documented
{feature}       -> my_example        documented as an alias, pinned by a test

Changes

  • README.md - documented BEHAT_SCREENSHOT_TOKEN_HOST, added a {feature} row to the filename token table as an alias of {feature_file}, corrected the {step_name} token description, and corrected the info-block HTML example to match actual nl2br() output.
  • .gitattributes - dropped export-ignore entries for docker-compose.yml, docker-compose.override.default.yml and phpmd.xml (none exist in this repo), and added .claude, CLAUDE.md and logo.png so they no longer ship in the distribution archive.
  • src/DrevOps/BehatScreenshotExtension/Context/ScreenshotContext.php - rewrote the captureScreenshot() comment to describe a driver without screenshot support rather than a "Goutte shipped with Behat" example that does not apply to this project.
  • tests/behat/bootstrap/BehatCliTrait.php - added a guard clause to beforeScenarioUpdateBaseUrl(), removed a comment restating the line below it, and dropped the incorrect truthiness claim from the behatCliIsDebug() docblock.
  • tests/phpunit/Unit/TokenizerTest.php - added a feature token alias dataset asserting {feature}.{feature_file}.{ext} resolves to foo-file.foo-file.png, pinning the newly documented alias.
  • tests/phpunit/Profile/AnimationAssemblyProfileTest.php - removed createBeforeScenarioScope() and its four now-unused imports; the class now uses BehatScopeTrait::createBeforeScenarioScope() exclusively.
  • tests/phpunit/Traits/BehatScopeTrait.php, ScreenshotContextTest.php, ScreenshotContextInfoTest.php, ScreenshotContextResizeTest.php, ScreenshotContextAnimationTest.php - replaced createMock() with createStub() for every double that configures no expectations, and removed one inert ScenarioInterface::hasTag() stub.

Verification

  • composer lint - phpcs, phpstan, rector and gherkinlint all clean.
  • composer test - 331 tests / 737 assertions passing. The 14 remaining PHPUnit notices all originate in createPartialMock(ScreenshotContext::class, ...), which has no stub equivalent.
  • composer test-bdd -- --tags=~@selenium --tags=~@headless - 29 scenarios / 199 steps passing, covering the edited FeatureContextTest heredoc template.
  • BEHAT_SCREENSHOT_PROFILE_STEPS=2 composer profile - confirms the removed createBeforeScenarioScope() was behaviour-identical to the trait method that now serves it.
  • git archive output confirmed clean, matching the new export-ignore list.

@coderabbitai

coderabbitai Bot commented Sep 16, 2026 •

Copy link
Copy Markdown

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Team

Run ID: e6282f58-789e-4a32-9579-e84fe0d3340c

📥 Commits

Reviewing files that changed from the base of the PR and between 12a4729 and 8a2631b.

📒 Files selected for processing (11)
  • .gitattributes
  • README.md
  • src/DrevOps/BehatScreenshotExtension/Context/ScreenshotContext.php
  • tests/behat/bootstrap/BehatCliTrait.php
  • tests/phpunit/Profile/AnimationAssemblyProfileTest.php
  • tests/phpunit/Traits/BehatScopeTrait.php
  • tests/phpunit/Unit/ScreenshotContextAnimationTest.php
  • tests/phpunit/Unit/ScreenshotContextInfoTest.php
  • tests/phpunit/Unit/ScreenshotContextResizeTest.php
  • tests/phpunit/Unit/ScreenshotContextTest.php
  • tests/phpunit/Unit/TokenizerTest.php
💤 Files with no reviewable changes (1)
  • tests/phpunit/Profile/AnimationAssemblyProfileTest.php

Included review availability: 1 review is currently available. Your included PR review attempts over the past 7 days set your current allowance at 2 reviews per hour.


📝 Walkthrough

Walkthrough

The pull request updates distribution archive exclusions, documents screenshot filename tokens and host normalization, and revises screenshot-related tests to use stubs and shared setup.

Changes

Screenshot documentation and test maintenance

Layer / File(s) Summary
Token documentation and validation
README.md, tests/phpunit/Unit/TokenizerTest.php, src/DrevOps/BehatScreenshotExtension/Context/ScreenshotContext.php
The README documents the {feature} alias, {step_name} behavior, and BEHAT_SCREENSHOT_TOKEN_HOST. The tokenizer test verifies the alias. The screenshot fallback documentation clarifies the unsupported-driver result.
Test support and fixture cleanup
tests/behat/bootstrap/BehatCliTrait.php, tests/phpunit/Profile/AnimationAssemblyProfileTest.php, tests/phpunit/Traits/BehatScopeTrait.php, tests/phpunit/Unit/*ScreenshotContext*Test.php
Behat setup uses an early return for non-initialized environments. Shared scope helpers and screenshot tests use stubs instead of mocks. The profile test removes its duplicated scope helper.

Distribution archive configuration

Layer / File(s) Summary
Archive exclusion updates
.gitattributes
Archive exclusions add repository files such as .claude, CLAUDE.md, and logo.png. Exclusions for the Docker Compose files and phpmd.xml are removed.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Other

Merge Risk: ⚪ Minimal · up to 8a263

The change remains limited to documentation, test maintenance, and packaging metadata, with no established production regression.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 27.78% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 18 functions across 8 files. (2 skipped: … Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the primary changes: documentation and comment updates, replacement of expectation-free mocks with stubs, and distribution archive cleanup.
Full details: Docstring Coverage

Explanation

Docstring coverage is 27.78% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 18 functions across 8 files. (2 skipped: 2 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feature/polish-260916-1728

Warning

Git: CodeRabbit could not clone the repository, so clone-backed analysis was skipped and this review may be incomplete. Verify repository clone access, such as SSH credentials, before requesting another full review. If clone access is intentionally unavailable, use path_filters to narrow the review scope.


Comment @coderabbitai help to get the list of available commands.

@codecov

codecov Bot commented Sep 16, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.09%. Comparing base (12a4729) to head (8a2631b).
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #306   +/-   ##
=======================================
  Coverage   99.09%   99.09%           
=======================================
  Files           6        6           
  Lines         442      442           
=======================================
  Hits          438      438           
  Misses          4        4           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

@AlexSkrypnyk AlexSkrypnyk added the Needs review Pull request needs a review from assigned developers label Sep 16, 2026
@AlexSkrypnyk
AlexSkrypnyk merged commit 665aada into main Sep 16, 2026
16 checks passed
@AlexSkrypnyk
AlexSkrypnyk deleted the feature/polish-260916-1728 branch September 16, 2026 07:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Needs review Pull request needs a review from assigned developers

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant