From e340d024df0e6ba2ad0ac6d41516a93eecfc22bb Mon Sep 17 00:00:00 2001 From: Steve Kieffer Date: Sat, 20 Jul 2024 07:59:16 -0400 Subject: [PATCH] s: Halt build on pfsc- directives with bad opt block in Sphinx pages --- ...-invalid-opt-block.branchnews.improved.txt | 2 + server/pfsc/sphinx/errors.py | 33 +++++++++++++-- .../resources/repo/spx/err/v0.3.0/conf.py | 29 +++++++++++++ .../resources/repo/spx/err/v0.3.0/index.rst | 30 +++++++++++++ server/tests/test_widgets.py | 42 +++++++++---------- 5 files changed, 111 insertions(+), 25 deletions(-) create mode 100644 changelog.d/skieffer.catch-invalid-opt-block.branchnews.improved.txt create mode 100644 server/tests/resources/repo/spx/err/v0.3.0/conf.py create mode 100644 server/tests/resources/repo/spx/err/v0.3.0/index.rst diff --git a/changelog.d/skieffer.catch-invalid-opt-block.branchnews.improved.txt b/changelog.d/skieffer.catch-invalid-opt-block.branchnews.improved.txt new file mode 100644 index 0000000..9c991de --- /dev/null +++ b/changelog.d/skieffer.catch-invalid-opt-block.branchnews.improved.txt @@ -0,0 +1,2 @@ +Provide a helpful error message in case of invalid option blocks +in pfsc directives in Sphinx pages. diff --git a/server/pfsc/sphinx/errors.py b/server/pfsc/sphinx/errors.py index f7756d0..3c45d71 100644 --- a/server/pfsc/sphinx/errors.py +++ b/server/pfsc/sphinx/errors.py @@ -14,11 +14,37 @@ # limitations under the License. # # --------------------------------------------------------------------------- # +import re + import sphinx.util.logging from pfsc.constants import PFSC_SPHINX_CRIT_ERR_MARKER +# Catch cases in which the option block of a pfsc- directive is malformed. +# If these are allowed to be mere warnings, then your Proofscape repo can build, and yet +# you may have Sphinx pages in which some of your widgets simply don't appear at all! +INVALID_OPT_BLOCK_RE = re.compile('Error in "pfsc-\\w+" directive:\ninvalid option block\\.') + + +def should_elevate(record): + """ + Return a boolean saying whether the given warning record is one that should + be elevated to a build-halting error. + """ + # Our custom directives and roles will start their messages with the value + # of the `PFSC_SPHINX_CRIT_ERR_MARKER` constant if they want to halt the build. + if record.msg.find(PFSC_SPHINX_CRIT_ERR_MARKER) >= 0: + return True + + # Below we catch special built-in Sphinx/docutils warnings that are serious enough + # that the build should stop. + if INVALID_OPT_BLOCK_RE.search(record.msg): + return True + + return False + + class ProofscapeWarningIsErrorFilter(sphinx.util.logging.WarningIsErrorFilter): """ It can be tricky to halt the Sphinx build, with a helpful error message describing @@ -47,13 +73,12 @@ class with this one, before we form our `Sphinx` instance. The patch is performe the bottom of this module, so importing this into our Sphinx extension is enough. When we form our `Sphinx` instance, we set ``warningiserror=False`; however, our - custom filter looks for a special substring, the value of `PFSC_SPHINX_CRIT_ERR_MARKER`, - in the warning message. When that is present, then we ensure that the warning *is* raised - as a build-halting exception. + custom filter checks the warning message for special cases that we have determined + (see `should_elevate()` function) should be raised as build-halting exceptions. """ def filter(self, record): - if record.msg.find(PFSC_SPHINX_CRIT_ERR_MARKER) >= 0: + if should_elevate(record): # Since we're about to cause an exception to be raised, it doesn't matter # if we permanently alter the Sphinx app's `warningiserror` setting. # That app instance is not going to do anything more anyway. diff --git a/server/tests/resources/repo/spx/err/v0.3.0/conf.py b/server/tests/resources/repo/spx/err/v0.3.0/conf.py new file mode 100644 index 0000000..9e8ea13 --- /dev/null +++ b/server/tests/resources/repo/spx/err/v0.3.0/conf.py @@ -0,0 +1,29 @@ +import os +import sys +sys.path.insert(0, os.path.abspath('../..')) + +# Configuration file for the Sphinx documentation builder. +# +# For the full list of built-in configuration values, see the documentation: +# https://www.sphinx-doc.org/en/master/usage/configuration.html + +# -- Project information ----------------------------------------------------- +# https://www.sphinx-doc.org/en/master/usage/configuration.html#project-information + +project = 'sphinx-proofscape doc with errors' +copyright = '2024, author' +author = 'author' + +# -- General configuration --------------------------------------------------- +# https://www.sphinx-doc.org/en/master/usage/configuration.html#general-configuration + +extensions = [] + +templates_path = ['_templates'] +exclude_patterns = ['_build', 'Thumbs.db', '.DS_Store', 'venv'] + +# -- Options for HTML output ------------------------------------------------- +# https://www.sphinx-doc.org/en/master/usage/configuration.html#options-for-html-output + +#html_theme = 'alabaster' +#html_static_path = ['_static'] diff --git a/server/tests/resources/repo/spx/err/v0.3.0/index.rst b/server/tests/resources/repo/spx/err/v0.3.0/index.rst new file mode 100644 index 0000000..d8de6f6 --- /dev/null +++ b/server/tests/resources/repo/spx/err/v0.3.0/index.rst @@ -0,0 +1,30 @@ +sphinx-proofscape doc with errors +================================= + +.. pfsc:: + + deduc Thm { + asrt C { + sy="C" + } + meson = "C" + } + + deduc Pf of Thm.C { + asrt R { + sy="R" + } + asrt S { + sy="S" + } + meson = "R, so S, therefore Thm.C." + } + +This chart widget has an error, because the ``viewOpts`` field uses +improper indentation. (The closing brace needs to be further indented.) + +.. pfsc-chart:: + :view: Thm.C + :viewOpts: { + transition: false, + } diff --git a/server/tests/test_widgets.py b/server/tests/test_widgets.py index 1bec2ba..64602cd 100644 --- a/server/tests/test_widgets.py +++ b/server/tests/test_widgets.py @@ -459,36 +459,36 @@ def test_chart_widget_select_field(app, raw_select_val, final_select_val): .. pfsc-chart:: :view: Thm.C, Pf.{R,S} -""".strip() - -@pytest.mark.psm -def test_sphinx_directive_widget_field_pf_json_parse_err(app): - """ - Check that we get the expected error message when we try to build a Sphinx - page in which a widget (in directive form) field has a PF-JSON parse error. - """ - with app.app_context(): - with pytest.raises(PfscExcep) as ei: - build_repo('test.spx.err', version='v0.1.0', quiet=True) - pe = ei.value - s = str(pe) - n = s.find(spx_err_txt_1) - assert n > 0 +""" spx_err_txt_2 = """ /lib/test/spx/err/index.rst:23:PROOFSCAPE-SPHINX-ERROR: Inline Proofscape chart widgets must have the form `SUBTEXT <VIEW>`. -""".strip() +""" + +spx_err_txt_3 = """ +/lib/test/spx/err/index.rst:26:Error in "pfsc-chart" directive: +invalid option block. +""" @pytest.mark.psm -def test_sphinx_role_widget_malformed(app): +@pytest.mark.parametrize('version, err_msg', [ + # A widget in directive form has a field with a PF-JSON parse error. + ['v0.1.0', spx_err_txt_1], + # A widget in role form has malformed interpreted text. + ['v0.2.0', spx_err_txt_2], + # A widget in directive form uses improper indentation in its option block. + ['v0.3.0', spx_err_txt_3], +]) +def test_sphinx_widget_errors(app, version, err_msg): """ - Check that we get the expected error message when we try to build a Sphinx - page in which a widget in role form has malformed interpreted text. + Check that we get the expected error messages when we try to build a Sphinx + pages with malformed widgets. """ with app.app_context(): with pytest.raises(PfscExcep) as ei: - build_repo('test.spx.err', version='v0.2.0', quiet=True) + build_repo('test.spx.err', version=version, quiet=True) pe = ei.value s = str(pe) - n = s.find(spx_err_txt_2) + print('\n', s) + n = s.find(err_msg.strip()) assert n > 0