Better tests, freeze all classes for better thread safety

packages/mrml-python/mrml.pyi

@@ -64,12 +64,18 @@ class ParserOptions:
 
 class RenderOptions:
     """RenderOptions configures rendering behavior, including whether to disable comments and how to handle social icons and fonts."""
-
-    disable_comments: bool
-    social_icon_origin: Optional[str]
-    fonts: Optional[Dict[str, str]]
-
-    def __init__(self) -> None: ...
+    def __init__(
+        self,
+        disable_comments: bool = False,
+        social_icon_origin: str | None = None,
+        fonts: Dict[str, str] | None = None,
+    ) -> None: ...
+    @property
+    def disable_comments(self) -> bool: ...
+    @property
+    def social_icon_origin(self) -> str | None: ...
+    @property
+    def fonts(self) -> Dict[str, str] | None: ...
 
 class Warning:
     @property

packages/mrml-python/src/lib.rs

@@ -10,19 +10,19 @@ use mrml::prelude::parser::noop_loader::NoopIncludeLoader;
 use pyo3::exceptions::PyIOError;
 use pyo3::prelude::*;
 
-#[pyclass]
+#[pyclass(frozen)]
 #[derive(Clone, Debug, Default)]
 pub struct NoopIncludeLoaderOptions;
 
-#[pyclass]
+#[pyclass(frozen)]
 #[derive(Clone, Debug, Default)]
 pub struct MemoryIncludeLoaderOptions(HashMap<String, String>);
 
-#[pyclass]
+#[pyclass(frozen)]
 #[derive(Clone, Debug, Default)]
 pub struct LocalIncludeLoaderOptions(PathBuf);
 
-#[pyclass(eq, eq_int)]
+#[pyclass(frozen, eq, eq_int)]
 #[derive(Clone, Debug, PartialEq, Eq)]
 pub enum HttpIncludeLoaderOptionsMode {
     Allow,
@@ -35,14 +35,14 @@ impl Default for HttpIncludeLoaderOptionsMode {
     }
 }
 
-#[pyclass]
+#[pyclass(frozen)]
 #[derive(Clone, Debug, Default)]
 pub struct HttpIncludeLoaderOptions {
     mode: HttpIncludeLoaderOptionsMode,
     list: HashSet<String>,
 }
 
-#[pyclass]
+#[pyclass(frozen)]
 #[derive(Clone, Debug)]
 pub enum ParserIncludeLoaderOptions {
     Noop(NoopIncludeLoaderOptions),
@@ -119,10 +119,10 @@ pub fn http_loader(
     })
 }
 
-#[pyclass]
+#[pyclass(frozen)]
 #[derive(Clone, Debug, Default)]
 pub struct ParserOptions {
-    #[pyo3(get, set)]
+    #[pyo3(get)]
     pub include_loader: ParserIncludeLoaderOptions,
 }
 
@@ -144,22 +144,31 @@ impl From<ParserOptions> for mrml::prelude::parser::ParserOptions {
     }
 }
 
-#[pyclass]
+#[pyclass(frozen)]
 #[derive(Clone, Debug, Default)]
 pub struct RenderOptions {
-    #[pyo3(get, set)]
+    #[pyo3(get)]
     pub disable_comments: bool,
-    #[pyo3(get, set)]
+    #[pyo3(get)]
     pub social_icon_origin: Option<String>,
-    #[pyo3(get, set)]
+    #[pyo3(get)]
     pub fonts: Option<HashMap<String, String>>,
 }
 
 #[pymethods]
 impl RenderOptions {
     #[new]
-    pub fn new() -> Self {
-        Self::default()
+    #[pyo3(signature = (disable_comments=false, social_icon_origin=None, fonts=None))]
+    pub fn new(
+        disable_comments: bool,
+        social_icon_origin: Option<String>,
+        fonts: Option<HashMap<String, String>>,
+    ) -> Self {
+        Self {
+            disable_comments,
+            social_icon_origin,
+            fonts,
+        }
     }
 }
 
@@ -182,10 +191,10 @@ impl From<RenderOptions> for mrml::prelude::render::RenderOptions {
     }
 }
 
-#[pyclass]
+#[pyclass(frozen)]
 #[derive(Clone, Debug, Default)]
 pub struct Warning {
-    #[pyo3(get, set)]
+    #[pyo3(get)]
     pub origin: Option<String>,
     #[pyo3(get)]
     pub kind: &'static str,
@@ -215,7 +224,7 @@ impl From<mrml::prelude::parser::Warning> for Warning {
     }
 }
 
-#[pyclass]
+#[pyclass(frozen)]
 #[derive(Clone, Debug, Default)]
 pub struct Output {
     #[pyo3(get)]

packages/mrml-python/tests/test_thread_safety.py

@@ -1,6 +1,7 @@
 import concurrent.futures
 
 import mrml
+import pytest
 
 
 def test_concurrent_simple_template():
@@ -17,14 +18,15 @@ def worker():
 
 
 def test_concurrent_memory_loader():
-    def worker():
-        parser_options = mrml.ParserOptions(
-            include_loader=mrml.memory_loader(
-                {
-                    "hello-world.mjml": "<mj-text>Hello World!</mj-text>",
-                }
-            )
+    parser_options = mrml.ParserOptions(
+        include_loader=mrml.memory_loader(
+            {
+                "hello-world.mjml": "<mj-text>Hello World!</mj-text>",
+            }
         )
+    )
+
+    def worker():
         result = mrml.to_html(
             '<mjml><mj-body><mj-include path="hello-world.mjml" /></mj-body></mjml>',
             parser_options=parser_options,
@@ -40,10 +42,11 @@ def worker():
 
 
 def test_concurrent_local_loader():
+    parser_options = mrml.ParserOptions(
+        include_loader=mrml.local_loader("./resources/partials")
+    )
+
     def worker():
-        parser_options = mrml.ParserOptions(
-            include_loader=mrml.local_loader("./resources/partials")
-        )
         result = mrml.to_html(
             '<mjml><mj-body><mj-include path="file:///hello-world.mjml" /></mj-body></mjml>',
             parser_options=parser_options,
@@ -59,13 +62,13 @@ def worker():
 
 
 def test_concurrent_http_loader():
+    http_loader = mrml.http_loader(
+        mode=mrml.HttpIncludeLoaderOptionsMode.Allow,
+        list=set(["https://gist.githubusercontent.com"]),
+    )
+    parser_options = mrml.ParserOptions(include_loader=http_loader)
+
     def worker():
-        parser_options = mrml.ParserOptions(
-            include_loader=mrml.http_loader(
-                mode=mrml.HttpIncludeLoaderOptionsMode.Allow,
-                list=set(["https://gist.githubusercontent.com"]),
-            )
-        )
         result = mrml.to_html(
             """<mjml>
       <mj-body>
@@ -90,34 +93,35 @@ def worker():
 
 def test_concurrent_mixed_operations():
     """Test different MRML operations running concurrently"""
+    memory_parser_options = mrml.ParserOptions(
+        include_loader=mrml.memory_loader(
+            {
+                "hello-world.mjml": "<mj-text>Hello World!</mj-text>",
+            }
+        )
+    )
+
+    local_parser_options = mrml.ParserOptions(
+        include_loader=mrml.local_loader("./resources/partials")
+    )
 
     def worker_simple():
         result = mrml.to_html("<mjml></mjml>")
         assert result.content.startswith("<!doctype html>")
         return "simple"
 
     def worker_memory():
-        parser_options = mrml.ParserOptions(
-            include_loader=mrml.memory_loader(
-                {
-                    "hello-world.mjml": "<mj-text>Hello World!</mj-text>",
-                }
-            )
-        )
         result = mrml.to_html(
             '<mjml><mj-body><mj-include path="hello-world.mjml" /></mj-body></mjml>',
-            parser_options=parser_options,
+            parser_options=memory_parser_options,
         )
         assert result.content.startswith("<!doctype html>")
         return "memory"
 
     def worker_local():
-        parser_options = mrml.ParserOptions(
-            include_loader=mrml.local_loader("./resources/partials")
-        )
         result = mrml.to_html(
             '<mjml><mj-body><mj-include path="file:///hello-world.mjml" /></mj-body></mjml>',
-            parser_options=parser_options,
+            parser_options=local_parser_options,
         )
         assert result.content.startswith("<!doctype html>")
         return "local"
@@ -137,27 +141,9 @@ def worker_local():
 
 
 def test_render_options_thread_safety():
-    """Test concurrent access with different render options"""
-
-    def worker(disable_comments: bool):
-        render_options = mrml.RenderOptions()
-        render_options.disable_comments = disable_comments
-        result = mrml.to_html(
-            "<mjml><mj-body><mj-text><!-- Comment --></mj-text></mj-body></mjml>",
-            render_options=render_options,
-        )
-        assert result.content.startswith("<!doctype html>")
-        return (disable_comments, result.content)
-
-    with concurrent.futures.ThreadPoolExecutor(max_workers=10) as executor:
-        task_count = 100
-        futures = []
-        for i in range(task_count):
-            futures.append(executor.submit(worker, i % 2 == 0))
-        results = [f.result() for f in futures]
-        assert len(results) == task_count
-        for result in results:
-            if result[0]:
-                assert "<!-- Comment -->" not in result[1]
-            else:
-                assert "<!-- Comment -->" in result[1]
+    """Test mutation throws AttributeError"""
+    render_options = mrml.RenderOptions(disable_comments=True)
+    assert render_options.disable_comments
+    assert render_options.social_icon_origin is None
+    with pytest.raises(AttributeError) as _:
+        render_options.disable_comments = False