Fix Save Selenium Screenshot when a screenshot directory is configured

[eastagiletracker]

This PR proposes a fix for Save Selenium Screenshot, which silently takes no screenshot at all when a screenshot directory is configured. We include this PR work along with a full history of your repo at https://eastagiletracker.com/projects/271. You can sign in with your GitHub ID to claim ownership of the project.

The defect

GUILibrary imports with run_on_failure='Save Selenium Screenshot', and documents screenshot_root_directory as "Path to folder where possible screenshots are created or EMBED". The keyword handles the EMBED half of that sentence but not the other one:

if is_string(self.screenshot_root_directory):
    if self.screenshot_root_directory.upper() == 'EMBED':
        return self.capture_page_screenshot()
else:
    timestamp = time()
    filename = 'selenium-screenshot-' + str(timestamp) + '-' + str(next(SCREENSHOT_COUNTER)) + '.png'
    return self.capture_page_screenshot(filename)

When screenshot_root_directory is a directory path — set either on import or with Set Screenshot Directory — the outer branch is taken, the inner one is not, and the keyword falls off the end and returns None without ever calling capture_page_screenshot. Nothing raises, so a suite configured that way loses every failure screenshot without a word in the log. The existing unit tests do not catch it because they exercise only EMBED and a Mock root directory, both of which take the other path.

Reproducing it on master

At c509a1c (current master), against a real headless Chrome with screenshot_root_directory pointed at an empty directory:

import os
from Zoomba.GUILibrary import GUILibrary

os.makedirs("/tmp/zoomba-shots", exist_ok=True)
lib = GUILibrary(screenshot_root_directory="/tmp/zoomba-shots")
lib.open_browser("data:text/html,<h1>zoomba</h1>", "headlesschrome")
print("returned:", lib.save_selenium_screenshot())
print("files written:", os.listdir("/tmp/zoomba-shots"))
lib.close_all_browsers()

returned: None
files written: []

The same script against this branch:

returned: /tmp/zoomba-shots/selenium-screenshot-1786599916.9748402-0.png
files written: ['selenium-screenshot-1786599916.9748402-0.png']

The change

save_selenium_screenshot now intercepts only when the configured directory is an embed-style value, and otherwise always captures with the generated unique filename, leaving SeleniumLibrary to resolve the configured directory (which it already does in _get_screenshot_path). EMBED behaves exactly as before. BASE64 — SeleniumLibrary's other embed-style directory value since 6.8, and the repo pins 6.9.0 — is handled alongside EMBED rather than being written into a directory literally named BASE64. The public keyword name, its signature and its return value for every case that worked before are unchanged, so existing suites are unaffected.

Removing the nested if also retires the one call to robot.utils.is_string, which is deprecated for removal in Robot Framework 9.0 and was the only DeprecationWarning in your unit test output; isinstance(item, str) is that helper's own implementation.

Verification

pytest --cov-config=.coveragerc --cov=src on master before the change: 212 passed, 2 warnings, 100% coverage. On this branch: 217 passed, 0 warnings, 100% coverage — no new failures. Five unit tests are added in test/GUI/test_gui.py covering the configured-directory case, filename uniqueness across calls, BASE64, lowercase embed, and the unset default. Three of them fail against the unmodified GUILibrary.py and pass with it:

$ git checkout master -- src/Zoomba/GUILibrary.py && pytest test/GUI/test_gui.py -k save_selenium_screenshot
FAILED test/GUI/test_gui.py::TestInternal::test_save_selenium_screenshot_base64
FAILED test/GUI/test_gui.py::TestInternal::test_save_selenium_screenshot_directory
FAILED test/GUI/test_gui.py::TestInternal::test_save_selenium_screenshot_directory_filenames_are_unique
3 failed, 4 passed

The two passing ones are deliberate controls: EMBED and the unset directory already worked, and still do.

How this was managed

This work was tracked as https://eastagiletracker.com/projects/271/stories/160635 on a board built from this repository's own issues and pull requests (491 stories, 13 labels imported), which you can browse at https://eastagiletracker.com/projects/271.

If you'd rather not receive contributions like this, reply no-more-prs on this pull request and we won't open any further ones on your repositories.

---

Lawrence W. Sinclair
CEO / East Agile
linkedin.com/in/lwsinclair/
eastagile.com

[mergify]

Tick the box to add this pull request to the merge queue (same as @mergifyio queue).

• Queue this pull request

[coveralls]

coverage: 100.0%. remained the same — eastagiletracker:agile-board/save-selenium-screenshot-directory into Accruent:master