Renamed the 'fullscreen' capture option to 'is_fullscreen', rejected unknown option keys and aligned test helpers and comments.

[AlexSkrypnyk]

Summary

ScreenshotContext::captureScreenshot() now reads the fullscreen flag from an is_fullscreen key and, before doing anything else, checks $config against the new public constant ScreenshotContext::CAPTURE_CONFIG_KEYS (filename, is_failed, is_fullscreen), throwing \InvalidArgumentException for any other key; afterStepCaptureFailedScreenshot(), afterStepCaptureScreenshot(), iSaveFullscreenScreenshot() and iSaveFullscreenScreenshotWithName() pass is_fullscreen, and ScreenshotAwareContextInterface::captureScreenshot() documents the new key and the new @throws.

Before this change the options array mixed the predicate is_failed with the bare fullscreen, and captureScreenshot() ignored any key it did not read, so a caller that misspelt fullscreen or passed an unsupported key such as mode got no error and a capture that ignored the key. ScreenshotConfig::createTypeException() also used the create*() prefix that the project reserves for collaborator factories.

Passing fullscreen, or any key outside filename, is_failed and is_fullscreen, now throws with the offending keys named in the message. The other src/ changes are the ScreenshotConfig::makeTypeException() rename and docblock corrections; everything else is under tests/, which is export-ignored from the package. Step texts, hook method names, configuration keys and environment variables are unchanged.

Before / After

BEFORE
  captureScreenshot(['fullscreen' => TRUE])      ──> fullscreen capture
  captureScreenshot(['mode' => 'wide'])          ──> key ignored, no error

AFTER
  captureScreenshot(['is_fullscreen' => TRUE])   ──> fullscreen capture
  captureScreenshot(['fullscreen' => TRUE])      ──> \InvalidArgumentException
  captureScreenshot(['mode' => 'wide'])          ──> \InvalidArgumentException

  "Unsupported screenshot configuration keys: fullscreen.
   Supported keys: filename, is_failed, is_fullscreen."

BEFORE
  ScreenshotConfigTest::collectLeafKeyPaths()        walks the config tree
  BehatDistConfigTest::getNodeOptionNames()          walks the config tree again
  AnimationArtifactsTest::createPage()               draws a ruled page
  AnimationAssemblyProfileTest::createPage()         draws a page of text rows

AFTER
  ScreenshotConfigTrait::collectLeafKeyPaths()
    ├─ ScreenshotConfigTest
    └─ BehatDistConfigTest
  PageImageTrait::createPage()                       draws a ruled page with text rows
    ├─ AnimationArtifactsTest
    └─ AnimationAssemblyProfileTest

Changes

Screenshot capture options (src/)

• ScreenshotContext::captureScreenshot() rejects any $config key outside ScreenshotContext::CAPTURE_CONFIG_KEYS with \InvalidArgumentException, checked before anything else in the method.
• afterStepCaptureFailedScreenshot(), afterStepCaptureScreenshot(), iSaveFullscreenScreenshot() and iSaveFullscreenScreenshotWithName() pass is_fullscreen instead of fullscreen.
• ScreenshotAwareContextInterface::captureScreenshot() documents is_fullscreen and the new @throws \InvalidArgumentException.

ScreenshotConfig helper naming (src/)

• ScreenshotConfig::createTypeException() is renamed makeTypeException(): create*() names the side-effect collaborator factories (createFilesystem(), createFinder(), createAnimatedGifEncoder()), and make*() names helpers that build values (makeFilename()).

Docblock and comment corrections (src/)

• The beforeScenarioInit() summary says it starts the driver and resizes the window, instead of "Init values required for screenshots."
• makeFilename() and makeAnimationFilename() no longer claim to return a unique filename; nothing enforces uniqueness when a pattern has no timestamp or step token.
• Three inline comments in ScreenshotContext reworded without changing their claims, and a docblock paragraph that restated the body of ScreenshotContextInitializer::applyEnvironmentOverrides() removed.

Shared test helpers (tests/)

• New tests/phpunit/Traits/PageImageTrait.php provides one createPage(int $width, int $height, string $title = '') for AnimationArtifactsTest and AnimationAssemblyProfileTest, replacing two different copies. The page keeps the header title, the 200px rulers with y labels and the right-edge band, and adds the text-like rows the profile test drew.
• ScreenshotConfigTrait::collectLeafKeyPaths() replaces ScreenshotConfigTest::collectLeafKeyPaths() and BehatDistConfigTest::getNodeOptionNames(); the settings walker in BehatDistConfigTest is renamed collectSettingKeyPaths().
• ScreenshotContextTest builds hook scopes through BehatScopeTrait instead of hand-built BeforeScenarioScope/AfterStepScope stubs; BehatDistConfigTest builds its configuration tree through ScreenshotConfigTrait::buildScreenshotConfigTree().

Naming and visibility (tests/)

• $passed is $is_passed in BehatScopeTrait::createAfterStepScope() and ScreenshotContextTest; $page_loaded/$image_supported are $is_page_loaded/$is_image_supported; $string_contains_tokens is $text_contains_tokens in TokenizerTest, matching the other test in that file and Tokenizer::replaceTokens(string $text, ...).
• BehatCliTrait::behatCliBeforeScenario() is behatCliBeforeScenarioWriteFeatureContext(), so every hook names its phase and its purpose.
• self:: replaces static:: for own constants and static helpers in BehatDistConfigTest, EnvironmentVariableNamingTest, CollaboratorCreationTest, EnvironmentVariableTraitTest and BehatCliTrait. The two late static binding sites in src/ stay.
• array<int,string> docblock generics in BehatScopeTrait and BehatDistConfigTest match the rest of the code.
• BehatCliTrait::behatCliWriteFeatureContextFile() and ScreenshotTrait::screenshotInitParams() are protected; they are neither steps nor hooks.
• The Behat bootstrap (ScreenshotTrait, FeatureContext, and the FeatureContextTest template inside BehatCliTrait) reads BEHAT_SCREENSHOT_DIR and BEHAT_SCREENSHOT_TOKEN_HOST through ScreenshotContextInitializer::ENV_DIR and ScreenshotContext::ENV_TOKEN_HOST, as the PHPUnit tests do.

Step definitions and feature files (tests/)

• File assertion steps in BehatCliTrait and ScreenshotTrait are declared with #[Then] instead of #[Given]. Behat matches step text regardless of the keyword.
• selenium.feature uses the save ... screenshot step aliases under Then/And, the keyword ScreenshotContext declares them with, instead of When. Step line numbers, and so the expected screenshot filenames, are unchanged.
• Two info_types scenario titles in screenshot_behatcli.feature describe the list they configure instead of 'true'/'false'.

Tests and comments (tests/)

• ScreenshotContextTest::testBehatRegistersHooksOnPhasePrefixedMethods() uses assertStringStartsWith() and fails on an empty phase name, which str_starts_with() accepted as a match.
• New ScreenshotContextTest::testCaptureScreenshotRejectsUnsupportedConfigKeys() covers an unsupported key, unsupported keys among supported ones and a positional value; it runs with no configuration set, so the guard must run first. The fullscreen expectations in ScreenshotContextTest and ScreenshotContextResizeTest use is_fullscreen.
• Comments that restated the code are removed from ScreenshotContextResizeTest, AnimatedGifEncoderTest, TokenizerTest and screenshot_behatcli.feature (the Chromium setup lines repeated CONTRIBUTING.md). The comments in BehatCliTrait::behatCliAssertFailWithError() and behatCliAssertFailWithException() describe what the assertions check: an assertion failure throws an exception class other than \RuntimeException.

Upgrade note

This PR belongs to the 3.0 release, which ships breaks without aliases.

• Loud: captureScreenshot(['fullscreen' => ...]) throws \InvalidArgumentException; pass is_fullscreen instead. Any other key outside filename, is_failed and is_fullscreen also throws.
• Silent: a subclass that overrides captureScreenshot() and reads $config['fullscreen'] no longer receives the flag, because the hooks and step methods pass is_fullscreen. Read $config['is_fullscreen'] instead.
• No change needed: the ScreenshotConfig::createTypeException() rename only affects code written against the unreleased 3.0 ScreenshotConfig. Step texts, hook method names, configuration keys and environment variables are unchanged.

Left unchanged on purpose

• The testIsave* test names in ScreenshotContextTest: the Drupal ValidFunctionName sniff checks camel caps in strict mode, which rejects two adjacent capitals, so testISave... fails composer lint.
• The hand-built TreeBuilder in the BehatScreenshotExtensionTest tests of configure(): configure() is the method under test there, so the call stays in the test body.
• tests/behat/bootstrap/BehatCliContext.php: an upstream copy excluded from phpcs and Rector.

Verification

• composer test: 334 tests, 746 assertions, green. The 14 PHPUnit notices come from the existing createPartialMock() doubles without expectations.
• composer lint: phpcs, PHPStan, Rector and gherkinlint clean.
• composer test-bdd -- --tags=~@selenium --tags=~@headless: 29 scenarios, 199 steps passed. @selenium and @headless run in CI; selenium.feature was also checked with behat --dry-run (48 steps, none undefined).
• composer profile with BEHAT_SCREENSHOT_PROFILE_STEPS=3,5: passed.
• Removing animation.max_height from behat.dist.php makes BehatDistConfigTest::testSetsEveryOption() fail with the missing key path in the diff.

[coderabbitai]

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: ee07d95d-a3b3-438f-b9a1-6ddc6d82a235

📥 Commits

Reviewing files that changed from the base of the PR and between 665aada and b89d6e8.

📒 Files selected for processing (23)

• src/DrevOps/BehatScreenshotExtension/Context/Initializer/ScreenshotContextInitializer.php
• src/DrevOps/BehatScreenshotExtension/Context/ScreenshotAwareContextInterface.php
• src/DrevOps/BehatScreenshotExtension/Context/ScreenshotContext.php
• src/DrevOps/BehatScreenshotExtension/ScreenshotConfig.php
• tests/behat/bootstrap/BehatCliTrait.php
• tests/behat/bootstrap/FeatureContext.php
• tests/behat/bootstrap/ScreenshotTrait.php
• tests/behat/features/screenshot_behatcli.feature
• tests/behat/features/selenium.feature
• tests/phpunit/Functional/AnimationArtifactsTest.php
• tests/phpunit/Profile/AnimationAssemblyProfileTest.php
• tests/phpunit/Traits/BehatScopeTrait.php
• tests/phpunit/Traits/PageImageTrait.php
• tests/phpunit/Traits/ScreenshotConfigTrait.php
• tests/phpunit/Unit/AnimatedGifEncoderTest.php
• tests/phpunit/Unit/BehatDistConfigTest.php
• tests/phpunit/Unit/CollaboratorCreationTest.php
• tests/phpunit/Unit/EnvironmentVariableNamingTest.php
• tests/phpunit/Unit/EnvironmentVariableTraitTest.php
• tests/phpunit/Unit/ScreenshotConfigTest.php
• tests/phpunit/Unit/ScreenshotContextResizeTest.php
• tests/phpunit/Unit/ScreenshotContextTest.php
• tests/phpunit/Unit/TokenizerTest.php

💤 Files with no reviewable changes (3)

• src/DrevOps/BehatScreenshotExtension/Context/Initializer/ScreenshotContextInitializer.php
• tests/phpunit/Unit/AnimatedGifEncoderTest.php
• tests/phpunit/Unit/ScreenshotConfigTest.php

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

---

📝 Walkthrough

Walkthrough

The change validates screenshot capture keys, renames the fullscreen option to is_fullscreen, updates Behat step wiring, centralizes PHPUnit helpers, and aligns related tests and documentation.

Changes

Screenshot capture contract

Layer / File(s)	Summary
Capture validation and option naming src/DrevOps/BehatScreenshotExtension/Context/..., src/DrevOps/BehatScreenshotExtension/ScreenshotConfig.php	captureScreenshot() now accepts only filename, is_failed, and is_fullscreen. Unsupported keys throw InvalidArgumentException. Internal type-exception helpers and fullscreen references were renamed.
Behat test wiring tests/behat/bootstrap/*, tests/behat/features/*	Behat contexts use shared constants, protected helpers, Then assertions, and updated screenshot scenario wording.
Shared PHPUnit helpers tests/phpunit/Traits/*, tests/phpunit/Functional/*, tests/phpunit/Profile/*	Synthetic page generation and configuration-tree traversal moved into reusable traits. Related tests now use those traits.
Capture validation tests tests/phpunit/Unit/ScreenshotContext*.php, tests/phpunit/Unit/BehatDistConfigTest.php	Tests use is_fullscreen, shared scope factories, and coverage for unsupported, mixed, and positional configuration keys.
Test consistency updates tests/phpunit/Unit/*, tests/phpunit/Traits/BehatScopeTrait.php	Tests use class-bound constants, clearer parameter names, and revised comments. Unused helpers and comments were removed.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to b89d6

The screenshot option rename, validation, and related test wiring show no actionable merge risk in the supplied evidence.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 57.35% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 68 functions across 18 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 identifies the primary changes: renaming the capture option to is_fullscreen and rejecting unknown option keys. It also accurately mentions the related test-helper and comment upda…

Full details: Docstring Coverage

Explanation

Docstring coverage is 57.35% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 68 functions across 18 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/improve-260916-1759

---

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

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.10%. Comparing base (665aada) to head (b89d6e8).
✅ All tests successful. No failed tests found.

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #307   +/-   ##
=======================================
  Coverage   99.09%   99.10%           
=======================================
  Files           6        6           
  Lines         442      445    +3     
=======================================
+ Hits          438      441    +3     
  Misses          4        4           

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