Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix test failure for Sphinx >= 2.2.2 #472

Merged
merged 1 commit into from
Feb 2, 2020

Conversation

D4N
Copy link
Contributor

@D4N D4N commented Feb 1, 2020

This fixes #465.

Sphinx changed the way it retrieves the CDomain in
sphinx-doc/sphinx@e42d546
to no longer use env.domaindata, but env.get_domain instead.

This breaks the mocks setup by breathe as they were implicitly only populatin
env.domaindata by invoking the constructor of the C and C++ domains (see
sphinx.domains.init: Domain.init). To properly populate the environment,
we need to make the mocked registry aware of the registered domains. Since we
only care about the C and C++ domain, we simply make it return both in
create_domains(), which registers them properly in the environment.

Note that this is unfortunately only a hack and a proper solution would be to port the testsuite to use sphinx' testing module instead, which in turn requires to use pytest instead of nosetests.

Sphinx changed the way it retrieves the CDomain in
sphinx-doc/sphinx@e42d546
to no longer use env.domaindata, but env.get_domain instead.

This breaks the mocks setup by breathe as they were implicitly only populatin
env.domaindata by invoking the constructor of the C and C++ domains (see
sphinx.domains.__init__: Domain.__init__). To properly populate the environment,
we need to make the mocked registry aware of the registered domains. Since we
only care about the C and C++ domain, we simply make it return both in
create_domains(), which registers them properly in the environment.
@t-b
Copy link
Contributor

t-b commented Feb 1, 2020

@D4N Nice! Your reasoning makes sense. I've checked locally that it works as advertised.

@vermeeren
Copy link
Collaborator

Thanks for the detailed analysis and the patches, all seems good to me.

I have created #473 as a potential follow-up issue for porting Breathe's tests using the information you found, which should be useful in the future.

@vermeeren vermeeren merged commit 946e3b1 into breathe-doc:master Feb 2, 2020
vermeeren added a commit that referenced this pull request Feb 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Problem in existing code test Unit tests, acceptance tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Breathe's tests fail for sphinx >= 2.2.2 with "Domain 'c' is not registered"
3 participants