xflr6 / graphviz

Simple Python interface for Graphviz
https://graphviz.readthedocs.io
MIT License
1.63k stars 211 forks source link

[self-tests] `test_render_unknown_parameter_raises` fails #215

Closed stanislavlevin closed 7 months ago

stanislavlevin commented 7 months ago

Running tests against current master:

[builder@localhost graphviz]$ python3 -mtox -epy3
.pkg: _optional_hooks> python /usr/src/.local/lib/python3/site-packages/pyproject_api/_backend.py True setuptools.build_meta __legacy__
.pkg: get_requires_for_build_sdist> python /usr/src/.local/lib/python3/site-packages/pyproject_api/_backend.py True setuptools.build_meta __legacy__
.pkg: get_requires_for_build_wheel> python /usr/src/.local/lib/python3/site-packages/pyproject_api/_backend.py True setuptools.build_meta __legacy__
.pkg: prepare_metadata_for_build_wheel> python /usr/src/.local/lib/python3/site-packages/pyproject_api/_backend.py True setuptools.build_meta __legacy__
.pkg: build_sdist> python /usr/src/.local/lib/python3/site-packages/pyproject_api/_backend.py True setuptools.build_meta __legacy__
py3: install_package> python -I -m pip install --force-reinstall --no-deps /usr/src/RPM/BUILD/graphviz/.tox/.tmp/package/2/graphviz-0.20.2.dev0.tar.gz
py3: commands[0]> python -X dev run-tests.py
run ['run-tests.py']
pytest.main([])
================================ test session starts =================================
platform linux -- Python 3.12.1, pytest-8.0.2, pluggy-1.4.0
cachedir: .tox/py3/.pytest_cache
rootdir: /usr/src/RPM/BUILD/graphviz
configfile: setup.cfg
testpaths: README.rst, docs, graphviz, tests
plugins: cov-4.1.0, mock-3.12.0

Tests fail with:

====================================== FAILURES ======================================
_______ test_render_unknown_parameter_raises[args0-ValueError-unknown engine] ________

args = ['', 'pdf', 'nonfilepath'], expected_exception = <class 'ValueError'>
match = 'unknown engine'

    @pytest.mark.parametrize(
        'args, expected_exception, match',
        [(['', 'pdf', 'nonfilepath'], ValueError, r'unknown engine'),
         (['dot', '', 'nonfilepath'], ValueError, r'unknown format'),
         (['dot', 'ps', 'nonfilepath', '', None], ValueError, r'unknown renderer'),
         (['dot', 'ps', 'nonfilepath', None, 'core'],
          graphviz.RequiredArgumentError, r'without renderer'),
         (['dot', 'ps', 'nonfilepath', 'ps', ''], ValueError, r'unknown formatter')],
        ids=lambda x: getattr(x, '__name__', x))
    def test_render_unknown_parameter_raises(args, expected_exception, match):
        with pytest.raises(expected_exception, match=match), pytest.deprecated_call():
>           graphviz.render(*args)

tests/backend/test_rendering.py:40: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
graphviz/_tools.py:171: in wrapper
    return func(*args, **kwargs)
graphviz/backend/rendering.py:314: in render
    cmd = dot_command.command(engine, format,
graphviz/backend/dot_command.py:31: in command
    parameters.verify_engine(engine, required=True)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

engine = ''

    def verify_engine(engine: str, *, required: bool = REQUIRED) -> None:
        if engine is None:
            if required:
                raise ValueError('missing engine')
        elif engine.lower() not in ENGINES:
>           raise ValueError(f'unknown engine: {engine!r}'
                             f' (must be one of {sorted(ENGINES)})')
E           ValueError: unknown engine: '' (must be one of ['circo', 'dot', 'fdp', 'neato', 'osage', 'patchwork', 'sfdp', 'twopi'])

graphviz/parameters/engines.py:28: ValueError

During handling of the above exception, another exception occurred:
args = ['', 'pdf', 'nonfilepath'], expected_exception = <class 'ValueError'>
match = 'unknown engine'

    @pytest.mark.parametrize(
        'args, expected_exception, match',
        [(['', 'pdf', 'nonfilepath'], ValueError, r'unknown engine'),
         (['dot', '', 'nonfilepath'], ValueError, r'unknown format'),
         (['dot', 'ps', 'nonfilepath', '', None], ValueError, r'unknown renderer'),
         (['dot', 'ps', 'nonfilepath', None, 'core'],
          graphviz.RequiredArgumentError, r'without renderer'),
         (['dot', 'ps', 'nonfilepath', 'ps', ''], ValueError, r'unknown formatter')],
        ids=lambda x: getattr(x, '__name__', x))
    def test_render_unknown_parameter_raises(args, expected_exception, match):
>       with pytest.raises(expected_exception, match=match), pytest.deprecated_call():
E       Failed: DID NOT WARN. No warnings of type (<class 'DeprecationWarning'>, <class 'PendingDeprecationWarning'>, <class 'FutureWarning'>) were emitted.
E        Emitted warnings: [].

tests/backend/test_rendering.py:39: Failed

Similar goes for test_render_unknown_parameter_raises[args1-ValueError-unknown format].

xflr6 commented 7 months ago

Thanks for the report.

Can you try with ./run-tests.py (https://graphviz.readthedocs.io/en/stable/development.html#tests). On my side the missing warning only happens when running with tox. I tried downgrading to different older versions (https://tox.wiki/en/legacy/changelog.html) but so far did not manage to find a version/setting where the warning is issued as intended.

stanislavlevin commented 7 months ago

I have checked it, the same.

Either the test or the code is incorrect (don't know why supported_number is hardcoded to 3 for render).

During the test 3 args are passed to graphviz.render()

https://github.com/xflr6/graphviz/blob/6fe8c4e3bc3d5a8a053115e238f1cc974cd793b5/tests/backend/test_rendering.py#L31

https://github.com/xflr6/graphviz/blob/6fe8c4e3bc3d5a8a053115e238f1cc974cd793b5/graphviz/backend/rendering.py#L199-L200

In this case supported_number == len(args), the warning is not emitted:

https://github.com/xflr6/graphviz/blob/6fe8c4e3bc3d5a8a053115e238f1cc974cd793b5/graphviz/_tools.py#L153-L169

The warning pytest.deprecated_call catches in this test under certain circumstances is some different warning, not the expected one and completely unrelated. The warning message can be check with match:

https://docs.pytest.org/en/stable/reference/reference.html#pytest-deprecated-call

In the context manager form you may use the keyword argument match to assert that the warning matches a text or regex.

xflr6 commented 7 months ago

Thank you very much for digging in: it did not occurr to me that the test was indeed incorrect (only visible in tox).

stanislavlevin commented 7 months ago

Thanks, the fix has been verified :+1:

xflr6 commented 7 months ago

Great, improved the messages (ignore cls and self, use singular) and their matching in the tests in e5578d39009469df2b7c6743458970643e228226.