Skip to content

[#166] Fixed triggering a full screen screenshot in the test will not resize the window back to the original size.#170

Merged
AlexSkrypnyk merged 1 commit intomainfrom
feature/fix-full-screen-not-resizing-back
Oct 6, 2025
Merged

[#166] Fixed triggering a full screen screenshot in the test will not resize the window back to the original size.#170
AlexSkrypnyk merged 1 commit intomainfrom
feature/fix-full-screen-not-resizing-back

Conversation

@AlexSkrypnyk
Copy link
Copy Markdown
Member

@AlexSkrypnyk AlexSkrypnyk commented Oct 6, 2025

Summary by CodeRabbit

  • Bug Fixes

    • Improves fullscreen screenshot reliability by defaulting to sane window dimensions and safely reading actual sizes.
    • Adds a short delay after resizing to prevent rendering artifacts before capture.
    • Ensures the original window size is always restored, ignoring non-critical errors.
  • Tests

    • Updates unit tests to validate resize behavior, including successful fullscreen flow and handling of invalid dimensions without resizing.
    • Verifies correct sequencing and values for resize operations and script evaluations.

… resize the window back to the original size.
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Oct 6, 2025

📝 Walkthrough

Walkthrough

Updates fullscreen screenshot resizing logic to default and probe window dimensions, add a stabilization delay, and always attempt to restore the original size. Unit tests are revised to simulate sequential JS evaluations, verify resize sequencing and values, and cover invalid-dimension scenarios.

Changes

Cohort / File(s) Summary of Changes
Screenshot resizing logic
src/DrevOps/BehatScreenshotExtension/Context/ScreenshotContext.php
Defaults original size to 1440x900; attempts to read actual window dimensions via JS in try/catch; falls back to defaults on failure; adds small delay after resize; unconditionally attempts window size restoration with errors ignored.
Unit tests for resizing
tests/phpunit/Unit/ScreenshotContextResizeTest.php
Mocks sequential evaluateScript calls for window and document dimensions; verifies two-step resize (to fullscreen height, then restore original size); adds callback assertions for call order/values; updates invalid-dimensions test to avoid resize and still return result.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant Test as Behat Test
  participant Ctx as ScreenshotContext
  participant Sess as Mink Session
  participant Drv as WebDriver (JS Eval)

  Test->>Ctx: getScreenshotFullscreenWithResize()
  activate Ctx

  note over Ctx: Initialize default original size 1440x900
  Ctx->>Sess: evaluateScript(window.outerWidth/outerHeight)
  alt JS ok
    Sess-->>Ctx: numeric width/height
    note over Ctx: Set original size from JS
  else JS fails or non-numeric
    Sess-->>Ctx: error/invalid
    note over Ctx: Keep defaults
  end

  Ctx->>Sess: evaluateScript(document dimensions)
  Sess-->>Ctx: page width/height

  alt valid dimensions
    Ctx->>Sess: resizeWindow(pageWidth, pageHeight)
    note over Ctx: Short delay (usleep)
    Ctx->>Sess: capture screenshot
    Ctx-->>Test: screenshot data
    note over Ctx: Best-effort restore
    Ctx->>Sess: resizeWindow(originalWidth, originalHeight)
    Sess-->>Ctx: (ignored on error)
  else invalid dimensions
    Ctx->>Sess: capture screenshot (no resize)
    Ctx-->>Test: screenshot data
  end
  deactivate Ctx
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title clearly identifies the primary change—restoring window size after a full screen screenshot—and is fully related to the changeset.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feature/fix-full-screen-not-resizing-back

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 139260a and 99e8e1c.

📒 Files selected for processing (2)
  • src/DrevOps/BehatScreenshotExtension/Context/ScreenshotContext.php (2 hunks)
  • tests/phpunit/Unit/ScreenshotContextResizeTest.php (2 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.php

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.php: Follow Drupal coding standards in all PHP code
Use snake_case for variable names (e.g., $file_path, not $filePath)
Use TRUE and FALSE (uppercase) instead of true/false in PHP code
Use NULL (uppercase) instead of null in PHP code
Maintain proper docblock annotations for classes, methods, and functions
Declare class constants with explicit visibility (public) in line with PHP 8.2+ standards

Files:

  • tests/phpunit/Unit/ScreenshotContextResizeTest.php
  • src/DrevOps/BehatScreenshotExtension/Context/ScreenshotContext.php
{phpunit.xml,tests/**/*.php}

📄 CodeRabbit inference engine (CLAUDE.md)

Fix PHPUnit deprecations as they arise (in tests and configuration)

Files:

  • tests/phpunit/Unit/ScreenshotContextResizeTest.php
🧬 Code graph analysis (1)
src/DrevOps/BehatScreenshotExtension/Context/ScreenshotContext.php (1)
src/DrevOps/BehatScreenshotExtension/Context/ScreenshotAwareContextInterface.php (1)
  • screenshot (44-44)
🪛 PHPMD (2.15.0)
tests/phpunit/Unit/ScreenshotContextResizeTest.php

56-56: The variable $call_count is not named in camelCase. (undefined)

(CamelCaseVariableName)

src/DrevOps/BehatScreenshotExtension/Context/ScreenshotContext.php

441-441: The variable $original_width is not named in camelCase. (undefined)

(CamelCaseVariableName)


442-442: The variable $original_height is not named in camelCase. (undefined)

(CamelCaseVariableName)


446-446: The variable $original_dimensions is not named in camelCase. (undefined)

(CamelCaseVariableName)


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

@codecov
Copy link
Copy Markdown

codecov Bot commented Oct 6, 2025

Codecov Report

❌ Patch coverage is 84.61538% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 69.35%. Comparing base (139260a) to head (99e8e1c).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
...tScreenshotExtension/Context/ScreenshotContext.php 84.61% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #170      +/-   ##
==========================================
+ Coverage   68.24%   69.35%   +1.10%     
==========================================
  Files           4        4              
  Lines         381      385       +4     
==========================================
+ Hits          260      267       +7     
+ Misses        121      118       -3     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@AlexSkrypnyk AlexSkrypnyk merged commit 59faa49 into main Oct 6, 2025
7 checks passed
@AlexSkrypnyk AlexSkrypnyk deleted the feature/fix-full-screen-not-resizing-back branch October 6, 2025 04:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant