sphinx-doc / sphinx

The Sphinx documentation generator
https://www.sphinx-doc.org/
Other
6.45k stars 2.1k forks source link

sphinx.testing is undocumented #7008

Open goerz opened 4 years ago

goerz commented 4 years ago

https://github.com/sphinx-doc/sphinx-testing is marked deprecated and points to sphinx.testing as a replacement. However, the only documentation for this is https://www.sphinx-doc.org/en/master/devguide.html#unit-testing, which does not provide any details.

My first assumption was that maybe it's sufficient to replace from sphinx_testing import with_app with from sphinx.testing import with_app, but that does not seem to be the case:

ImportError: cannot import name 'with_app' from 'sphinx.testing'

I can sort of figure out the use of sphinx.testing by looking at the source code for Sphinx' own tests, but nonetheless there should be proper documentation. The (closed) issue #3458 already pointed out that documentation is still a todo item.

tk0miya commented 4 years ago

Absolutely. It must be a task for us!

tk0miya commented 4 years ago

As a quick note:

from sphinx.testing.path import path

pytest_plugins = 'sphinx.testing.fixtures'

@pytest.fixture(scope='session') def rootdir(): return path(file).parent.abspath() / 'roots'

* It provides absolute path of root directory of sphinx projects for testing as `rootdir()`. With above settings, they are placed under `tests/roots`.
* Please put sphinx projects for testing under `tests/roots/test-*`.  By default, `test-root` is used for testing.
* Please write your test code.

import pytest

def test(app):

app is a Sphinx application object for default sphinx project (tests/roots/test-root).

app.build()

@pytest.mark.sphinx(buildername='latex') def test_latex(app):

latex builder is chosen here.

 app.build()

@pytest.mark.sphinx(testroot='case1') def test_case1(app):

app is Sphinx application for case1 sphinx project (tests/roots/test-case1)

app.build()

@pytest.mark.sphinx(confoverrides={'master_doc': 'content'}) def test_confoverrides(app):

a Sphinx application configured with given setting

app.build()


I'll make a document in detail if I have time...
jnikula commented 3 years ago

Proper documentation for sphinx.testing would really be appreciated.

I'm banging my head against the wall migrating just this simple @with_app usage from sphinx-testing:

@with_app(confdir=testdir, create_new_srcdir=True, buildername='text')

Looking at the sphinx.testing source, I can't find support for creating new srcdir or even having separate confdir and srcdir. The above "quick note" seems to assume some rigid test structure/layout I'm not interested in (tests/roots/test-*), and I don't really know what's recommendation and what's requirement.

jnikula commented 3 years ago

Getting off-topic, but is it actually SphinxTestApp that conflates confdir and srcdir while Sphinx itself supports split conf and src? This is certainly a step back from sphinx-testing.

jnikula commented 3 years ago

Also, with @with_app I could trivially set up two independent apps, build them both in the same unit test, and compare the outputs. How would I do that with the make_app fixture that kind of assumes one app per unit test?

danieleades commented 3 years ago

Is it worth pointing out the irony of a documentation generator having such poor documentation? ;)

tk0miya commented 3 years ago

Yes, We must admit our documentation is poor. It's real of our project. So we need your help. How about read the code and contribute?

tk0miya commented 3 years ago

Looking at the sphinx.testing source, I can't find support for creating new srcdir or even having separate confdir and srcdir.

Indeed, it's not supported. If you'd like to do that, please post a PR.

The above "quick note" seems to assume some rigid test structure/layout I'm not interested in (tests/roots/test-*), and I don't really know what's recommendation and what's requirement.

sphinx.testing is helper functions and classes for pytest. So there is no "requirements". You don't need to use any fixtures and decorators. Of course, you don't need to create "test-root" directories too.

jnikula commented 3 years ago

Looking at the sphinx.testing source, I can't find support for creating new srcdir or even having separate confdir and srcdir.

Indeed, it's not supported. If you'd like to do that, please post a PR.

I'm afraid it just gets too involved for me. The Sphinx code base isn't exactly easy to approach. And I mean no disrespect, that's pretty much the case for any non-trivial project. But here it's amplified by the fact that there's no documentation on how sphinx.testing is supposed to work and be used in the first place! Hence the bug at hand.

The end result is that I'm stuck with sphinx-testing for as long as it works, and after that I'm more likely to hack together something in my own project to work around the limitations.

I've been relying on sphinx-testing for years, and I didn't expect its features to be ditched in the curb without a straightforward migration path: https://github.com/sphinx-doc/sphinx-testing/issues/11

jnikula commented 3 years ago

On a related note, I did the bulk of the work to convert the Linux kernel to use Sphinx. One of the main criticisms towards Sphinx since then has been this kind of disregard of backwards compatibility. The kernel as a project does not want to impose latest tool version requirements to its developers or users, so we've had to work around the differences between Sphinx versions over time. Obviously it's your choice to develop the project as you see fit, but please take this as feedback from your users. It's not fun to have to keep catching up and adapting. Old stuff should keep working, and migrating to new stuff should be trivial.

astrojuanlu commented 2 years ago

I'm trying to follow this advice https://github.com/sphinx-doc/sphinx/issues/7008#issuecomment-573092764 to test my Sphinx extension. Is there a way to create the app with warningiserror=True, keep_going=True? I tried several combinations of @pytest.mark.sphinx and @pytest.mark.test_params without success @tk0miya

timobrembeck commented 2 years ago

@astrojuanlu Since warningiserror = False is hardcoded in SphinxTestApp, I think you would have to monkeypatch SphinxTestApp (or create a subclass and overwrite the corresponding make_app() fixture), but maybe I'm missing something.

astrojuanlu commented 2 years ago

For now I can do

def test(app):
    app.warningiserror = app.keep_going = True

although yes, it will get repetitive after some time. I can open a separate issue for this.

timobrembeck commented 2 years ago

Well in that case, you can just overwrite the app fixture in your conftest.py, right?

@pytest.fixture()
def app(app):
    app.warningiserror = app.keep_going = True
    return app
astrojuanlu commented 2 years ago

One more thing: with this framework, what would be the recommended way of testing that the app fails on creation, for example because of bad config? If one does this:

@pytest.mark.sphinx(confoverrides={"bad_config": "BOOM"})
def test_bad_config_raises_error(app: SphinxTestApp) -> None:
    app.build()
    assert app.statuscode != 0, "Expected build problem but it finished successfully"

(say that you have a setup function connected to the config-inited event that raises an error)

The test cannot even run, because the fixture cannot be initialized: the code crashes at line

https://github.com/sphinx-doc/sphinx/blob/7d7c59aaf22f110bfc7ed5a1385e6865ccf327fa/sphinx/testing/fixtures.py#L147

Example from https://github.com/astrojuanlu/sphinx-github-role/blob/4999ba506634e3e5dda6975ff00f4956bfe9ef5e/tests/test_github_role.py#L46-L53 ``` self = , name = 'config-inited', allowed_exceptions = (), args = (,), results = [] listeners = [EventListener(id=63, handler=, priority=500), EventListener(id=0, handl..., priority=800), EventListener(id=5, handler=, priority=800), ...] def emit(self, name: str, *args: Any, allowed_exceptions: Tuple[Type[Exception], ...] = ()) -> List: """Emit a Sphinx event.""" try: logger.debug('[app] emitting event: %r%s', name, repr(args)[:100]) except Exception: # not every object likes to be repr()'d (think # random stuff coming via autodoc) pass results = [] listeners = sorted(self.listeners[name], key=attrgetter("priority")) for listener in listeners: try: > results.append(listener.handler(self.app, *args)) .tox/py39/lib/python3.9/site-packages/sphinx/events.py:101: _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ = <[AttributeError("'NoneType' object has no attribute 'name'") raised in repr()] SphinxTestApp object at 0x7f4ca9be28b0>, config = def setup_github_role(_: Sphinx, config: Config) -> None: if "github_default_org_project" in config.values: default_organization, default_project = config["github_default_org_project"] # if not default_organization and default_project: if not default_organization and default_project: > raise ValueError( "GitHub default organization cannot be empty if default project is set" ) E ValueError: GitHub default organization cannot be empty if default project is set .tox/py39/lib/python3.9/site-packages/sphinx_github_role/__init__.py:30: ValueError The above exception was the direct cause of the following exception: test_params = {'shared_result': None}, app_params = app_params(args=[], kwargs={'confoverrides': {'github_default_org_project': ('', 'proj')}, 'srcdir': path('/tmp/pytest-of-juanlu/pytest-60/root')}) make_app = .make at 0x7f4ca9c2bee0>, shared_result = @pytest.fixture(scope='function') def app(test_params: Dict, app_params: Tuple[Dict, Dict], make_app: Callable, shared_result: SharedResult) -> Generator[SphinxTestApp, None, None]: """ Provides the 'sphinx.application.Sphinx' object """ args, kwargs = app_params > app_ = make_app(*args, **kwargs) .tox/py39/lib/python3.9/site-packages/sphinx/testing/fixtures.py:147: _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ .tox/py39/lib/python3.9/site-packages/sphinx/testing/fixtures.py:193: in make app_: Any = SphinxTestApp(*args, **kwargs) .tox/py39/lib/python3.9/site-packages/sphinx/testing/util.py:133: in __init__ super().__init__(srcdir, confdir, outdir, doctreedir, .tox/py39/lib/python3.9/site-packages/sphinx/application.py:261: in __init__ self.events.emit('config-inited', self.config) _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ self = , name = 'config-inited', allowed_exceptions = (), args = (,), results = [] listeners = [EventListener(id=63, handler=, priority=500), EventListener(id=0, handl..., priority=800), EventListener(id=5, handler=, priority=800), ...] def emit(self, name: str, *args: Any, allowed_exceptions: Tuple[Type[Exception], ...] = ()) -> List: """Emit a Sphinx event.""" try: logger.debug('[app] emitting event: %r%s', name, repr(args)[:100]) except Exception: # not every object likes to be repr()'d (think # random stuff coming via autodoc) pass results = [] listeners = sorted(self.listeners[name], key=attrgetter("priority")) for listener in listeners: try: results.append(listener.handler(self.app, *args)) except allowed_exceptions: # pass through the errors specified as *allowed_exceptions* raise except SphinxError: raise except Exception as exc: modname = safe_getattr(listener.handler, '__module__', None) > raise ExtensionError(__("Handler %r for event %r threw an exception") % (listener.handler, name), exc, modname=modname) from exc E sphinx.errors.ExtensionError: Handler for event 'config-inited' threw an exception (exception: BOOM) .tox/py39/lib/python3.9/site-packages/sphinx/events.py:109: ExtensionError ```
tk0miya commented 2 years ago

Is there a way to create the app with warningiserror=True, keep_going=True?

At present, there is no way to create such an application. So please override them after creating an object (as you and @timoludwig posted). BTW, no reason to keep them not configurable. Let's make them configurable? Could you send a PR?

tk0miya commented 2 years ago

One more thing: with this framework, what would be the recommended way of testing that the app fails on creation, for example because of bad config? If one does this:

Please use make_app fixture instead of app. https://github.com/sphinx-doc/sphinx/blob/647314133d7a9a524087eddd9c21203010cb4aa7/tests/test_config.py#L157-L162

timobrembeck commented 2 years ago

One more thing: with this framework, what would be the recommended way of testing that the app fails on creation, for example because of bad config?

Just as an addition to @tk0miya's answer: I have multiple test cases with different config overrides and used a fixture for it.

in conftest.py:

@pytest.fixture()
def make_app_with_different_config(app_params, make_app):
    def inner(**confoverrides):
        args, kwargs = app_params
        kwargs["confoverrides"] = confoverrides
        _app = make_app(*args, **kwargs)
        return _app
    return inner

in test_config.py:

def test_setup_with_bad_config(make_app_with_different_config):
    with pytest.raises(ConfigError):
        make_app_with_different_config("bad_config": "BOOM")