diff --git a/marimo/_ast/load.py b/marimo/_ast/load.py index b207adc987b..44192e1b4ab 100644 --- a/marimo/_ast/load.py +++ b/marimo/_ast/load.py @@ -35,6 +35,15 @@ @dataclass class LoadResult: + """Result of attempting to load a marimo notebook. + + status can be one of: + - empty: No content, or only comments / a doc string + - has_errors: Parsed, but has marimo-specific errors (**can load!!**) + - invalid: Could not be parsed as a marimo notebook (**cannot load**) + - valid: Parsed and valid marimo notebook + """ + status: Literal["empty", "has_errors", "invalid", "valid"] = "empty" notebook: Optional[NotebookSerialization] = None contents: Optional[str] = None diff --git a/tests/_ast/codegen_data/_test_not_parsable.py b/tests/_ast/codegen_data/_test_not_parsable.py new file mode 100644 index 00000000000..0a3de33cad0 --- /dev/null +++ b/tests/_ast/codegen_data/_test_not_parsable.py @@ -0,0 +1,2 @@ +# INTENTIONALLY BROKEN! +print("hello world diff --git a/tests/_ast/codegen_data/_test_parse_error_in_notebook.py b/tests/_ast/codegen_data/_test_parse_error_in_notebook.py new file mode 100644 index 00000000000..2a857d072de --- /dev/null +++ b/tests/_ast/codegen_data/_test_parse_error_in_notebook.py @@ -0,0 +1,27 @@ +import marimo + +__generated_with = "0.17.4" +app = marimo.App(width="medium") + + +@app.cell +def _(mo): + mo.md(""" + r"""Markdown cell with error""" + """) + return + + +@app.cell +def _(): + import marimo as mo + return (mo,) + + +@app.cell +def _(): + return + + +if __name__ == "__main__": + app.run() diff --git a/tests/_ast/codegen_data/test_get_app_kwargs.py b/tests/_ast/codegen_data/test_get_app_kwargs.py index babd68a3fba..fbbc3e10154 100644 --- a/tests/_ast/codegen_data/test_get_app_kwargs.py +++ b/tests/_ast/codegen_data/test_get_app_kwargs.py @@ -1,5 +1,4 @@ import marimo - app = marimo.App( app_title="title", auto_download="html", diff --git a/tests/_ast/codegen_data/test_with_bad_decorator.py b/tests/_ast/codegen_data/test_with_decorator.py similarity index 100% rename from tests/_ast/codegen_data/test_with_bad_decorator.py rename to tests/_ast/codegen_data/test_with_decorator.py diff --git a/tests/_ast/test_codegen.py b/tests/_ast/test_codegen.py index c2d50e0314f..209e23ec0c1 100644 --- a/tests/_ast/test_codegen.py +++ b/tests/_ast/test_codegen.py @@ -1,6 +1,7 @@ # Copyright 2024 Marimo. All rights reserved. from __future__ import annotations +import ast import json from functools import partial from inspect import cleandoc @@ -722,6 +723,24 @@ def test_fn_with_all_config(self) -> None: assert fndef == expected +def test_markdown_invalid() -> None: + expected = wrap_generate_filecontents( + ['mo.md("Unclosed string)', "import marimo as mo"], ["a", "b"] + ) + ast.parse(expected) # should not raise + expected = wrap_generate_filecontents( + ['mo.md("Unclosed call"', "import marimo as mo"], ["a", "b"] + ) + ast.parse(expected) # should not raise + + +def test_markdown_with_alt_strings() -> None: + expected = wrap_generate_filecontents( + ['mo.md(\'has """\')', "import marimo as mo"], ["a", "b"] + ) + ast.parse(expected) # should not raise + + def test_recover(tmp_path: Path) -> None: cells: NotebookV1 = { "version": "1", diff --git a/tests/_ast/test_load.py b/tests/_ast/test_load.py index 3443b210b9a..c004e97931f 100644 --- a/tests/_ast/test_load.py +++ b/tests/_ast/test_load.py @@ -291,8 +291,8 @@ def test_get_bad_kwargs(load_app, caplog) -> None: assert "kwarg_that_doesnt_exist" in caplog.text @staticmethod - def test_get_app_with_bad_decorator(static_load) -> None: - app = static_load(get_filepath("test_with_bad_decorator")) + def test_get_app_with_decorator(static_load) -> None: + app = static_load(get_filepath("test_with_decorator")) assert app is not None assert app._cell_manager.get_cell_data_by_name("wrap").cell.defs == { "wrap" @@ -300,38 +300,86 @@ def test_get_app_with_bad_decorator(static_load) -> None: assert app._cell_manager.get_cell_data_by_name( "hundred" ).cell.defs == {"hundred"} - from codegen_data.test_with_bad_decorator import hundred + from codegen_data.test_with_decorator import hundred assert hundred == 100 class TestGetStatus: @staticmethod - def test_get_status_valid() -> None: - assert ( - load.get_notebook_status(get_filepath("test_main")).status - == "valid" - ) - - @staticmethod - def test_get_status_empty() -> None: - assert ( - load.get_notebook_status(get_filepath("test_empty")).status - == "empty" - ) + @pytest.mark.parametrize( + ("filename", "expected_status"), + [ + # Valid marimo apps + ("test_main", "valid"), + ("test_generate_filecontents", "valid"), + ("test_generate_filecontents_async", "valid"), + ("test_generate_filecontents_async_long_signature", "valid"), + ("test_generate_filecontents_single_cell", "valid"), + ("test_generate_filecontents_toplevel", "valid"), + ("test_generate_filecontents_toplevel_pytest", "valid"), + ("test_decorators", "valid"), + ("test_get_codes_multiline_string", "valid"), + ("test_get_codes_messy", "valid"), + ("test_get_codes_single_line_fn", "valid"), + ("test_get_codes_multiline_fndef", "valid"), + ("test_get_codes_comment_after_sig", "valid"), + ("test_get_codes_empty", "valid"), + ("test_get_header_comments", "valid"), + ("test_get_setup", "valid"), + ("test_get_setup_blank", "valid"), + ("test_generate_filecontents_shadowed_builtin", "valid"), + ("test_generate_filecontents_unshadowed_builtin", "valid"), + ("test_app_with_annotation_typing", "valid"), + ("test_long_line_in_main", "valid"), + ("test_with_decorator", "valid"), + # Potentially confusing but valid + ("test_get_codes_with_name_error", "valid"), # runtime error + ("test_syntax_errors", "valid"), # runtime syntax errors + # Broken signature, but fine otherwise + ("test_get_codes_with_incorrect_args_rets", "valid"), + # unparse cells + ("test_generate_filecontents_with_syntax_error", "valid"), + # Empty files + ("test_empty", "empty"), + # No cells + ("test_app_with_only_comments", "invalid"), + # Invalid (not marimo apps) + ("test_invalid", "invalid"), + # Has errors + ("test_get_codes_messy_toplevel", "has_errors"), + ("test_get_header_comments_invalid", "has_errors"), + ("test_get_bad_kwargs", "has_errors"), + # Unparsable + ( + "test_get_codes_non_marimo_python_script", + "invalid", + ), # not marimo + # Potentially confusing and has_errors + ("test_get_alias_import", "has_errors"), # not official format + ("test_get_app_kwargs", "has_errors"), # Intentionally bad kwargs + # Empty files can still be opened. + ( + "test_generate_filecontents_empty_with_config", + "has_errors", + ), # no body + ("test_generate_filecontents_empty", "has_errors"), # no body + ("test_app_with_no_cells", "has_errors"), # No body is an error + # Syntax errors in code + ("_test_not_parsable", "broken"), + ("_test_parse_error_in_notebook", "broken"), + # A script that is not a marimo notebook, but uses marimo is + # indeterminant, so throws an exception. + ("test_non_marimo", "broken"), + ], + ) + def test_get_status(filename: str, expected_status: str) -> None: + if expected_status == "broken": + with pytest.raises((MarimoFileError, SyntaxError)): + load.get_notebook_status(get_filepath(filename)) + return - @staticmethod - def test_get_status_invalid() -> None: - assert ( - load.get_notebook_status(get_filepath("test_invalid")).status - == "invalid" - ) - - @staticmethod - def test_get_status_warn() -> None: assert ( - load.get_notebook_status( - get_filepath("test_get_codes_messy_toplevel") - ).status - == "has_errors" + load.get_notebook_status(get_filepath(filename)).status + == expected_status )