holoviz-dev / nbsite

Build a tested, sphinx-based website from notebooks
https://nbsite.pyviz.org
BSD 3-Clause "New" or "Revised" License
28 stars 14 forks source link

Testing the Markdown files that use the `pyodide` directive #242

Open maximlt opened 1 year ago

maximlt commented 1 year ago

Panel is the first HoloViz project to use the pyodide directive. It's easy to expect that more and more projects are going to use it. Before this happens, we should try to set up a system to handle it correctly. This issue is about testing files that contain that directive. I'm opening this issue here but could also have opened it on nbsmoke or some other places, it doesn't really matter.

In https://github.com/holoviz/panel/pull/3880 a test module was added to test the docs. I've copied below the test that goes through Markdown files and run them if then contain the pyodide directive (and also python code blocks?):

@doc_available
@pytest.mark.parametrize(
    "file", doc_files, ids=[str(f.relative_to(DOC_PATH)) for f in doc_files]
)
def test_markdown_codeblocks(file):
    from markdown_it import MarkdownIt

    exceptions = ("await", "pn.serve", "django")

    md_ast = MarkdownIt().parse(file.read_text(encoding="utf-8"))
    lines = ""
    for n in md_ast:
        if n.tag == "code" and n.info is not None:
            if "pyodide" in n.info.lower() or "python" in n.info.lower():
                if ">>>" not in n.content:
                    lines += n.content
    if lines:
        ast.parse(lines)

    if lines and not any(w in lines for w in exceptions):
        run(["python", "-c", lines], timeout=30, check=True)

Running the files with python -c is a good check. However I think that it'd be best to have the option to run these files as if they were a Jupyter Notebook, as it influences the code paths executed by HoloViews/Panel (thinking about the display machinery of HoloViews in particular). Does that make sense?

If so, I can imagine that instead of relying on the test above we would have a step that converts the Markdown files that use the pyodide directive to Jupyter Notebooks, which would then be tested the usual way (nbsmoke/nbval...).

(This by the way doesn't apply only to files that use the pyodide directive, I think we have and will have more and more Markdown files, some that are MyST text-based notebooks and some that contain python code blocks. We will need to find a consistent way to handle them across HoloViz)

cc @philippjfr @Hoxbro

hoxbro commented 1 year ago

The reason it also uses python blocks is to check if the code blocks could be run if copied/pasted. It found some errors, which were also updated in https://github.com/holoviz/panel/pull/3880. The test was meant to catch mistakes like import errors and name errors, not to verify that it could run top to bottom.

At first glance, I think it gives a false sense of security with your suggested approach of converting markdown files with pyodide code blocks to notebooks. This is mainly because we are not matching the environment and running the exact same code as used on the site. If the test fails with the notebook approach, we know there is a problem, but if they don't, we can't be 100% sure that it would also work on the site. It is better than the "python -c" approach, but not perfect.

I would rather see we test the converted pyodide markdown files with something like playwright.

maximlt commented 1 year ago

The reason it also uses python blocks is to check if the code blocks could be run if copied/pasted. It found some errors, which were also updated in https://github.com/holoviz/panel/pull/3880. The test was meant to catch mistakes like import errors and name errors, not to verify that it could run top to bottom.

Hmm I actually think this is a different issue, we should open another issue for the python code blocks, as if we test them we should do it consistently across HoloViz.

This is mainly because we are not matching the environment and running the exact same code as used on the site.

Right thanks I didn't think about the environment. Ideally the environment used to run the pyodide code cells would be exactly the same one as the one used to build the site. Is that possible currently @philippjfr ? I think however that they run the same code, i.e. whatever is in the pyodide directives.

I would rather see we test the converted pyodide markdown files with something like playwright.

I think that the pyodide directive implementation could, or actually should, have such tests. Every HoloViz project having Playwright tests is also an interesting idea. I don't know if such tests could be done outside of a doc build, but that could be a post doc build step (as anyway ideally doc builds should fail on errors https://github.com/pyviz-dev/nbsite/issues/236).

Note that testing these files as if they were Jupyter Notebooks is interesting for other things, especially if we make these files available to users in a nicer format (https://github.com/pyviz-dev/nbsite/issues/243).