jenkins-bot has submitted this change. ( https://gerrit.wikimedia.org/r/c/pywikibot/core/+/1108093?usp=email )

Change subject: [IMPR] Improvements for require_version decorator ......................................................................

[IMPR] Improvements for require_version decorator

- version_needed is a positional only parameter - TypeError and ValueError are used for validation fails instead of unspecific Exception - try/except with re.split to detect unpack errors and raise a ValueError('There is no valid operator given with version...') - remove test for empty op which cannot happen with re.split() - raise ValueError if the left part of re.split() is not empty - do not pass arguments to called method because a TypeError is raised already for such cases - add tests for this decorator

Change-Id: I5d3e14aac05dbf66229fd2c897796740d7f6ec5e --- M tests/aspects.py M tests/tests_tests.py 2 files changed, 111 insertions(+), 14 deletions(-)

Approvals: Xqt: Looks good to me, approved jenkins-bot: Verified

diff --git a/tests/aspects.py b/tests/aspects.py index 084cd29..8c71a21 100644 --- a/tests/aspects.py +++ b/tests/aspects.py @@ -284,7 +284,7 @@ return test_requirement

-def require_version(version_needed: str, reason: str = ''): +def require_version(version_needed: str, /, reason: str = ''): """Require minimum MediaWiki version to be queried.

The version needed for the test; must be given with a preleading rich @@ -298,9 +298,17 @@

.. versionadded:: 8.0

+ .. versionchanged:: 10.0 + TypeError and ValueError are used for validation fails. + *version_needed* parameter is positional only. + :param version_needed: The version needed :param reason: A reason for skipping the test. - :raises Exception: Usage validation fails + :raises TypeError: self.site is not a BaseSite or the decorated + method has parameters. + :raises ValueError: The given *version_needed* parameter is invalid + or an operand is given on the left or the version number is + invalid """ def test_requirement(method): """Test the requirement and return an optionally decorated object.""" @@ -309,26 +317,33 @@ """Validate environment.""" if not isinstance(self.site, BaseSite) \ or isinstance(self.site, DrySite): - raise Exception( # pragma: no cover + raise TypeError( f'{type(self).__name__}.site must be a BaseSite not ' f'{type(self.site).__name__}.')

if args or kwargs: - raise Exception( # pragma: no cover + raise TypeError( f'Test method {method.__name__!r} has parameters which is ' - f'not supported with require_version decorator.') + f'not supported with require_version decorator.' + )

- _, op, version = re.split('([<>]=?)', version_needed) - if not op: # pragma: no cover - raise Exception(f'There is no valid operator given with ' - f'version {version_needed!r}') + try: + site_vers, op, version = re.split('([<>]=?)', version_needed) + except ValueError: + raise ValueError(f'There is no valid operator given with ' + f'version {version_needed!r}') + + if site_vers: + raise ValueError( + f'first operand {site_vers} should not be set')

skip = not eval( f'self.site.mw_version {op} MediaWikiVersion(version)') - if not skip: - return method(self, *args, **kwargs)

- myreason = ' to ' + reason if reason else '' # pragma: no cover + if not skip: + return method(self) + + myreason = ' to ' + reason if reason else '' raise unittest.SkipTest( f'MediaWiki {op} v{version} required{myreason}.')

diff --git a/tests/tests_tests.py b/tests/tests_tests.py index 4ccd012..749aab1 100755 --- a/tests/tests_tests.py +++ b/tests/tests_tests.py @@ -1,7 +1,7 @@ #!/usr/bin/env python3 """Tests for the tests package.""" # -# (C) Pywikibot team, 2014-2023 +# (C) Pywikibot team, 2014-2025 # # Distributed under the terms of the MIT license. from __future__ import annotations @@ -10,7 +10,7 @@ from contextlib import suppress

from tests import utils -from tests.aspects import TestCase +from tests.aspects import DefaultSiteTestCase, TestCase, require_version

class HttpServerProblemTestCase(TestCase): @@ -78,6 +78,88 @@ self.assertLength(None, self.seq)

+class TestRequireVersionDry(DefaultSiteTestCase): + + """Test require_version decorator.""" + + dry = True + + @require_version('') + def method(self): + """Test method for decorator.""" + + def test_require_version(self): + """Test require_version for DrySite.""" + with self.assertRaisesRegex( + TypeError, + f'{type(self).__name__}.site must be a BaseSite not DrySite'): + self.method() + + +class TestRequireVersion(DefaultSiteTestCase): + + """Test require_version decorator.""" + + @require_version('') + def method_with_params(self, key): + """Test method for decorated methods with unsupported arguments.""" + + def method_failing(self): + """Test method for decorator with invalid parameter.""" + self.assertTrue(False, 'should never happen') + + @require_version('>=1.31') + def method_succeed(self): + """Test that decorator passes.""" + self.assertTrue(False, 'intentional fail for method_succeed test') + + @require_version('<1.31') + def method_fail(self): + """Test that decorator skips.""" + self.assertTrue(False, 'intentional fail for test') + + def test_unsupported_methods(self): + """Test require_version with unsupported methods.""" + with self.assertRaisesRegex( + TypeError, "Test method 'method_with_params' has parameters"): + self.method_with_params('42') + with self.assertRaisesRegex( + TypeError, "Test method 'method_with_params' has parameters"): + self.method_with_params(key='42') + with self.assertRaisesRegex(ValueError, + 'There is no valid operator given '): + self.method_with_params() + + def test_version_needed(self): + """Test for invalid decorator parameters.""" + with self.assertRaisesRegex(ValueError, + 'There is no valid operator given '): + require_version('foo')(self.method_failing)(self) + with self.assertRaisesRegex(ValueError, + 'first operand foo should not be set'): + require_version('foo>bar')(self.method_failing)(self) + with self.assertRaisesRegex(ValueError, 'Invalid version number'): + require_version('>bar')(self.method_failing)(self) + with self.assertRaisesRegex(unittest.SkipTest, + r'MediaWiki < v1.31 required'): + require_version('<1.31')(self.method_failing)(self) + with self.assertRaisesRegex( + unittest.SkipTest, + r'MediaWiki < v1.31 required to run this test'): + require_version('<1.31', + 'run this test')(self.method_failing)(self) + + def test_decorator(self): + """Test that decorator passes or skips.""" + with self.assertRaisesRegex( + AssertionError, + 'intentional fail for method_succeed test'): + self.method_succeed() + with self.assertRaisesRegex(unittest.SkipTest, + r'MediaWiki < v1.31 required'): + self.method_fail() + + class UtilsTests(TestCase):

"""Tests for tests.utils."""