From 13442f690183cc2fb724c51af2321164b5ba749d Mon Sep 17 00:00:00 2001 From: Adam Turner <9087854+aa-turner@users.noreply.github.com> Date: Fri, 17 Feb 2023 23:46:31 +0000 Subject: [PATCH] Fix pytest style issues --- sphinx/ext/apidoc.py | 3 ++- sphinx/testing/fixtures.py | 4 ++-- sphinx/writers/_html4.py | 2 +- sphinx/writers/html5.py | 2 +- sphinx/writers/texinfo.py | 2 +- tests/test_api_translator.py | 2 +- tests/test_build_html.py | 18 +++++++----------- tests/test_build_latex.py | 5 +---- tests/test_catalogs.py | 8 ++++---- tests/test_correct_year.py | 11 ++++++----- tests/test_ext_autosummary.py | 2 +- tests/test_ext_imgconverter.py | 4 ++-- tests/test_ext_intersphinx.py | 2 +- tests/test_ext_napoleon_docstring.py | 25 ++++++++----------------- tests/test_intl.py | 2 +- tests/test_locale.py | 2 +- tests/test_setup_command.py | 4 ++-- tests/test_util_images.py | 4 ++-- tests/test_util_inspect.py | 18 +++--------------- tests/test_util_logging.py | 6 +++--- tests/test_versioning.py | 2 +- 21 files changed, 51 insertions(+), 77 deletions(-) diff --git a/sphinx/ext/apidoc.py b/sphinx/ext/apidoc.py index 839486d55..578b54903 100644 --- a/sphinx/ext/apidoc.py +++ b/sphinx/ext/apidoc.py @@ -270,7 +270,8 @@ def recurse_tree(rootpath: str, excludes: list[str], opts: Any, toplevels.append(module_join(root_package, subpackage)) else: # if we are at the root level, we don't require it to be a package - assert root == rootpath and root_package is None + assert root == rootpath + assert root_package is None for py_file in files: if not is_skipped_module(path.join(rootpath, py_file), opts, excludes): module = py_file.split('.')[0] diff --git a/sphinx/testing/fixtures.py b/sphinx/testing/fixtures.py index 8e6fe36b3..710cadcd8 100644 --- a/sphinx/testing/fixtures.py +++ b/sphinx/testing/fixtures.py @@ -198,7 +198,7 @@ def _shared_result_cache() -> None: @pytest.fixture() -def if_graphviz_found(app: SphinxTestApp) -> None: +def if_graphviz_found(app: SphinxTestApp) -> None: # NoQA: PT004 """ The test will be skipped when using 'if_graphviz_found' fixture and graphviz dot command is not found. @@ -233,7 +233,7 @@ def tempdir(tmpdir: str) -> util.path: @pytest.fixture() -def rollback_sysmodules(): +def rollback_sysmodules(): # NoQA: PT004 """ Rollback sys.modules to its value before testing to unload modules during tests. diff --git a/sphinx/writers/_html4.py b/sphinx/writers/_html4.py index 927309fca..0b670bf99 100644 --- a/sphinx/writers/_html4.py +++ b/sphinx/writers/_html4.py @@ -215,7 +215,7 @@ class HTML4Translator(SphinxTranslator, BaseTranslator): 'References must have "refuri" or "refid" attribute.' atts['href'] = '#' + node['refid'] if not isinstance(node.parent, nodes.TextElement): - assert len(node) == 1 and isinstance(node[0], nodes.image) + assert len(node) == 1 and isinstance(node[0], nodes.image) # NoQA: PT018 atts['class'] += ' image-reference' if 'reftitle' in node: atts['title'] = node['reftitle'] diff --git a/sphinx/writers/html5.py b/sphinx/writers/html5.py index afd873908..ab26bab1e 100644 --- a/sphinx/writers/html5.py +++ b/sphinx/writers/html5.py @@ -221,7 +221,7 @@ class HTML5Translator(SphinxTranslator, BaseTranslator): 'References must have "refuri" or "refid" attribute.' atts['href'] = '#' + node['refid'] if not isinstance(node.parent, nodes.TextElement): - assert len(node) == 1 and isinstance(node[0], nodes.image) + assert len(node) == 1 and isinstance(node[0], nodes.image) # NoQA: PT018 atts['class'] += ' image-reference' if 'reftitle' in node: atts['title'] = node['reftitle'] diff --git a/sphinx/writers/texinfo.py b/sphinx/writers/texinfo.py index 22817b03c..087f13665 100644 --- a/sphinx/writers/texinfo.py +++ b/sphinx/writers/texinfo.py @@ -286,7 +286,7 @@ class TexinfoTranslator(SphinxTranslator): targets: list[Element] = [self.document] targets.extend(self.document.findall(nodes.section)) for node in targets: - assert 'node_name' in node and node['node_name'] + assert 'node_name' in node and node['node_name'] # NoQA: PT018 entries = [s['node_name'] for s in find_subsections(node)] node_menus[node['node_name']] = entries # try to find a suitable "Top" node diff --git a/tests/test_api_translator.py b/tests/test_api_translator.py index 2185fb8db..9f2bd4488 100644 --- a/tests/test_api_translator.py +++ b/tests/test_api_translator.py @@ -6,7 +6,7 @@ import pytest @pytest.fixture(scope='module', autouse=True) -def setup_module(rootdir): +def _setup_module(rootdir): p = rootdir / 'test-api-set-translator' sys.path.insert(0, p) yield diff --git a/tests/test_build_html.py b/tests/test_build_html.py index 2b4143d87..3f2be8bbc 100644 --- a/tests/test_build_html.py +++ b/tests/test_build_html.py @@ -1491,13 +1491,12 @@ def test_html_math_renderer_is_imgmath(app, status, warning): confoverrides={'extensions': ['sphinxcontrib.jsmath', 'sphinx.ext.imgmath']}) def test_html_math_renderer_is_duplicated(make_app, app_params): - try: - args, kwargs = app_params + args, kwargs = app_params + with pytest.raises( + ConfigError, + match='Many math_renderers are registered. But no math_renderer is selected.', + ): make_app(*args, **kwargs) - raise AssertionError() - except ConfigError as exc: - assert str(exc) == ('Many math_renderers are registered. ' - 'But no math_renderer is selected.') @pytest.mark.sphinx('html', testroot='basic', @@ -1521,12 +1520,9 @@ def test_html_math_renderer_is_chosen(app, status, warning): 'sphinx.ext.mathjax'], 'html_math_renderer': 'imgmath'}) def test_html_math_renderer_is_mismatched(make_app, app_params): - try: - args, kwargs = app_params + args, kwargs = app_params + with pytest.raises(ConfigError, match="Unknown math_renderer 'imgmath' is given."): make_app(*args, **kwargs) - raise AssertionError() - except ConfigError as exc: - assert str(exc) == "Unknown math_renderer 'imgmath' is given." @pytest.mark.sphinx('html', testroot='basic') diff --git a/tests/test_build_latex.py b/tests/test_build_latex.py index 946e6ded2..9f679e897 100644 --- a/tests/test_build_latex.py +++ b/tests/test_build_latex.py @@ -1012,11 +1012,8 @@ def test_image_in_section(app, status, warning): @pytest.mark.sphinx('latex', testroot='basic', confoverrides={'latex_logo': 'notfound.jpg'}) def test_latex_logo_if_not_found(app, status, warning): - try: + with pytest.raises(SphinxError): app.builder.build_all() - raise AssertionError() # SphinxError not raised - except Exception as exc: - assert isinstance(exc, SphinxError) @pytest.mark.sphinx('latex', testroot='toctree-maxdepth') diff --git a/tests/test_catalogs.py b/tests/test_catalogs.py index 0e75e8b35..f8a2129a2 100644 --- a/tests/test_catalogs.py +++ b/tests/test_catalogs.py @@ -7,7 +7,7 @@ from sphinx.testing.util import find_files @pytest.fixture() -def setup_test(app_params): +def _setup_test(app_params): srcdir = app_params.kwargs['srcdir'] src_locale_dir = srcdir / 'xx' / 'LC_MESSAGES' dest_locale_dir = srcdir / 'locale' @@ -25,7 +25,7 @@ def setup_test(app_params): (srcdir / '_build').rmtree(True) -@pytest.mark.usefixtures('setup_test') +@pytest.mark.usefixtures('_setup_test') @pytest.mark.test_params(shared_result='test-catalogs') @pytest.mark.sphinx( 'html', testroot='intl', @@ -44,7 +44,7 @@ def test_compile_all_catalogs(app, status, warning): assert actual == expect -@pytest.mark.usefixtures('setup_test') +@pytest.mark.usefixtures('_setup_test') @pytest.mark.test_params(shared_result='test-catalogs') @pytest.mark.sphinx( 'html', testroot='intl', @@ -62,7 +62,7 @@ def test_compile_specific_catalogs(app, status, warning): assert actual == {'admonitions.mo'} -@pytest.mark.usefixtures('setup_test') +@pytest.mark.usefixtures('_setup_test') @pytest.mark.test_params(shared_result='test-catalogs') @pytest.mark.sphinx( 'html', testroot='intl', diff --git a/tests/test_correct_year.py b/tests/test_correct_year.py index 4e9de6c52..4ef77a6f5 100644 --- a/tests/test_correct_year.py +++ b/tests/test_correct_year.py @@ -14,11 +14,12 @@ import pytest ) def expect_date(request, monkeypatch): sde, expect = request.param - if sde: - monkeypatch.setenv('SOURCE_DATE_EPOCH', sde) - else: - monkeypatch.delenv('SOURCE_DATE_EPOCH', raising=False) - yield expect + with monkeypatch.context() as m: + if sde: + m.setenv('SOURCE_DATE_EPOCH', sde) + else: + m.delenv('SOURCE_DATE_EPOCH', raising=False) + yield expect @pytest.mark.sphinx('html', testroot='correct-year') diff --git a/tests/test_ext_autosummary.py b/tests/test_ext_autosummary.py index b8e39c9cf..ed8c1e68d 100644 --- a/tests/test_ext_autosummary.py +++ b/tests/test_ext_autosummary.py @@ -40,7 +40,7 @@ default_kw = { @pytest.fixture(scope='function', autouse=True) -def unload_target_module(): +def _unload_target_module(): sys.modules.pop('target', None) diff --git a/tests/test_ext_imgconverter.py b/tests/test_ext_imgconverter.py index 9af917052..18be70048 100644 --- a/tests/test_ext_imgconverter.py +++ b/tests/test_ext_imgconverter.py @@ -6,7 +6,7 @@ import pytest @pytest.fixture() -def if_converter_found(app): +def _if_converter_found(app): image_converter = getattr(app.config, 'image_converter', '') try: if image_converter: @@ -18,7 +18,7 @@ def if_converter_found(app): pytest.skip('image_converter "%s" is not available' % image_converter) -@pytest.mark.usefixtures('if_converter_found') +@pytest.mark.usefixtures('_if_converter_found') @pytest.mark.sphinx('latex', testroot='ext-imgconverter') def test_ext_imgconverter(app, status, warning): app.builder.build_all() diff --git a/tests/test_ext_intersphinx.py b/tests/test_ext_intersphinx.py index ba65bc0af..060f9eca4 100644 --- a/tests/test_ext_intersphinx.py +++ b/tests/test_ext_intersphinx.py @@ -48,7 +48,7 @@ def set_config(app, mapping): @mock.patch('sphinx.ext.intersphinx.InventoryFile') @mock.patch('sphinx.ext.intersphinx._read_from_url') -def test_fetch_inventory_redirection(_read_from_url, InventoryFile, app, status, warning): +def test_fetch_inventory_redirection(_read_from_url, InventoryFile, app, status, warning): # NoQA: PT019 intersphinx_setup(app) _read_from_url().readline.return_value = b'# Sphinx inventory version 2' diff --git a/tests/test_ext_napoleon_docstring.py b/tests/test_ext_napoleon_docstring.py index 0be25d63d..238e2380a 100644 --- a/tests/test_ext_napoleon_docstring.py +++ b/tests/test_ext_napoleon_docstring.py @@ -2,7 +2,6 @@ import re from collections import namedtuple -from contextlib import contextmanager from inspect import cleandoc from textwrap import dedent from unittest import mock @@ -2563,21 +2562,6 @@ definition_after_normal_text : int actual = str(NumpyDocstring(docstring, config)) assert expected == actual - -@contextmanager -def warns(warning, match): - match_re = re.compile(match) - try: - yield warning - finally: - raw_warnings = warning.getvalue() - warnings = [w for w in raw_warnings.split("\n") if w.strip()] - - assert len(warnings) == 1 and all(match_re.match(w) for w in warnings) - warning.truncate(0) - - -class TestNumpyDocstring: def test_token_type_invalid(self, warning): tokens = ( "{1, 2", @@ -2596,8 +2580,15 @@ class TestNumpyDocstring: r".+: malformed string literal \(missing opening quote\):", ) for token, error in zip(tokens, errors): - with warns(warning, match=error): + try: _token_type(token) + finally: + raw_warnings = warning.getvalue() + warnings = [w for w in raw_warnings.split("\n") if w.strip()] + + assert len(warnings) == 1 + assert re.compile(error).match(warnings[0]) + warning.truncate(0) @pytest.mark.parametrize( ("name", "expected"), diff --git a/tests/test_intl.py b/tests/test_intl.py index bed425b26..a588aef4b 100644 --- a/tests/test_intl.py +++ b/tests/test_intl.py @@ -43,7 +43,7 @@ def write_mo(pathname, po): @pytest.fixture(autouse=True) -def setup_intl(app_params): +def _setup_intl(app_params): srcdir = path(app_params.kwargs['srcdir']) for dirpath, _dirs, files in os.walk(srcdir): dirpath = path(dirpath) diff --git a/tests/test_locale.py b/tests/test_locale.py index 1dcad64eb..4861079ec 100644 --- a/tests/test_locale.py +++ b/tests/test_locale.py @@ -6,7 +6,7 @@ from sphinx import locale @pytest.fixture(autouse=True) -def cleanup_translations(): +def _cleanup_translations(): yield locale.translators.clear() diff --git a/tests/test_setup_command.py b/tests/test_setup_command.py index 3f1ed6752..3690bbfcc 100644 --- a/tests/test_setup_command.py +++ b/tests/test_setup_command.py @@ -66,7 +66,7 @@ def test_build_sphinx_multiple_invalid_builders(setup_command): @pytest.fixture() -def nonascii_srcdir(request, setup_command): +def _nonascii_srcdir(request, setup_command): mb_name = '\u65e5\u672c\u8a9e' srcdir = (setup_command.pkgroot / 'doc') try: @@ -90,7 +90,7 @@ def nonascii_srcdir(request, setup_command): """ % locals())).encode()) -@pytest.mark.usefixtures('nonascii_srcdir') +@pytest.mark.usefixtures('_nonascii_srcdir') def test_build_sphinx_with_nonascii_path(setup_command): proc = setup_command.proc out, err = proc.communicate() diff --git a/tests/test_util_images.py b/tests/test_util_images.py index 42e45cea4..e2c407d4c 100644 --- a/tests/test_util_images.py +++ b/tests/test_util_images.py @@ -68,7 +68,7 @@ def test_parse_data_uri(): assert image is None # invalid data URI (no properties) + uri = ("data:iVBORw0KGgoAAAANSUhEUgAAAAUAAAAFCAYAAACNbyblAAAAHElEQVQI12P4" + "//8/w38GIAXDIBKE0DHxgljNBAAO9TXL0Y4OHwAAAABJRU5ErkJggg==") with pytest.raises(ValueError): - uri = ("data:iVBORw0KGgoAAAANSUhEUgAAAAUAAAAFCAYAAACNbyblAAAAHElEQVQI12P4" - "//8/w38GIAXDIBKE0DHxgljNBAAO9TXL0Y4OHwAAAABJRU5ErkJggg==") parse_data_uri(uri) diff --git a/tests/test_util_inspect.py b/tests/test_util_inspect.py index bc1b25674..cc67e37b3 100644 --- a/tests/test_util_inspect.py +++ b/tests/test_util_inspect.py @@ -463,12 +463,8 @@ def test_safe_getattr_with_exception(): obj = Foo() - try: + with pytest.raises(AttributeError, match='bar'): inspect.safe_getattr(obj, 'bar') - except AttributeError as exc: - assert exc.args[0] == 'bar' - else: - pytest.fail('AttributeError not raised') def test_safe_getattr_with_property_exception(): @@ -479,12 +475,8 @@ def test_safe_getattr_with_property_exception(): obj = Foo() - try: + with pytest.raises(AttributeError, match='bar'): inspect.safe_getattr(obj, 'bar') - except AttributeError as exc: - assert exc.args[0] == 'bar' - else: - pytest.fail('AttributeError not raised') def test_safe_getattr_with___dict___override(): @@ -495,12 +487,8 @@ def test_safe_getattr_with___dict___override(): obj = Foo() - try: + with pytest.raises(AttributeError, match='bar'): inspect.safe_getattr(obj, 'bar') - except AttributeError as exc: - assert exc.args[0] == 'bar' - else: - pytest.fail('AttributeError not raised') def test_dictionary_sorting(): diff --git a/tests/test_util_logging.py b/tests/test_util_logging.py index b9756f947..83116b93d 100644 --- a/tests/test_util_logging.py +++ b/tests/test_util_logging.py @@ -348,8 +348,8 @@ def test_skip_warningiserror(app, status, warning): logger.warning('message') # if False, warning raises SphinxWarning exception - with pytest.raises(SphinxWarning): - with logging.skip_warningiserror(False): + with logging.skip_warningiserror(False): + with pytest.raises(SphinxWarning): logger.warning('message') # It also works during pending_warnings. @@ -357,7 +357,7 @@ def test_skip_warningiserror(app, status, warning): with logging.skip_warningiserror(): logger.warning('message') - with pytest.raises(SphinxWarning): + with pytest.raises(SphinxWarning): # NoQA: PT012 with logging.pending_warnings(): with logging.skip_warningiserror(False): logger.warning('message') diff --git a/tests/test_versioning.py b/tests/test_versioning.py index 3172b31a1..80f1798f6 100644 --- a/tests/test_versioning.py +++ b/tests/test_versioning.py @@ -11,7 +11,7 @@ app = original = original_uids = None @pytest.fixture(scope='module', autouse=True) -def setup_module(rootdir, sphinx_test_tempdir): +def _setup_module(rootdir, sphinx_test_tempdir): global app, original, original_uids srcdir = sphinx_test_tempdir / 'test-versioning' if not srcdir.exists():