intersphinx: Handle the case where intersphinx_cache_limit is negative

[SilverRainZ]

Subject: intersphinx: Handle the case where intersphinx_cache_limit is negative

Feature or Bugfix

• Bugfix

Purpose

Ensuring the confval intersphinx_cache_limit behaves as documented.

Detail

The documentation said:

Set this to a negative value to cache inventories
for unlimited time.

In the current implementation, a negative intersphinx_cache_limit causes
inventories always expire, this patch ensures that it behaves as documented.

Relates

• https://www.sphinx-doc.org/en/master/usage/extensions/intersphinx.html#confval-intersphinx_cache_limit

[picnixz]

Can you add some tests for that as well?

By the way, I'm surprised no one ever complained...

[SilverRainZ]

@picnixz I have added a test case, but only test the negative case, I am not sure how to test the positive one for it needs more mocked data.

[picnixz]

I am not sure how to test the positive one for it needs more mocked data.

If I have time tomorrow (I've got some reviews to do + some stuff to complete on CPython), I'll make a PR on your forked branch (or just dump the code here). But I'm not sure I'll have time. Otherwise, I think it's fine. I just need to remember how I mocked my own tests for those kind of things...

[SilverRainZ]

rebased to master again.

[SilverRainZ]

I can not run my test because of the following error:

\ $ python -m pytest -v tests/test_extensions/test_ext_intersphinx.py::test_intersphinx_cache_limit
ImportError while loading conftest '/Users/bytedance/git/public/sphinx-la/tests/conftest.py'.
tests/conftest.py:15: in <module>
    from sphinx.testing.util import _clean_up_global_state
sphinx/testing/util.py:17: in <module>
    import sphinx.application
sphinx/application.py:30: in <module>
    from sphinx.highlighting import lexer_classes
sphinx/highlighting.py:86: in <module>
    class PygmentsBridge:
sphinx/highlighting.py:89: in PygmentsBridge
    html_formatter = HtmlFormatter[str]
E   TypeError: type 'HtmlFormatter' is not subscriptable

[picnixz]

You should update your version of pygments to include this commit: pygments/pygments@0f40614 (though it's weird because your PR is more recent ?)

[AA-Turner]

I am not sure how to test the positive one for it needs more mocked data.

If I have time tomorrow (I've got some reviews to do + some stuff to complete on CPython), I'll make a PR on your forked branch (or just dump the code here). But I'm not sure I'll have time. Otherwise, I think it's fine. I just need to remember how I mocked my own tests for those kind of things...

@picnixz did you want to add these tests or are we OK to merge?

A

[picnixz]

did you want to add these tests or are we OK to merge?

I shamefully forgot. I'm fine with your test (much smaller, less complicate).

[AA-Turner]

I shamefully forgot. I'm fine with your test (much smaller, less complicate).

There's no shame in being busy! Thanks for the quick reply

A