From b28fc400a09ebc31ccdcf5b08d2fa4a1b03665b0 Mon Sep 17 00:00:00 2001 From: Artem Klimov Date: Fri, 14 Mar 2025 12:22:10 +0200 Subject: [PATCH 1/9] Expand TestRunStatus enum: added autoApproved --- visual_regression_tracker/types.py | 1 + 1 file changed, 1 insertion(+) diff --git a/visual_regression_tracker/types.py b/visual_regression_tracker/types.py index f74334c..f6ee8ea 100644 --- a/visual_regression_tracker/types.py +++ b/visual_regression_tracker/types.py @@ -34,6 +34,7 @@ class TestRunStatus(enum.Enum): NEW = 'new' OK = 'ok' UNRESOLVED = 'unresolved' + AUTO_APPROVED = 'autoApproved' @dataclasses.dataclass From 822e63efe0ebde1c4f59258c84b5bb5d1ddeb880 Mon Sep 17 00:00:00 2001 From: "alexey.polovinkin" Date: Sat, 1 Jul 2023 18:35:20 +0300 Subject: [PATCH 2/9] Obtain TestRunResult at tracking with playwright --- visual_regression_tracker/playwright.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/visual_regression_tracker/playwright.py b/visual_regression_tracker/playwright.py index dfb6da6..21cc7e5 100644 --- a/visual_regression_tracker/playwright.py +++ b/visual_regression_tracker/playwright.py @@ -66,7 +66,7 @@ def trackPage( screenshot = page.screenshot(**screenshotOptions) imageBase64 = base64.b64encode(screenshot).decode('ascii') - self.track(TestRun( + return self.track(TestRun( name, imageBase64, options.agent.os if options and options.agent else None, @@ -89,7 +89,7 @@ async def trackPageAsync( screenshot = await page.screenshot(**screenshotOptions) imageBase64 = base64.b64encode(screenshot).decode('ascii') - self.track(TestRun( + return self.track(TestRun( name, imageBase64, options.agent.os if options and options.agent else None, @@ -110,7 +110,7 @@ def trackElementHandle( screenshot = elementHandle.screenshot(**screenshotOptions) imageBase64 = base64.b64encode(screenshot).decode('ascii') - self.track(TestRun( + return self.track(TestRun( name, imageBase64, options.agent.os if options and options.agent else None, @@ -131,7 +131,7 @@ async def trackElementHandleAsync( screenshot = await elementHandle.screenshot(**screenshotOptions) imageBase64 = base64.b64encode(screenshot).decode('ascii') - self.track(TestRun( + return self.track(TestRun( name, imageBase64, options.agent.os if options and options.agent else None, From 01ecdcbf12432a4de2eb72438bf374993143dd54 Mon Sep 17 00:00:00 2001 From: Pavlo Strunkin Date: Mon, 2 Nov 2026 13:25:46 +0200 Subject: [PATCH 3/9] feat!: compatibility with VRT 6.0 server - TestRunStatus knows all six server statuses; an unknown one maps to UNKNOWN with a warning instead of raising. Only new and unresolved fail a track, as in sdk-js. - start() returns the Build (#20). - Config strips a trailing slash from apiUrl (#25). - playwright.py uses typing.Literal, not the undeclared typing_extensions (#31). - vrt.json is read from the working directory, not next to __main__ (under the pytest entry point that was .venv/bin). - Every request has a timeout: httpTimeoutInSeconds, 30 s by default, also VRT_HTTPTIMEOUTINSECONDS and vrt.json. - HTTP errors raise ServerError(status_code, message) with the server's message, checked before the body is parsed; 413 and 422 carry a hint. - Image URLs are {apiUrl}/images/{name}, which the server redirects for both HDD and S3 storage. BREAKING CHANGE: ServerError takes (status_code, message); a 404 no longer always reads "Project not found"; image URLs gain /images/. Co-Authored-By: Claude Opus 5.5 --- tests/test_config.py | 78 +++++----- tests/test_playwright.py | 15 +- tests/test_playwright_async.py | 6 +- tests/test_types.py | 18 ++- tests/test_visualRegressionTracker.py | 141 ++++++++++++------ visual_regression_tracker/config.py | 29 ++-- visual_regression_tracker/exceptions.py | 9 +- visual_regression_tracker/playwright.py | 3 +- visual_regression_tracker/types.py | 15 +- .../visualRegressionTracker.py | 72 ++++----- 10 files changed, 239 insertions(+), 147 deletions(-) diff --git a/tests/test_config.py b/tests/test_config.py index 322d068..46f1054 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -1,12 +1,11 @@ import json -import os -import pytest -import pathlib -import subprocess import sys +import types + +import pytest from visual_regression_tracker import Config, MissingConfigurationError -from visual_regression_tracker.config import determine_config_path, ENV_MAPPING +from visual_regression_tracker.config import ENV_MAPPING @pytest.fixture @@ -19,6 +18,7 @@ def config_file(tmpdir): 'project': 'file project', 'apiKey': 'file api key', 'enableSoftAssert': True, + 'httpTimeoutInSeconds': 5, })) yield p @@ -32,6 +32,7 @@ def config_env(): 'VRT_PROJECT': 'env project', 'VRT_APIKEY': 'env api key', 'VRT_ENABLESOFTASSERT': 'False', + 'VRT_HTTPTIMEOUTINSECONDS': '7.5', } yield env @@ -44,6 +45,7 @@ def test_config_from_file(config_file): assert cfg.project == 'file project' assert cfg.apiKey == 'file api key' assert cfg.enableSoftAssert == True + assert cfg.httpTimeoutInSeconds == 5 def test_config_from_environment(config_env): @@ -54,6 +56,25 @@ def test_config_from_environment(config_env): assert cfg.project == 'env project' assert cfg.apiKey == 'env api key' assert cfg.enableSoftAssert == False + assert cfg.httpTimeoutInSeconds == 7.5 + + +def test_config_timeout_defaults_to_30_seconds(): + assert Config().httpTimeoutInSeconds == 30 + + +@pytest.mark.parametrize('make_config', [ + lambda: Config(apiUrl='http://vrt:4200/'), + lambda: Config.from_environment({'VRT_APIURL': 'http://vrt:4200/'}), +]) +def test_config_strips_trailing_slash_from_apiUrl(make_config): + assert make_config().apiUrl == 'http://vrt:4200' + + +def test_config_from_file_strips_trailing_slash_from_apiUrl(tmp_path): + path = tmp_path / 'vrt.json' + path.write_text(json.dumps({'apiUrl': 'http://vrt:4200/'})) + assert Config.from_file(path).apiUrl == 'http://vrt:4200' def test_default_uses_path(config_file): @@ -96,10 +117,12 @@ def test_default_uses_defaults(config_env, mocker): mocker.patch('os.path.isfile').return_value = False del config_env['VRT_CIBUILDID'] del config_env['VRT_APIURL'] + del config_env['VRT_HTTPTIMEOUTINSECONDS'] cfg = Config.default(environment=config_env) assert cfg.apiUrl == Config.apiUrl assert cfg.ciBuildId is None + assert cfg.httpTimeoutInSeconds == 30 def test_default_prefers_environment(config_env, config_file): @@ -123,36 +146,19 @@ def test_default_raises_on_missing_settings(mocker, config_env, missing_field): Config.default(environment=config_env) -def test_determine_config_path_from_pytest(): - current_file = pathlib.Path(__file__) - sdk_python_dir = str(current_file.parent.parent) - assert determine_config_path() == sdk_python_dir - - -@pytest.fixture -def change_current_dir(tmpdir): - olddir = os.curdir - os.chdir(os.path.expanduser('~')) - yield os.path.abspath(os.curdir) - os.chdir(olddir) - - -def test_detemine_config_path_from_repl(change_current_dir): - current_file = pathlib.Path(__file__) - sdk_python_dir = str(current_file.parent.parent) - - output = subprocess.check_output( - sys.executable, - encoding='ascii', - input=f''' -import sys -sys.path.insert(0, '{sdk_python_dir}') +def test_default_reads_vrt_json_from_working_directory_when_run_by_a_script(tmp_path, monkeypatch): + (tmp_path / 'vrt.json').write_text(json.dumps({ + 'apiUrl': 'http://vrt:4200', + 'branchName': 'develop', + 'project': 'project', + 'apiKey': 'key', + })) + script = types.ModuleType('__main__') + script.__file__ = '/venv/bin/pytest' + script.__spec__ = None + monkeypatch.setitem(sys.modules, '__main__', script) + monkeypatch.chdir(tmp_path) -from visual_regression_tracker.config import determine_config_path -print(determine_config_path()) -quit() -''' - ) - result = output.strip() - assert result == change_current_dir + cfg = Config.default(environment={}) + assert cfg.project == 'project' diff --git a/tests/test_playwright.py b/tests/test_playwright.py index 1f4a49a..0801f3b 100644 --- a/tests/test_playwright.py +++ b/tests/test_playwright.py @@ -1,6 +1,8 @@ import pytest import base64 import binascii +import subprocess +import sys import visual_regression_tracker playwright = pytest.importorskip('playwright') import visual_regression_tracker.playwright @@ -24,6 +26,13 @@ def pvrt(sync_browserType, mocker): return pvrt +def test_playwright_module_imports_without_typing_extensions(): + subprocess.run([ + sys.executable, '-c', + "import sys; sys.modules['typing_extensions'] = None; import visual_regression_tracker.playwright", + ], check=True) + + def test_pvrt_extends_vrt(sync_browserType): pvrt = visual_regression_tracker.playwright.PlaywrightVisualRegressionTracker(sync_browserType, CONFIG) assert isinstance(pvrt, visual_regression_tracker.VisualRegressionTracker) @@ -78,9 +87,10 @@ def sync_page(sync_browserType): def test_pvrt_trackPage__default_options(pvrt, sync_page, sync_browserType): - pvrt.trackPage(sync_page, 'image name') + result = pvrt.trackPage(sync_page, 'image name') pvrt.track.assert_called_once() + assert result is pvrt.track.return_value testRun = pvrt.track.call_args[0][0] assert testRun.name == 'image name' @@ -139,9 +149,10 @@ def test_pvrt_trackElementHandle__throws_if_no_page(pvrt): def test_pvrt_trackElementHandle__default_options(pvrt, sync_elementHandle, sync_browserType): - pvrt.trackElementHandle(sync_elementHandle, 'image name') + result = pvrt.trackElementHandle(sync_elementHandle, 'image name') pvrt.track.assert_called_once() + assert result is pvrt.track.return_value testRun = pvrt.track.call_args[0][0] assert testRun.name == 'image name' diff --git a/tests/test_playwright_async.py b/tests/test_playwright_async.py index 3fb05d5..8984a03 100644 --- a/tests/test_playwright_async.py +++ b/tests/test_playwright_async.py @@ -61,9 +61,10 @@ async def test_pvrt_trackPageAsync__throws_if_no_page(pvrt): @pytest.mark.asyncio(loop_scope='session') async def test_pvrt_trackPageAsync__default_options(pvrt, async_page, async_browserType): - await pvrt.trackPageAsync(async_page, 'image name') + result = await pvrt.trackPageAsync(async_page, 'image name') pvrt.track.assert_called_once() + assert result is pvrt.track.return_value testRun = pvrt.track.call_args[0][0] assert testRun.name == 'image name' @@ -125,9 +126,10 @@ async def test_pvrt_trackElementHandleAsync__throws_if_no_page(pvrt): @pytest.mark.asyncio(loop_scope='session') async def test_pvrt_trackElementHandleAsync__default_options(pvrt, async_elementHandle, async_browserType): - await pvrt.trackElementHandleAsync(async_elementHandle, 'image name') + result = await pvrt.trackElementHandleAsync(async_elementHandle, 'image name') pvrt.track.assert_called_once() + assert result is pvrt.track.return_value testRun = pvrt.track.call_args[0][0] assert testRun.name == 'image name' diff --git a/tests/test_types.py b/tests/test_types.py index 1dd3077..21bed55 100644 --- a/tests/test_types.py +++ b/tests/test_types.py @@ -85,9 +85,9 @@ def test__TestRunResult_all_images(): result = TestRunResult(test_run_response, "http://localhost") assert result.testRunResponse == test_run_response - assert result.imageUrl == "http://localhost/imageName" - assert result.diffUrl == "http://localhost/diffName" - assert result.baselineUrl == "http://localhost/baselineName" + assert result.imageUrl == "http://localhost/images/imageName" + assert result.diffUrl == "http://localhost/images/diffName" + assert result.baselineUrl == "http://localhost/images/baselineName" def test__TestRunResult_only_required_images(): @@ -100,6 +100,16 @@ def test__TestRunResult_only_required_images(): result = TestRunResult(test_run_response, "http://localhost") assert result.testRunResponse == test_run_response - assert result.imageUrl == "http://localhost/imageName" + assert result.imageUrl == "http://localhost/images/imageName" assert result.diffUrl is None assert result.baselineUrl is None + + +@pytest.mark.parametrize('value', ['failed', 'new', 'ok', 'unresolved', 'approved', 'autoApproved']) +def test__TestRunStatus_knows_every_server_status(value): + assert TestRunStatus(value).value == value + + +def test__TestRunStatus_maps_unknown_status_to_UNKNOWN(caplog): + assert TestRunStatus('someFutureStatus') is TestRunStatus.UNKNOWN + assert 'someFutureStatus' in caplog.text diff --git a/tests/test_visualRegressionTracker.py b/tests/test_visualRegressionTracker.py index 8fd977f..3c95be4 100644 --- a/tests/test_visualRegressionTracker.py +++ b/tests/test_visualRegressionTracker.py @@ -1,11 +1,10 @@ -import json import logging import re import pytest from visual_regression_tracker import \ - Config, IgnoreArea, VisualRegressionTracker, \ + Build, Config, IgnoreArea, VisualRegressionTracker, \ TestRun, TestRunResponse, TestRunStatus, \ ServerError, TestRunError, VisualRegressionTrackerError from visual_regression_tracker.types import \ @@ -20,7 +19,12 @@ project='Default project', apiKey='CPKVK4JNK24NVNPNGVFQ853HXXEG', enableSoftAssert=False, + httpTimeoutInSeconds=12, ) +HEADERS = { + 'apiKey': CONFIG.apiKey, + 'project': CONFIG.project +} @pytest.fixture @@ -75,9 +79,34 @@ def test__track__should_track_success(vrt, mocker): vrt._submitTestResult.assert_called_once_with(test_run) assert test_run_result.testRunResponse == test_run_response - assert test_run_result.imageUrl == f'{CONFIG.apiUrl}/{test_run_response.imageName}' - assert test_run_result.diffUrl == f'{CONFIG.apiUrl}/{test_run_response.diffName}' - assert test_run_result.baselineUrl == f'{CONFIG.apiUrl}/{test_run_response.baselineName}' + assert test_run_result.imageUrl == f'{CONFIG.apiUrl}/images/{test_run_response.imageName}' + assert test_run_result.diffUrl == f'{CONFIG.apiUrl}/images/{test_run_response.diffName}' + assert test_run_result.baselineUrl == f'{CONFIG.apiUrl}/images/{test_run_response.baselineName}' + + +@pytest.mark.parametrize('status', [ + TestRunStatus.OK, + TestRunStatus.APPROVED, + TestRunStatus.AUTO_APPROVED, + TestRunStatus.FAILED, + TestRunStatus.UNKNOWN, +]) +def test__track__should_pass_statuses_other_than_new_and_unresolved(status, vrt, mocker, caplog): + vrt._submitTestResult = mocker.Mock(return_value=TestRunResponse(url='url', status=status, imageName='imageName')) + + vrt.track(TestRun()) + + assert caplog.record_tuples == [] + + +def test__track__should_accept_autoApproved_from_server(vrt, mock_request): + vrt.buildId = '1312' + vrt.projectId = 'asd' + mock_request.return_value = {'url': 'url', 'status': 'autoApproved', 'imageName': 'imageName'} + + result = vrt.track(TestRun(name='name')) + + assert result.testRunResponse.status == TestRunStatus.AUTO_APPROVED track_test_data = [ @@ -132,7 +161,7 @@ def test__start__should_start_build(vrt, mock_request): projectId = 'asd' mock_request.return_value = {'id': buildId, 'projectId': projectId} - vrt.start() + build = vrt.start() mock_request.assert_called_once_with( f'{CONFIG.apiUrl}/builds', @@ -142,16 +171,24 @@ def test__start__should_start_build(vrt, mock_request): 'branchName': CONFIG.branchName, 'project': CONFIG.project }, - { - 'apiKey': CONFIG.apiKey, - 'project': CONFIG.project - }, + HEADERS, + CONFIG.httpTimeoutInSeconds, ) + assert build == Build(buildId, projectId) assert vrt.buildId == buildId assert vrt.projectId == projectId +def test__start__should_not_double_the_slash_after_apiUrl(mock_request): + vrt = VisualRegressionTracker(Config(**{**vars(CONFIG), 'apiUrl': 'http://localhost:4200/'})) + mock_request.return_value = {'id': '1312', 'projectId': 'asd'} + + vrt.start() + + assert mock_request.call_args[0][0] == 'http://localhost:4200/builds' + + def test__stop__should_stop_build(vrt, mock_request, mocker): buildId = '1312' vrt.buildId = buildId @@ -163,11 +200,9 @@ def test__stop__should_stop_build(vrt, mock_request, mocker): mock_request.assert_called_once_with( f'{CONFIG.apiUrl}/builds/{buildId}', 'patch', - data={'isRunning': False}, - headers={ - 'apiKey': CONFIG.apiKey, - 'project': CONFIG.project - }, + {'isRunning': False}, + HEADERS, + CONFIG.httpTimeoutInSeconds, ) assert vrt.buildId is None assert vrt.projectId is None @@ -255,10 +290,8 @@ def test__submitTestResults__should_submit_test_run(vrt, mock_request): } ] }, - { - 'apiKey': CONFIG.apiKey, - 'project': CONFIG.project - }, + HEADERS, + CONFIG.httpTimeoutInSeconds, ) @@ -278,36 +311,60 @@ def test__submitTestResults__should_throw_not_started(vrt, mocker): vrt._submitTestResult(testRun) +def mock_response(mocker, status_code, body=None, text='', reason=''): + response = mocker.Mock() + response.status_code = status_code + response.text = text + response.reason = reason + if body is None: + response.json.side_effect = ValueError('Expecting value') + else: + response.json.return_value = body + return response + + def test__http_request__success(mocker): expected = {'a': 'b', 'c': 'd'} + post = mocker.patch('requests.post', return_value=mock_response(mocker, 200, expected)) - post = mocker.patch('requests.post') - response = post.return_value = mocker.Mock() - response.status_code = 200 - response.json.return_value = expected - - actual = _http_request('url', 'post', {'1': 2}, {2: '3'}) + actual = _http_request('url', 'post', {'1': 2}, {2: '3'}, 7) - post.assert_called_once_with('url', json={'1': 2}, headers={2: '3'}) + post.assert_called_once_with('url', json={'1': 2}, headers={2: '3'}, timeout=7) assert actual == expected -@pytest.mark.parametrize('status_code,response_body,expected_match', [ - (401, {}, 'Unauthorized'), - (403, {}, 'Api key not authenticated'), - (404, {}, 'Project not found'), - (500, '{"message": "exception message"}', json.dumps( - '{"message": "exception message"}', indent=2)), +@pytest.mark.parametrize('status_code,body,text,reason,expected_message', [ + (401, {'statusCode': 401, 'message': 'Unauthorized'}, '', '', 'Unauthorized'), + (403, {'message': 'You do not have permission to perform this operation.'}, '', '', + 'You do not have permission to perform this operation.'), + (404, {'statusCode': 404, 'path': '/builds/1', 'name': 'NotFoundException', 'message': 'Build not found'}, '', '', + 'Build not found'), + (400, {'statusCode': 400, 'message': ['name must be a string', 'branchName must be a string']}, '', '', + 'name must be a string, branchName must be a string'), + (502, None, 'Bad Gateway', 'Bad Gateway', 'Bad Gateway'), + (503, {}, '', 'Service Unavailable', 'Service Unavailable'), + (500, {'error': 'no message field'}, '{"error": "no message field"}', '', '{"error": "no message field"}'), ]) -def test__http_request__maps_status_code( - mocker, status_code, response_body, expected_match): - post = mocker.patch('requests.post') - response = post.return_value = mocker.Mock() - response.status_code = status_code - response.json = mocker.Mock(return_value=response_body) - response.raise_for_status.side_effect = Exception('oh no') +def test__http_request__raises_server_error_with_server_message( + mocker, status_code, body, text, reason, expected_message): + mocker.patch('requests.post', return_value=mock_response(mocker, status_code, body, text, reason)) + + with pytest.raises(ServerError) as error: + _http_request('url', 'post', {}, {}, 30) + + assert error.value.status_code == status_code + assert error.value.message == expected_message + assert str(error.value) == f'HTTP {status_code}: {expected_message}' + + +@pytest.mark.parametrize('status_code,body,hint', [ + (413, None, 'BODY_PARSER_JSON_LIMIT'), + (422, {'message': 'Image is 20000x20000 (400000000 pixels), above MAX_IMAGE_PIXELS=100000000'}, 'MAX_IMAGE_PIXELS'), +]) +def test__http_request__adds_hint_for_image_limits(mocker, status_code, body, hint): + mocker.patch('requests.post', return_value=mock_response(mocker, status_code, body, 'request entity too large')) - with pytest.raises(ServerError, match=re.escape(expected_match)): - _http_request('url', 'post', {'1': 2}, {2: '3'}) + with pytest.raises(ServerError, match=re.escape(hint)) as error: + _http_request('url', 'post', {}, {}, 30) - post.assert_called_once_with('url', json={'1': 2}, headers={2: '3'}) + assert error.value.status_code == status_code diff --git a/visual_regression_tracker/config.py b/visual_regression_tracker/config.py index f99ceeb..9baf572 100644 --- a/visual_regression_tracker/config.py +++ b/visual_regression_tracker/config.py @@ -4,7 +4,6 @@ import os.path import json import pathlib -import sys from .exceptions import MissingConfigurationError from .types import _from_dict @@ -19,6 +18,7 @@ 'project': 'VRT_PROJECT', 'apiKey': 'VRT_APIKEY', 'enableSoftAssert': 'VRT_ENABLESOFTASSERT', + 'httpTimeoutInSeconds': 'VRT_HTTPTIMEOUTINSECONDS', } @@ -30,18 +30,22 @@ class Config: project: str = None apiKey: str = None enableSoftAssert: bool = False + httpTimeoutInSeconds: float = 30 + + def __setattr__(self, name, value): + if name == 'apiUrl' and isinstance(value, str): + value = value.rstrip('/') + super().__setattr__(name, value) @staticmethod def default( path: typing.Union[str, pathlib.Path] = None, environment: dict = None, ): - default_path = os.path.join(determine_config_path(), DEFAULT_PATH) - if path: cfg = Config.from_file(path) - elif os.path.isfile(default_path): - cfg = Config.from_file(default_path) + elif os.path.isfile(DEFAULT_PATH): + cfg = Config.from_file(DEFAULT_PATH) else: cfg = Config() @@ -75,6 +79,8 @@ def apply_environment(self, environment: dict = None): pass elif field_type is bool: val = val.lower() in ('true', '1') + elif field_type is float: + val = float(val) else: raise Exception('Unsupported type') setattr(self, field_name, val) @@ -90,16 +96,3 @@ def check_complete(self): if getattr(self, field_name) is None: raise MissingConfigurationError(field_name, f'{field_name} is not specified.') - -def determine_config_path(): - """ - Determines the absolute path to the directory the current project is in: - 1. If in a script, then use the directory the script is in. - 2. If in a REPL, then use PWD. - 3. If running an installed module (ours or 3rd-party), then use PWD. - """ - main_module = sys.modules['__main__'].__dict__ - is_script = '__file__' in main_module and main_module['__spec__'] is None - rel_dir = os.path.dirname(main_module['__file__']) if is_script else os.curdir - abs_dir = os.path.abspath(rel_dir) - return abs_dir diff --git a/visual_regression_tracker/exceptions.py b/visual_regression_tracker/exceptions.py index b50626d..1087a82 100644 --- a/visual_regression_tracker/exceptions.py +++ b/visual_regression_tracker/exceptions.py @@ -8,11 +8,12 @@ class VisualRegressionTrackerError(Exception): class ServerError(VisualRegressionTrackerError): - """An error occurred on the VisualRegressionTracker server.""" + """The VisualRegressionTracker server answered with an HTTP error.""" - def __init__(self, *args): - """Initialises ServerError from server response.""" - super(ServerError, self).__init__(*args) + def __init__(self, status_code: int, message: str): + self.status_code = status_code + self.message = message + super(ServerError, self).__init__(f'HTTP {status_code}: {message}') class TestRunError(VisualRegressionTrackerError): diff --git a/visual_regression_tracker/playwright.py b/visual_regression_tracker/playwright.py index 21cc7e5..476fb68 100644 --- a/visual_regression_tracker/playwright.py +++ b/visual_regression_tracker/playwright.py @@ -2,8 +2,7 @@ import dataclasses import pathlib -from typing import Union, List -from typing_extensions import Literal +from typing import Union, List, Literal from playwright.sync_api import FloatRect from playwright import sync_api from playwright import async_api diff --git a/visual_regression_tracker/types.py b/visual_regression_tracker/types.py index f6ee8ea..3dd1f28 100644 --- a/visual_regression_tracker/types.py +++ b/visual_regression_tracker/types.py @@ -1,5 +1,6 @@ import dataclasses import enum +import logging import typing import dacite @@ -31,10 +32,18 @@ class Build: class TestRunStatus(enum.Enum): + FAILED = 'failed' NEW = 'new' OK = 'ok' UNRESOLVED = 'unresolved' + APPROVED = 'approved' AUTO_APPROVED = 'autoApproved' + UNKNOWN = 'unknown' + + @classmethod + def _missing_(cls, value): + logging.getLogger(__name__).warning('Unknown test run status: %r', value) + return cls.UNKNOWN @dataclasses.dataclass @@ -66,9 +75,9 @@ def __init__(self, test_run_response: TestRunResponse, api_url: str): :param api_url: URL to use in image urls """ self.testRunResponse = test_run_response - self.imageUrl = f'{api_url}/{test_run_response.imageName}' - self.diffUrl = test_run_response.diffName and f'{api_url}/{test_run_response.diffName}' - self.baselineUrl = test_run_response.baselineName and f'{api_url}/{test_run_response.baselineName}' + self.imageUrl = f'{api_url}/images/{test_run_response.imageName}' + self.diffUrl = test_run_response.diffName and f'{api_url}/images/{test_run_response.diffName}' + self.baselineUrl = test_run_response.baselineName and f'{api_url}/images/{test_run_response.baselineName}' def _to_dict(obj): diff --git a/visual_regression_tracker/visualRegressionTracker.py b/visual_regression_tracker/visualRegressionTracker.py index bcaeadb..c2e9d8a 100644 --- a/visual_regression_tracker/visualRegressionTracker.py +++ b/visual_regression_tracker/visualRegressionTracker.py @@ -1,4 +1,3 @@ -import json import logging import requests @@ -33,33 +32,32 @@ def __init__(self, config: Config = None): def _isStarted(self): return self.buildId is not None and self.projectId is not None - def start(self): + def _request(self, method: str, path: str, data: dict) -> dict: + return _http_request( + f'{self.config.apiUrl}/{path}', + method, + data, + self.headers, + self.config.httpTimeoutInSeconds, + ) + + def start(self) -> Build: data = { 'ciBuildId': self.config.ciBuildId, 'branchName': self.config.branchName, 'project': self.config.project, } - result = _http_request( - f'{self.config.apiUrl}/builds', - 'post', - data, - self.headers - ) - build = _from_dict(result, Build) + build = _from_dict(self._request('post', 'builds', data), Build) self.buildId = build.id self.projectId = build.projectId + return build def stop(self): if not self._isStarted(): raise VisualRegressionTrackerError( "Visual Regression Tracker has not been started") - _http_request( - f'{self.config.apiUrl}/builds/{self.buildId}', - 'patch', - data={'isRunning': False}, - headers=self.headers - ) + self._request('patch', f'builds/{self.buildId}', {'isRunning': False}) self.buildId = None self.projectId = None @@ -84,18 +82,13 @@ def _submitTestResult(self, test: TestRun) -> TestRunResponse: branchName=self.config.branchName, ) - result = _http_request( - f'{self.config.apiUrl}/test-runs', - 'post', - data, - self.headers - ) + result = self._request('post', 'test-runs', data) result['status'] = TestRunStatus(result['status']) testRunResult = _from_dict(result, TestRunResponse) return testRunResult - def track(self, test: TestRun): + def track(self, test: TestRun) -> TestRunResult: result = self._submitTestResult(test) switcher = { @@ -113,19 +106,30 @@ def track(self, test: TestRun): return TestRunResult(result, self.config.apiUrl) -def _http_request(url: str, method: str, data: dict, headers: dict) -> dict: +_HINTS = { + 413: 'the request is larger than BODY_PARSER_JSON_LIMIT on the server; raise it or send a smaller screenshot', + 422: 'the image has more pixels than MAX_IMAGE_PIXELS on the server; raise it or send a smaller screenshot', +} + + +def _http_request(url: str, method: str, data: dict, headers: dict, timeout: float) -> dict: request = getattr(requests, method.lower()) - response = request(url, json=data, headers=headers) + response = request(url, json=data, headers=headers, timeout=timeout) status = response.status_code - result = response.json() - - if status == 401: - raise ServerError('Unauthorized') - if status == 403: - raise ServerError('Api key not authenticated') - if status == 404: - raise ServerError('Project not found') + if status >= 400: - raise ServerError(json.dumps(result, indent=2)) + message = _error_message(response) + hint = _HINTS.get(status) + raise ServerError(status, f'{message} (hint: {hint})' if hint else message) + + return response.json() + - return result +def _error_message(response) -> str: + try: + message = response.json().get('message') + except (ValueError, AttributeError): + message = None + if isinstance(message, list): + message = ', '.join(message) + return message or response.text.strip() or response.reason or 'no error message' From f84d6cace397af84d033dba1817a658b627f2987 Mon Sep 17 00:00:00 2001 From: Pavlo Strunkin Date: Mon, 2 Nov 2026 13:27:38 +0200 Subject: [PATCH 4/9] build: release 5.0.0 from pyproject.toml via PyPI trusted publishing - Packaging moves from setup.py/setup.cfg to pyproject.toml; version 5.0.0, requires-python >=3.10, current dependency floors. - CI matrix is 3.10-3.14; actions bumped. - release.yml builds on a version tag, checks the tag matches the package version and publishes with pypa/gh-action-pypi-publish (OIDC). - README: working Playwright imports and links, new_page, timeout option, track results, statuses and errors. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/release.yml | 45 +++++++++++++++++++++++++++++++ .github/workflows/workflow.yml | 6 ++--- README.md | 48 ++++++++++++++++++++++++---------- pyproject.toml | 41 +++++++++++++++++++++++++++++ requirements.txt | 15 ++++------- setup.cfg | 9 ------- setup.py | 23 ---------------- tox.ini | 4 +-- 8 files changed, 130 insertions(+), 61 deletions(-) create mode 100644 .github/workflows/release.yml create mode 100644 pyproject.toml delete mode 100644 setup.cfg delete mode 100644 setup.py diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml new file mode 100644 index 0000000..8f2a2e9 --- /dev/null +++ b/.github/workflows/release.yml @@ -0,0 +1,45 @@ +name: Release + +on: + push: + tags: ['[0-9]+.[0-9]+.[0-9]+*'] + +permissions: + contents: read + +jobs: + build: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v7 + - uses: actions/setup-python@v7 + with: + python-version: '3.14' + - name: Check the tag matches the package version + run: | + version=$(python -c 'import tomllib; print(tomllib.load(open("pyproject.toml", "rb"))["project"]["version"])') + if [ "$version" != "$GITHUB_REF_NAME" ]; then + echo "::error::Tag $GITHUB_REF_NAME does not match version $version in pyproject.toml" + exit 1 + fi + - name: Build + run: pip install build && python -m build + - uses: actions/upload-artifact@v7 + with: + name: dist + path: dist/ + + publish: + needs: build + runs-on: ubuntu-latest + environment: + name: pypi + url: https://pypi.org/project/visual-regression-tracker/${{ github.ref_name }}/ + permissions: + id-token: write + steps: + - uses: actions/download-artifact@v8 + with: + name: dist + path: dist/ + - uses: pypa/gh-action-pypi-publish@release/v1 diff --git a/.github/workflows/workflow.yml b/.github/workflows/workflow.yml index 72e2aca..fe0f948 100644 --- a/.github/workflows/workflow.yml +++ b/.github/workflows/workflow.yml @@ -13,12 +13,12 @@ jobs: strategy: fail-fast: false matrix: - python-version: ['3.9', '3.10', '3.11', '3.12', '3.13', '3.14'] + python-version: ['3.10', '3.11', '3.12', '3.13', '3.14'] steps: - - uses: actions/checkout@v5 + - uses: actions/checkout@v7 - name: Set up Python ${{ matrix.python-version }} - uses: actions/setup-python@v6 + uses: actions/setup-python@v7 with: python-version: ${{ matrix.python-version }} - name: Install Tox diff --git a/README.md b/README.md index 6c1c555..02778d1 100644 --- a/README.md +++ b/README.md @@ -3,9 +3,11 @@ [![Codacy Badge](https://app.codacy.com/project/badge/Grade/0b98190cea064d6f9f1210da653ba37b)](https://www.codacy.com/gh/Visual-Regression-Tracker/sdk-python?utm_source=github.com&utm_medium=referral&utm_content=Visual-Regression-Tracker/sdk-python&utm_campaign=Badge_Grade) [![Codacy Badge](https://app.codacy.com/project/badge/Coverage/0b98190cea064d6f9f1210da653ba37b)](https://www.codacy.com/gh/Visual-Regression-Tracker/sdk-python?utm_source=github.com&utm_medium=referral&utm_content=Visual-Regression-Tracker/sdk-python&utm_campaign=Badge_Coverage) +Requires Python 3.10 or later. Image URLs in results need VRT server 5.0.10 or later; use 5.x of this SDK with VRT 6.0. + ## Install -```python +```sh pip install visual-regression-tracker # or, with playwright integration @@ -18,7 +20,7 @@ python -m playwright install ### Import ```python -from visual_regression_tracker import VisualRegressionTracker, Config, TestRun +from visual_regression_tracker import VisualRegressionTracker, Config, TestRun, IgnoreArea ``` ### Configure connection @@ -43,6 +45,9 @@ config = Config( # enableSoftAssert - Log errors instead of exceptions enableSoftAssert=False, + + # httpTimeoutInSeconds - Timeout of each request to the server + httpTimeoutInSeconds=30, ) vrt = VisualRegressionTracker(config) @@ -56,9 +61,12 @@ vrt = VisualRegressionTracker(config) "apiKey":"tXZVHX0EA4YQM1MGDD", "ciBuildId":"commit_sha", "branchName":"develop", - "enableSoftAssert":false + "enableSoftAssert":false, + "httpTimeoutInSeconds":30 } ``` + +`vrt.json` is read from the current working directory. ```python vrt = VisualRegressionTracker() ``` @@ -71,6 +79,7 @@ VRT_APIKEY="tXZVHX0EA4YQM1MGDD" \ VRT_CIBUILDID="commit_sha" \ VRT_BRANCHNAME="develop" \ VRT_ENABLESOFTASSERT=true \ +VRT_HTTPTIMEOUTINSECONDS=30 \ python ``` ```python @@ -89,7 +98,7 @@ with vrt: Without context manager: ```python -vrt.start() +build = vrt.start() # Build(id, projectId) ... # track test runs ... @@ -99,7 +108,7 @@ vrt.stop() ### Assert ```python -vrt.track(TestRun( +result = vrt.track(TestRun( # Name to be displayed # Required name='Image name', @@ -145,13 +154,22 @@ vrt.track(TestRun( )) ``` +`track` returns a `TestRunResult`: `testRunResponse` (with `status` and the `url` of the test run in the +VRT app) and `imageUrl`, `diffUrl`, `baselineUrl`. + +`track` raises `TestRunError` when the status is `new` (no baseline yet) or `unresolved` (difference found); +with `enableSoftAssert` it logs the error instead. `ok`, `approved` and `autoApproved` pass; `failed` passes +too, as in the JavaScript SDK. A status this SDK does not know becomes `TestRunStatus.UNKNOWN`. + +An HTTP error from the server raises `ServerError` with `status_code` and the server's `message`. + ### Integration with Microsoft Playwright #### Imports ```python -from playwright import sync_playwright -from visual_regression_tracker import Config, TestRun -from visual_regression_tracker.p import PlaywrightVisualRegressionTracker +from playwright.sync_api import sync_playwright +from visual_regression_tracker import Config +from visual_regression_tracker.playwright import PlaywrightVisualRegressionTracker ``` #### Start playwright and navigate to page @@ -159,7 +177,7 @@ from visual_regression_tracker.p import PlaywrightVisualRegressionTracker playwright = sync_playwright().start() browserType = playwright.chromium browser = browserType.launch(headless=False) -page = browser.newPage() +page = browser.new_page() page.goto('https://www.python.org/') ``` @@ -189,10 +207,10 @@ vrt.stop() #### Track page ```python -vrt.trackPage(page, imageName[, options]) +result = vrt.trackPage(page, imageName[, options]) ``` -- `page: Page` [Playwright page](https://microsoft.github.io/playwright-python/sync_api.html#playwright.sync_api.Page) +- `page: Page` [Playwright page](https://playwright.dev/python/docs/api/class-page) - `imageName: str` name for the taken screenshot image @@ -206,7 +224,7 @@ vrt.trackPage(page, imageName[, options]) - `height: int` area height in px - `screenshotOptions: PageScreenshotOptions` configuration for Playwrights `screenshot` method - - `full_page: bool` When true, takes a screenshot of the full scrollable page, instead of the currently visibvle viewport. Defaults to `false`. + - `full_page: bool` When true, takes a screenshot of the full scrollable page, instead of the currently visible viewport. Defaults to `false`. - `omit_background: bool` Hides default white background and allows capturing screenshots with transparency. Defaults to `false`. @@ -226,10 +244,12 @@ vrt.trackPage(page, imageName[, options]) #### Track element handle ```python -vrt.trackElementHandle(elementHandle, imageName[, options]) +result = vrt.trackElementHandle(elementHandle, imageName[, options]) ``` -- `elementHandle: ElementHandle` [Playwright ElementHandle](https://microsoft.github.io/playwright-python/sync_api.html#playwright.sync_api.ElementHandle) +The async API has `trackPageAsync` and `trackElementHandleAsync`. All four return the same `TestRunResult` as `track`. + +- `elementHandle: ElementHandle` [Playwright ElementHandle](https://playwright.dev/python/docs/api/class-elementhandle) - `imageName: str` name for the taken screenshot image diff --git a/pyproject.toml b/pyproject.toml new file mode 100644 index 0000000..d64310c --- /dev/null +++ b/pyproject.toml @@ -0,0 +1,41 @@ +[build-system] +requires = ["setuptools>=77"] +build-backend = "setuptools.build_meta" + +[project] +name = "visual_regression_tracker" +version = "5.0.0" +description = "Open source, self hosted solution for visual testing and managing results of visual testing." +readme = "README.md" +license = "Apache-2.0" +license-files = ["LICENSE"] +requires-python = ">=3.10" +dependencies = [ + "requests>=2.32.4", + "dacite>=1.9", +] +classifiers = [ + "Programming Language :: Python :: 3", + "Programming Language :: Python :: 3.10", + "Programming Language :: Python :: 3.11", + "Programming Language :: Python :: 3.12", + "Programming Language :: Python :: 3.13", + "Programming Language :: Python :: 3.14", + "Topic :: Software Development :: Testing", +] + +[project.optional-dependencies] +playwright = ["playwright>=1.50"] + +[project.urls] +Homepage = "https://github.com/Visual-Regression-Tracker/Visual-Regression-Tracker" +Source = "https://github.com/Visual-Regression-Tracker/sdk-python" +Issues = "https://github.com/Visual-Regression-Tracker/sdk-python/issues" + +[tool.setuptools] +packages = ["visual_regression_tracker"] + +[tool.pytest.ini_options] +python_classes = "NoThanks" +junit_family = "xunit2" +asyncio_default_fixture_loop_scope = "function" diff --git a/requirements.txt b/requirements.txt index c0286da..fc898b9 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,10 +1,5 @@ -requests -dacite -playwright >= 1.10 -typing-extensions -pytest -pytest-cov -pytest-mock -pytest-asyncio >= 0.24 -twine -pylint +pytest >= 9.2 +pytest-cov >= 7.1 +pytest-mock >= 3.17 +pytest-asyncio >= 1.4 +pylint >= 4.1 diff --git a/setup.cfg b/setup.cfg deleted file mode 100644 index efd41e1..0000000 --- a/setup.cfg +++ /dev/null @@ -1,9 +0,0 @@ -[bdist_wheel] -universal = 1 - -[metadata] -license_file = LICENSE - -[tool:pytest] -python_classes=NoThanks -junit_family=xunit2 \ No newline at end of file diff --git a/setup.py b/setup.py deleted file mode 100644 index 4f394af..0000000 --- a/setup.py +++ /dev/null @@ -1,23 +0,0 @@ -from setuptools import setup - -setup( - name='visual_regression_tracker', - version='4.9.0', - description='Open source, self hosted solution for visual testing ' - 'and managing results of visual testing.', - long_description=open('README.md').read(), - long_description_content_type='text/markdown', - author='', - license='APACHE', - url='https://github.com/Visual-Regression-Tracker/' - 'Visual-Regression-Tracker', - packages=['visual_regression_tracker'], - python_requires='>=3.9', - install_requires=['requests', 'dacite'], - extras_require={ - # Playwright provides browser automation. - "playwright": [ - "playwright", - ], - }, -) diff --git a/tox.ini b/tox.ini index 5c117e2..94016ad 100644 --- a/tox.ini +++ b/tox.ini @@ -4,7 +4,7 @@ # and then run "tox" from this directory. [tox] -envlist = py39, py310, py311, py312, py313, py314 +envlist = py310, py311, py312, py313, py314 [testenv] commands_pre = @@ -17,11 +17,11 @@ commands = --cov-report=xml \ --cov-report=html \ {posargs:tests} +extras = playwright deps = -r requirements.txt [gh-actions] python = - 3.9: py39 3.10: py310 3.11: py311 3.12: py312 From 5406f951d5a9f17139c916f1467a90dde7d132e9 Mon Sep 17 00:00:00 2001 From: Pavlo Strunkin Date: Mon, 2 Nov 2026 13:29:16 +0200 Subject: [PATCH 5/9] refactor: let dacite cast the status enum; tidy tests Co-Authored-By: Claude Opus 5.5 --- tests/test_config.py | 21 ++++++++++--------- tests/test_visualRegressionTracker.py | 11 ++++++---- visual_regression_tracker/types.py | 2 +- .../visualRegressionTracker.py | 1 - 4 files changed, 19 insertions(+), 16 deletions(-) diff --git a/tests/test_config.py b/tests/test_config.py index 46f1054..f841057 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -63,18 +63,19 @@ def test_config_timeout_defaults_to_30_seconds(): assert Config().httpTimeoutInSeconds == 30 -@pytest.mark.parametrize('make_config', [ - lambda: Config(apiUrl='http://vrt:4200/'), - lambda: Config.from_environment({'VRT_APIURL': 'http://vrt:4200/'}), -]) -def test_config_strips_trailing_slash_from_apiUrl(make_config): - assert make_config().apiUrl == 'http://vrt:4200' +def config_from_file(tmp_path, data): + path = tmp_path / 'vrt.json' + path.write_text(json.dumps(data)) + return Config.from_file(path) -def test_config_from_file_strips_trailing_slash_from_apiUrl(tmp_path): - path = tmp_path / 'vrt.json' - path.write_text(json.dumps({'apiUrl': 'http://vrt:4200/'})) - assert Config.from_file(path).apiUrl == 'http://vrt:4200' +@pytest.mark.parametrize('make_config', [ + lambda tmp_path: Config(apiUrl='http://vrt:4200/'), + lambda tmp_path: Config.from_environment({'VRT_APIURL': 'http://vrt:4200/'}), + lambda tmp_path: config_from_file(tmp_path, {'apiUrl': 'http://vrt:4200/'}), +]) +def test_config_strips_trailing_slash_from_apiUrl(make_config, tmp_path): + assert make_config(tmp_path).apiUrl == 'http://vrt:4200' def test_default_uses_path(config_file): diff --git a/tests/test_visualRegressionTracker.py b/tests/test_visualRegressionTracker.py index 3c95be4..60a5722 100644 --- a/tests/test_visualRegressionTracker.py +++ b/tests/test_visualRegressionTracker.py @@ -1,3 +1,5 @@ +import dataclasses +import json import logging import re @@ -181,7 +183,7 @@ def test__start__should_start_build(vrt, mock_request): def test__start__should_not_double_the_slash_after_apiUrl(mock_request): - vrt = VisualRegressionTracker(Config(**{**vars(CONFIG), 'apiUrl': 'http://localhost:4200/'})) + vrt = VisualRegressionTracker(dataclasses.replace(CONFIG, apiUrl='http://localhost:4200/')) mock_request.return_value = {'id': '1312', 'projectId': 'asd'} vrt.start() @@ -314,11 +316,12 @@ def test__submitTestResults__should_throw_not_started(vrt, mocker): def mock_response(mocker, status_code, body=None, text='', reason=''): response = mocker.Mock() response.status_code = status_code - response.text = text response.reason = reason if body is None: + response.text = text response.json.side_effect = ValueError('Expecting value') else: + response.text = json.dumps(body) response.json.return_value = body return response @@ -342,8 +345,8 @@ def test__http_request__success(mocker): (400, {'statusCode': 400, 'message': ['name must be a string', 'branchName must be a string']}, '', '', 'name must be a string, branchName must be a string'), (502, None, 'Bad Gateway', 'Bad Gateway', 'Bad Gateway'), - (503, {}, '', 'Service Unavailable', 'Service Unavailable'), - (500, {'error': 'no message field'}, '{"error": "no message field"}', '', '{"error": "no message field"}'), + (503, None, '', 'Service Unavailable', 'Service Unavailable'), + (500, {'error': 'no message field'}, '', '', '{"error": "no message field"}'), ]) def test__http_request__raises_server_error_with_server_message( mocker, status_code, body, text, reason, expected_message): diff --git a/visual_regression_tracker/types.py b/visual_regression_tracker/types.py index 3dd1f28..0441eab 100644 --- a/visual_regression_tracker/types.py +++ b/visual_regression_tracker/types.py @@ -89,5 +89,5 @@ def dict_factory(key_values): def _from_dict(data, clazz): - obj = dacite.from_dict(clazz, data) + obj = dacite.from_dict(clazz, data, dacite.Config(cast=[enum.Enum])) return obj diff --git a/visual_regression_tracker/visualRegressionTracker.py b/visual_regression_tracker/visualRegressionTracker.py index c2e9d8a..e2555ad 100644 --- a/visual_regression_tracker/visualRegressionTracker.py +++ b/visual_regression_tracker/visualRegressionTracker.py @@ -83,7 +83,6 @@ def _submitTestResult(self, test: TestRun) -> TestRunResponse: ) result = self._request('post', 'test-runs', data) - result['status'] = TestRunStatus(result['status']) testRunResult = _from_dict(result, TestRunResponse) return testRunResult From 594f74e03b0d29e0ade97563a15381891e9e84b7 Mon Sep 17 00:00:00 2001 From: Pavlo Strunkin Date: Mon, 2 Nov 2026 13:30:44 +0200 Subject: [PATCH 6/9] fix: ServerError on a non-JSON success body; 0 disables the timeout; test before release Co-Authored-By: Claude Opus 5.5 --- .github/workflows/release.yml | 5 +++++ README.md | 10 +++++++++- tests/test_visualRegressionTracker.py | 19 +++++++++++++++++++ .../visualRegressionTracker.py | 9 ++++++--- 4 files changed, 39 insertions(+), 4 deletions(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 8f2a2e9..9c319de 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -22,6 +22,11 @@ jobs: echo "::error::Tag $GITHUB_REF_NAME does not match version $version in pyproject.toml" exit 1 fi + - name: Test + run: | + pip install '.[playwright]' -r requirements.txt + python -m playwright install --with-deps chromium + python -m pytest tests - name: Build run: pip install build && python -m build - uses: actions/upload-artifact@v7 diff --git a/README.md b/README.md index 02778d1..3f5363b 100644 --- a/README.md +++ b/README.md @@ -46,7 +46,7 @@ config = Config( # enableSoftAssert - Log errors instead of exceptions enableSoftAssert=False, - # httpTimeoutInSeconds - Timeout of each request to the server + # httpTimeoutInSeconds - Timeout of each request to the server, 0 for none httpTimeoutInSeconds=30, ) @@ -272,3 +272,11 @@ The async API has `trackPageAsync` and `trackElementHandleAsync`. All four retur - `os: str` operating system name, like Windows, Mac, etc. - `device: str` device name, PC identifier, mobile identifier etc. - `viewport: str` viewport size. + +## Development + +```sh +pip install -e '.[playwright]' -r requirements.txt +python -m playwright install chromium +python -m pytest +``` diff --git a/tests/test_visualRegressionTracker.py b/tests/test_visualRegressionTracker.py index 60a5722..196e1ac 100644 --- a/tests/test_visualRegressionTracker.py +++ b/tests/test_visualRegressionTracker.py @@ -191,6 +191,15 @@ def test__start__should_not_double_the_slash_after_apiUrl(mock_request): assert mock_request.call_args[0][0] == 'http://localhost:4200/builds' +def test__request__treats_zero_timeout_as_none(mock_request): + vrt = VisualRegressionTracker(dataclasses.replace(CONFIG, httpTimeoutInSeconds=0)) + mock_request.return_value = {'id': '1312', 'projectId': 'asd'} + + vrt.start() + + assert mock_request.call_args[0][4] is None + + def test__stop__should_stop_build(vrt, mock_request, mocker): buildId = '1312' vrt.buildId = buildId @@ -347,6 +356,7 @@ def test__http_request__success(mocker): (502, None, 'Bad Gateway', 'Bad Gateway', 'Bad Gateway'), (503, None, '', 'Service Unavailable', 'Service Unavailable'), (500, {'error': 'no message field'}, '', '', '{"error": "no message field"}'), + (400, {'message': [{'property': 'name'}]}, '', '', "{'property': 'name'}"), ]) def test__http_request__raises_server_error_with_server_message( mocker, status_code, body, text, reason, expected_message): @@ -360,6 +370,15 @@ def test__http_request__raises_server_error_with_server_message( assert str(error.value) == f'HTTP {status_code}: {expected_message}' +def test__http_request__raises_server_error_on_success_without_json(mocker): + mocker.patch('requests.post', return_value=mock_response(mocker, 200, None, 'Sign in')) + + with pytest.raises(ServerError, match='not JSON: Sign in') as error: + _http_request('url', 'post', {}, {}, 30) + + assert error.value.status_code == 200 + + @pytest.mark.parametrize('status_code,body,hint', [ (413, None, 'BODY_PARSER_JSON_LIMIT'), (422, {'message': 'Image is 20000x20000 (400000000 pixels), above MAX_IMAGE_PIXELS=100000000'}, 'MAX_IMAGE_PIXELS'), diff --git a/visual_regression_tracker/visualRegressionTracker.py b/visual_regression_tracker/visualRegressionTracker.py index e2555ad..7a0a5b0 100644 --- a/visual_regression_tracker/visualRegressionTracker.py +++ b/visual_regression_tracker/visualRegressionTracker.py @@ -38,7 +38,7 @@ def _request(self, method: str, path: str, data: dict) -> dict: method, data, self.headers, - self.config.httpTimeoutInSeconds, + self.config.httpTimeoutInSeconds or None, ) def start(self) -> Build: @@ -121,7 +121,10 @@ def _http_request(url: str, method: str, data: dict, headers: dict, timeout: flo hint = _HINTS.get(status) raise ServerError(status, f'{message} (hint: {hint})' if hint else message) - return response.json() + try: + return response.json() + except ValueError: + raise ServerError(status, f'response is not JSON: {response.text.strip()[:200]}') def _error_message(response) -> str: @@ -130,5 +133,5 @@ def _error_message(response) -> str: except (ValueError, AttributeError): message = None if isinstance(message, list): - message = ', '.join(message) + message = ', '.join(map(str, message)) return message or response.text.strip() or response.reason or 'no error message' From 23f8dab5dd4003e6c98c7f2644c604470a5825f0 Mon Sep 17 00:00:00 2001 From: Pavlo Strunkin Date: Mon, 2 Nov 2026 14:05:27 +0200 Subject: [PATCH 7/9] fix: 10 s connect / 360 s read timeout; wrap request errors The server compares images inside POST /test-runs (VLM checks up to 300 s, diff jobs up to 120 s plus queue wait), so a 30 s timeout failed tracks the server had stored. httpTimeoutInSeconds is now the read timeout, 360 s by default; connecting gets 10 s. - requests exceptions (timeouts, connection errors) raise RequestError with the URL and without the request headers. - An empty VRT_HTTPTIMEOUTINSECONDS is unset; a negative or non-numeric one raises InvalidConfigurationError naming the variable. - A non-JSON success body names the response URL. - release.yml refuses a tag whose commit is not on master. BREAKING CHANGE: vrt.json is read from the current working directory, no longer from the directory of the running script. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/release.yml | 8 +++++ README.md | 15 +++++--- tests/test_config.py | 20 ++++++++--- tests/test_visualRegressionTracker.py | 34 +++++++++++++++---- visual_regression_tracker/__init__.py | 7 ++-- visual_regression_tracker/config.py | 17 ++++++++-- visual_regression_tracker/exceptions.py | 16 +++++++++ .../visualRegressionTracker.py | 15 +++++--- 8 files changed, 106 insertions(+), 26 deletions(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 9c319de..e45d476 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -12,6 +12,14 @@ jobs: runs-on: ubuntu-latest steps: - uses: actions/checkout@v7 + with: + fetch-depth: 0 + - name: Check the tag is on master + run: | + if ! git merge-base --is-ancestor "$GITHUB_SHA" origin/master; then + echo "::error::Tag $GITHUB_REF_NAME points at a commit that is not on master" + exit 1 + fi - uses: actions/setup-python@v7 with: python-version: '3.14' diff --git a/README.md b/README.md index 3f5363b..19317d5 100644 --- a/README.md +++ b/README.md @@ -11,7 +11,7 @@ Requires Python 3.10 or later. Image URLs in results need VRT server 5.0.10 or l pip install visual-regression-tracker # or, with playwright integration -pip install visual-regression-tracker[playwright] +pip install "visual-regression-tracker[playwright]" python -m playwright install ``` @@ -46,8 +46,10 @@ config = Config( # enableSoftAssert - Log errors instead of exceptions enableSoftAssert=False, - # httpTimeoutInSeconds - Timeout of each request to the server, 0 for none - httpTimeoutInSeconds=30, + # httpTimeoutInSeconds - How long to wait for the server's answer, 0 for no limit. + # The server compares images while answering track(); with VLM checks or a busy + # comparison queue that can take minutes, so keep it above 300. + httpTimeoutInSeconds=360, ) vrt = VisualRegressionTracker(config) @@ -62,7 +64,7 @@ vrt = VisualRegressionTracker(config) "ciBuildId":"commit_sha", "branchName":"develop", "enableSoftAssert":false, - "httpTimeoutInSeconds":30 + "httpTimeoutInSeconds":360 } ``` @@ -79,7 +81,7 @@ VRT_APIKEY="tXZVHX0EA4YQM1MGDD" \ VRT_CIBUILDID="commit_sha" \ VRT_BRANCHNAME="develop" \ VRT_ENABLESOFTASSERT=true \ -VRT_HTTPTIMEOUTINSECONDS=30 \ +VRT_HTTPTIMEOUTINSECONDS=360 \ python ``` ```python @@ -162,6 +164,9 @@ with `enableSoftAssert` it logs the error instead. `ok`, `approved` and `autoApp too, as in the JavaScript SDK. A status this SDK does not know becomes `TestRunStatus.UNKNOWN`. An HTTP error from the server raises `ServerError` with `status_code` and the server's `message`. +A request that gets no answer (connection refused, connect timeout after 10 s, read timeout after +`httpTimeoutInSeconds`) raises `RequestError` with the `url`. A read timeout on `track` does not mean +the server dropped the test run: it may still be stored. ### Integration with Microsoft Playwright diff --git a/tests/test_config.py b/tests/test_config.py index f841057..d1b181c 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -4,7 +4,7 @@ import pytest -from visual_regression_tracker import Config, MissingConfigurationError +from visual_regression_tracker import Config, InvalidConfigurationError, MissingConfigurationError from visual_regression_tracker.config import ENV_MAPPING @@ -59,8 +59,20 @@ def test_config_from_environment(config_env): assert cfg.httpTimeoutInSeconds == 7.5 -def test_config_timeout_defaults_to_30_seconds(): - assert Config().httpTimeoutInSeconds == 30 +def test_config_timeout_defaults_to_360_seconds(): + assert Config().httpTimeoutInSeconds == 360 + + +def test_config_treats_empty_timeout_variable_as_unset(): + assert Config.from_environment({'VRT_HTTPTIMEOUTINSECONDS': ''}).httpTimeoutInSeconds == 360 + + +@pytest.mark.parametrize('value', ['-1', 'soon']) +def test_config_rejects_invalid_timeout_variable(value): + with pytest.raises(InvalidConfigurationError, match='VRT_HTTPTIMEOUTINSECONDS') as error: + Config.from_environment({'VRT_HTTPTIMEOUTINSECONDS': value}) + + assert error.value.field == 'httpTimeoutInSeconds' def config_from_file(tmp_path, data): @@ -123,7 +135,7 @@ def test_default_uses_defaults(config_env, mocker): cfg = Config.default(environment=config_env) assert cfg.apiUrl == Config.apiUrl assert cfg.ciBuildId is None - assert cfg.httpTimeoutInSeconds == 30 + assert cfg.httpTimeoutInSeconds == 360 def test_default_prefers_environment(config_env, config_file): diff --git a/tests/test_visualRegressionTracker.py b/tests/test_visualRegressionTracker.py index 196e1ac..9b5ee77 100644 --- a/tests/test_visualRegressionTracker.py +++ b/tests/test_visualRegressionTracker.py @@ -4,11 +4,12 @@ import re import pytest +import requests from visual_regression_tracker import \ Build, Config, IgnoreArea, VisualRegressionTracker, \ TestRun, TestRunResponse, TestRunStatus, \ - ServerError, TestRunError, VisualRegressionTrackerError + RequestError, ServerError, TestRunError, VisualRegressionTrackerError from visual_regression_tracker.types import \ _to_dict from visual_regression_tracker.visualRegressionTracker import \ @@ -27,6 +28,7 @@ 'apiKey': CONFIG.apiKey, 'project': CONFIG.project } +TIMEOUT = (10, CONFIG.httpTimeoutInSeconds) @pytest.fixture @@ -174,7 +176,7 @@ def test__start__should_start_build(vrt, mock_request): 'project': CONFIG.project }, HEADERS, - CONFIG.httpTimeoutInSeconds, + TIMEOUT, ) assert build == Build(buildId, projectId) @@ -191,13 +193,13 @@ def test__start__should_not_double_the_slash_after_apiUrl(mock_request): assert mock_request.call_args[0][0] == 'http://localhost:4200/builds' -def test__request__treats_zero_timeout_as_none(mock_request): +def test__request__treats_zero_timeout_as_no_read_timeout(mock_request): vrt = VisualRegressionTracker(dataclasses.replace(CONFIG, httpTimeoutInSeconds=0)) mock_request.return_value = {'id': '1312', 'projectId': 'asd'} vrt.start() - assert mock_request.call_args[0][4] is None + assert mock_request.call_args[0][4] == (10, None) def test__stop__should_stop_build(vrt, mock_request, mocker): @@ -213,7 +215,7 @@ def test__stop__should_stop_build(vrt, mock_request, mocker): 'patch', {'isRunning': False}, HEADERS, - CONFIG.httpTimeoutInSeconds, + TIMEOUT, ) assert vrt.buildId is None assert vrt.projectId is None @@ -302,7 +304,7 @@ def test__submitTestResults__should_submit_test_run(vrt, mock_request): ] }, HEADERS, - CONFIG.httpTimeoutInSeconds, + TIMEOUT, ) @@ -324,6 +326,7 @@ def test__submitTestResults__should_throw_not_started(vrt, mocker): def mock_response(mocker, status_code, body=None, text='', reason=''): response = mocker.Mock() + response.url = 'http://vrt/builds' response.status_code = status_code response.reason = reason if body is None: @@ -373,10 +376,27 @@ def test__http_request__raises_server_error_with_server_message( def test__http_request__raises_server_error_on_success_without_json(mocker): mocker.patch('requests.post', return_value=mock_response(mocker, 200, None, 'Sign in')) - with pytest.raises(ServerError, match='not JSON: Sign in') as error: + with pytest.raises(ServerError, match='http://vrt/builds is not JSON: Sign in') as error: _http_request('url', 'post', {}, {}, 30) assert error.value.status_code == 200 + assert error.value.__suppress_context__ + + +@pytest.mark.parametrize('exception', [ + requests.exceptions.ReadTimeout('Read timed out. (read timeout=360)'), + requests.exceptions.ConnectionError('Failed to establish a new connection'), +]) +def test__http_request__wraps_request_exceptions_without_headers(mocker, exception): + mocker.patch('requests.post', side_effect=exception) + + with pytest.raises(RequestError) as error: + _http_request('http://vrt/test-runs', 'post', {}, {'apiKey': 'secret-key'}, (10, 360)) + + assert error.value.url == 'http://vrt/test-runs' + assert str(error.value) == f'POST http://vrt/test-runs failed: {type(exception).__name__}: {exception}' + assert 'secret-key' not in str(error.value) + assert error.value.__cause__ is None and error.value.__suppress_context__ @pytest.mark.parametrize('status_code,body,hint', [ diff --git a/visual_regression_tracker/__init__.py b/visual_regression_tracker/__init__.py index b37e478..4aacdf5 100644 --- a/visual_regression_tracker/__init__.py +++ b/visual_regression_tracker/__init__.py @@ -20,12 +20,15 @@ """ from .types import Build, IgnoreArea, TestRun, TestRunResponse, TestRunStatus -from .exceptions import VisualRegressionTrackerError, ServerError, TestRunError, MissingConfigurationError +from .exceptions import \ + VisualRegressionTrackerError, ServerError, RequestError, TestRunError, \ + MissingConfigurationError, InvalidConfigurationError from .config import Config from .visualRegressionTracker import VisualRegressionTracker __all__ = [ 'Config', 'Build', 'IgnoreArea', 'TestRun', 'TestRunResponse', 'TestRunStatus', - 'VisualRegressionTrackerError', 'ServerError', 'TestRunError', 'MissingConfigurationError', + 'VisualRegressionTrackerError', 'ServerError', 'RequestError', 'TestRunError', + 'MissingConfigurationError', 'InvalidConfigurationError', 'VisualRegressionTracker', ] diff --git a/visual_regression_tracker/config.py b/visual_regression_tracker/config.py index 9baf572..801806d 100644 --- a/visual_regression_tracker/config.py +++ b/visual_regression_tracker/config.py @@ -5,7 +5,7 @@ import json import pathlib -from .exceptions import MissingConfigurationError +from .exceptions import InvalidConfigurationError, MissingConfigurationError from .types import _from_dict @@ -30,7 +30,7 @@ class Config: project: str = None apiKey: str = None enableSoftAssert: bool = False - httpTimeoutInSeconds: float = 30 + httpTimeoutInSeconds: float = 360 def __setattr__(self, name, value): if name == 'apiUrl' and isinstance(value, str): @@ -80,7 +80,9 @@ def apply_environment(self, environment: dict = None): elif field_type is bool: val = val.lower() in ('true', '1') elif field_type is float: - val = float(val) + if not val: + continue + val = _seconds(field_name, env_name, val) else: raise Exception('Unsupported type') setattr(self, field_name, val) @@ -96,3 +98,12 @@ def check_complete(self): if getattr(self, field_name) is None: raise MissingConfigurationError(field_name, f'{field_name} is not specified.') + +def _seconds(field_name: str, env_name: str, val: str) -> float: + try: + seconds = float(val) + except ValueError: + seconds = None + if seconds is None or not seconds >= 0: + raise InvalidConfigurationError(field_name, f'{env_name} must be a non-negative number of seconds, got {val!r}.') + return seconds diff --git a/visual_regression_tracker/exceptions.py b/visual_regression_tracker/exceptions.py index 1087a82..7cfc8d2 100644 --- a/visual_regression_tracker/exceptions.py +++ b/visual_regression_tracker/exceptions.py @@ -16,6 +16,14 @@ def __init__(self, status_code: int, message: str): super(ServerError, self).__init__(f'HTTP {status_code}: {message}') +class RequestError(VisualRegressionTrackerError): + """A request to the VisualRegressionTracker server got no response.""" + + def __init__(self, url: str, message: str): + self.url = url + super(RequestError, self).__init__(message) + + class TestRunError(VisualRegressionTrackerError): """A visual test failed.""" @@ -25,6 +33,14 @@ def __init__(self, status: TestRunStatus, *args): super(TestRunError, self).__init__(*args, status) +class InvalidConfigurationError(VisualRegressionTrackerError): + """A VRT configuration value is invalid.""" + + def __init__(self, field: str, message: str): + self.field = field + super(InvalidConfigurationError, self).__init__(message) + + class MissingConfigurationError(VisualRegressionTrackerError): """VRT Configuration is incomplete.""" diff --git a/visual_regression_tracker/visualRegressionTracker.py b/visual_regression_tracker/visualRegressionTracker.py index 7a0a5b0..dfa8976 100644 --- a/visual_regression_tracker/visualRegressionTracker.py +++ b/visual_regression_tracker/visualRegressionTracker.py @@ -6,7 +6,7 @@ Build, TestRun, TestRunResponse, TestRunStatus, \ _to_dict, _from_dict, TestRunResult from .exceptions import \ - ServerError, TestRunError, VisualRegressionTrackerError + RequestError, ServerError, TestRunError, VisualRegressionTrackerError from .config import Config @@ -38,7 +38,7 @@ def _request(self, method: str, path: str, data: dict) -> dict: method, data, self.headers, - self.config.httpTimeoutInSeconds or None, + (CONNECT_TIMEOUT_SECONDS, self.config.httpTimeoutInSeconds or None), ) def start(self) -> Build: @@ -105,15 +105,20 @@ def track(self, test: TestRun) -> TestRunResult: return TestRunResult(result, self.config.apiUrl) +CONNECT_TIMEOUT_SECONDS = 10 + _HINTS = { 413: 'the request is larger than BODY_PARSER_JSON_LIMIT on the server; raise it or send a smaller screenshot', 422: 'the image has more pixels than MAX_IMAGE_PIXELS on the server; raise it or send a smaller screenshot', } -def _http_request(url: str, method: str, data: dict, headers: dict, timeout: float) -> dict: +def _http_request(url: str, method: str, data: dict, headers: dict, timeout: tuple) -> dict: request = getattr(requests, method.lower()) - response = request(url, json=data, headers=headers, timeout=timeout) + try: + response = request(url, json=data, headers=headers, timeout=timeout) + except requests.RequestException as e: + raise RequestError(url, f'{method.upper()} {url} failed: {type(e).__name__}: {e}') from None status = response.status_code if status >= 400: @@ -124,7 +129,7 @@ def _http_request(url: str, method: str, data: dict, headers: dict, timeout: flo try: return response.json() except ValueError: - raise ServerError(status, f'response is not JSON: {response.text.strip()[:200]}') + raise ServerError(status, f'response from {response.url} is not JSON: {response.text.strip()[:200]}') from None def _error_message(response) -> str: From a9932532c912653a2445fc4b48744915b8960e32 Mon Sep 17 00:00:00 2001 From: Pavlo Strunkin Date: Mon, 2 Nov 2026 14:06:57 +0200 Subject: [PATCH 8/9] refactor: validate the timeout from every config source; empty env vars are unset Co-Authored-By: Claude Opus 5.5 --- .github/workflows/release.yml | 8 +++--- README.md | 1 + tests/test_config.py | 26 +++++++++++++++---- visual_regression_tracker/config.py | 24 ++++++++--------- .../visualRegressionTracker.py | 4 +-- 5 files changed, 40 insertions(+), 23 deletions(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index e45d476..a8977b3 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -14,10 +14,12 @@ jobs: - uses: actions/checkout@v7 with: fetch-depth: 0 - - name: Check the tag is on master + - name: Check the tag is on the default branch + env: + DEFAULT_BRANCH: ${{ github.event.repository.default_branch }} run: | - if ! git merge-base --is-ancestor "$GITHUB_SHA" origin/master; then - echo "::error::Tag $GITHUB_REF_NAME points at a commit that is not on master" + if ! git merge-base --is-ancestor "$GITHUB_SHA" "origin/$DEFAULT_BRANCH"; then + echo "::error::Tag $GITHUB_REF_NAME points at a commit that is not on $DEFAULT_BRANCH" exit 1 fi - uses: actions/setup-python@v7 diff --git a/README.md b/README.md index 19317d5..950ec92 100644 --- a/README.md +++ b/README.md @@ -84,6 +84,7 @@ VRT_ENABLESOFTASSERT=true \ VRT_HTTPTIMEOUTINSECONDS=360 \ python ``` +Empty variables are ignored. ```python vrt = VisualRegressionTracker() ``` diff --git a/tests/test_config.py b/tests/test_config.py index d1b181c..3c5ca09 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -59,6 +59,12 @@ def test_config_from_environment(config_env): assert cfg.httpTimeoutInSeconds == 7.5 +def config_from_file(tmp_path, data): + path = tmp_path / 'vrt.json' + path.write_text(json.dumps(data)) + return Config.from_file(path) + + def test_config_timeout_defaults_to_360_seconds(): assert Config().httpTimeoutInSeconds == 360 @@ -67,7 +73,14 @@ def test_config_treats_empty_timeout_variable_as_unset(): assert Config.from_environment({'VRT_HTTPTIMEOUTINSECONDS': ''}).httpTimeoutInSeconds == 360 -@pytest.mark.parametrize('value', ['-1', 'soon']) +def test_config_treats_empty_variables_as_unset(): + cfg = Config.from_environment({'VRT_CIBUILDID': '', 'VRT_APIURL': ''}) + + assert cfg.ciBuildId is None + assert cfg.apiUrl == 'http://localhost:4200' + + +@pytest.mark.parametrize('value', ['-1', 'soon', 'nan']) def test_config_rejects_invalid_timeout_variable(value): with pytest.raises(InvalidConfigurationError, match='VRT_HTTPTIMEOUTINSECONDS') as error: Config.from_environment({'VRT_HTTPTIMEOUTINSECONDS': value}) @@ -75,10 +88,13 @@ def test_config_rejects_invalid_timeout_variable(value): assert error.value.field == 'httpTimeoutInSeconds' -def config_from_file(tmp_path, data): - path = tmp_path / 'vrt.json' - path.write_text(json.dumps(data)) - return Config.from_file(path) +@pytest.mark.parametrize('make_config', [ + lambda tmp_path: Config(httpTimeoutInSeconds=-1), + lambda tmp_path: config_from_file(tmp_path, {'httpTimeoutInSeconds': -5}), +]) +def test_config_rejects_negative_timeout_from_any_source(make_config, tmp_path): + with pytest.raises(InvalidConfigurationError, match='httpTimeoutInSeconds'): + make_config(tmp_path) @pytest.mark.parametrize('make_config', [ diff --git a/visual_regression_tracker/config.py b/visual_regression_tracker/config.py index 801806d..54ba22b 100644 --- a/visual_regression_tracker/config.py +++ b/visual_regression_tracker/config.py @@ -35,6 +35,9 @@ class Config: def __setattr__(self, name, value): if name == 'apiUrl' and isinstance(value, str): value = value.rstrip('/') + if name == 'httpTimeoutInSeconds' and not _is_seconds(value): + raise InvalidConfigurationError( + name, f'{name} ({ENV_MAPPING[name]}) must be a non-negative number of seconds, got {value!r}.') super().__setattr__(name, value) @staticmethod @@ -71,18 +74,19 @@ def from_file(path: typing.Union[str, pathlib.Path]): def apply_environment(self, environment: dict = None): fields = {f.name:f.type for f in dataclasses.fields(self)} for field_name, env_name in ENV_MAPPING.items(): - val = environment.get(env_name, None) + val = environment.get(env_name) field_type = fields[field_name] - if val is None: + if not val: continue if field_type is str: pass elif field_type is bool: val = val.lower() in ('true', '1') elif field_type is float: - if not val: - continue - val = _seconds(field_name, env_name, val) + try: + val = float(val) + except ValueError: + pass else: raise Exception('Unsupported type') setattr(self, field_name, val) @@ -99,11 +103,5 @@ def check_complete(self): raise MissingConfigurationError(field_name, f'{field_name} is not specified.') -def _seconds(field_name: str, env_name: str, val: str) -> float: - try: - seconds = float(val) - except ValueError: - seconds = None - if seconds is None or not seconds >= 0: - raise InvalidConfigurationError(field_name, f'{env_name} must be a non-negative number of seconds, got {val!r}.') - return seconds +def _is_seconds(value) -> bool: + return value is None or (isinstance(value, (int, float)) and not isinstance(value, bool) and value >= 0) diff --git a/visual_regression_tracker/visualRegressionTracker.py b/visual_regression_tracker/visualRegressionTracker.py index dfa8976..7cfa07a 100644 --- a/visual_regression_tracker/visualRegressionTracker.py +++ b/visual_regression_tracker/visualRegressionTracker.py @@ -9,6 +9,8 @@ RequestError, ServerError, TestRunError, VisualRegressionTrackerError from .config import Config +CONNECT_TIMEOUT_SECONDS = 10 + class VisualRegressionTracker: config: Config = None @@ -105,8 +107,6 @@ def track(self, test: TestRun) -> TestRunResult: return TestRunResult(result, self.config.apiUrl) -CONNECT_TIMEOUT_SECONDS = 10 - _HINTS = { 413: 'the request is larger than BODY_PARSER_JSON_LIMIT on the server; raise it or send a smaller screenshot', 422: 'the image has more pixels than MAX_IMAGE_PIXELS on the server; raise it or send a smaller screenshot', From 0ab4736e5a54227c624a974b59af078ec8369b11 Mon Sep 17 00:00:00 2001 From: Pavlo Strunkin Date: Mon, 2 Nov 2026 14:08:06 +0200 Subject: [PATCH 9/9] fix: reject an infinite timeout; from_environment() reads os.environ Co-Authored-By: Claude Opus 5.5 --- tests/test_config.py | 8 +++++++- visual_regression_tracker/config.py | 7 +++++-- visual_regression_tracker/exceptions.py | 2 +- 3 files changed, 13 insertions(+), 4 deletions(-) diff --git a/tests/test_config.py b/tests/test_config.py index 3c5ca09..b07f338 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -73,6 +73,12 @@ def test_config_treats_empty_timeout_variable_as_unset(): assert Config.from_environment({'VRT_HTTPTIMEOUTINSECONDS': ''}).httpTimeoutInSeconds == 360 +def test_config_from_environment_reads_os_environ_by_default(monkeypatch): + monkeypatch.setenv('VRT_PROJECT', 'env project') + + assert Config.from_environment().project == 'env project' + + def test_config_treats_empty_variables_as_unset(): cfg = Config.from_environment({'VRT_CIBUILDID': '', 'VRT_APIURL': ''}) @@ -80,7 +86,7 @@ def test_config_treats_empty_variables_as_unset(): assert cfg.apiUrl == 'http://localhost:4200' -@pytest.mark.parametrize('value', ['-1', 'soon', 'nan']) +@pytest.mark.parametrize('value', ['-1', 'soon', 'nan', 'inf']) def test_config_rejects_invalid_timeout_variable(value): with pytest.raises(InvalidConfigurationError, match='VRT_HTTPTIMEOUTINSECONDS') as error: Config.from_environment({'VRT_HTTPTIMEOUTINSECONDS': value}) diff --git a/visual_regression_tracker/config.py b/visual_regression_tracker/config.py index 54ba22b..b6a2978 100644 --- a/visual_regression_tracker/config.py +++ b/visual_regression_tracker/config.py @@ -3,6 +3,7 @@ import os import os.path import json +import math import pathlib from .exceptions import InvalidConfigurationError, MissingConfigurationError @@ -72,6 +73,7 @@ def from_file(path: typing.Union[str, pathlib.Path]): return cfg def apply_environment(self, environment: dict = None): + environment = os.environ if environment is None else environment fields = {f.name:f.type for f in dataclasses.fields(self)} for field_name, env_name in ENV_MAPPING.items(): val = environment.get(env_name) @@ -94,7 +96,7 @@ def apply_environment(self, environment: dict = None): @staticmethod def from_environment(environment: dict = None): cfg = Config() - cfg.apply_environment(environment) + cfg.apply_environment(environment or os.environ) return cfg def check_complete(self): @@ -104,4 +106,5 @@ def check_complete(self): def _is_seconds(value) -> bool: - return value is None or (isinstance(value, (int, float)) and not isinstance(value, bool) and value >= 0) + return value is None or ( + isinstance(value, (int, float)) and not isinstance(value, bool) and math.isfinite(value) and value >= 0) diff --git a/visual_regression_tracker/exceptions.py b/visual_regression_tracker/exceptions.py index 7cfc8d2..426d0e2 100644 --- a/visual_regression_tracker/exceptions.py +++ b/visual_regression_tracker/exceptions.py @@ -17,7 +17,7 @@ def __init__(self, status_code: int, message: str): class RequestError(VisualRegressionTrackerError): - """A request to the VisualRegressionTracker server got no response.""" + """A request to the VisualRegressionTracker server could not be sent or got no response.""" def __init__(self, url: str, message: str): self.url = url