Remove subprocess usage in test_examples.py for Pyodide support

[yeicor]

As discussed on Discord, the subprocess module is not supported in browser environments like Pyodide. To address this, I replaced the subprocess calls in test_examples.py with exec-based evaluation.

While exec does not provide the same isolation as spawning a new Python process, it's acceptable here because we control the example scripts being tested.

I already verified that the current examples run as expected in both Pyodide and standard Python.

[gumyr]

This PR drops the checking of the return code. Copilot suggests that the following would capture the output and check like the original did:

exit_code = 0
try:
    with contextlib.redirect_stdout(stdout_buffer), contextlib.redirect_stderr(stderr_buffer):
        exec(_MOCK_OCP_VSCODE_CONTENTS + example_source, {})
except SystemExit as e:
    exit_code = e.code if isinstance(e.code, int) else 1
finally:
    os.chdir(oldwd)
    sys.modules.pop("ocp_vscode", None)

self.assertEqual(
    0, exit_code, f"stdout/stderr: {stdout_buffer.getvalue()} / {stderr_buffer.getvalue()}"
)

What do you think, is it worth doing the checking?

[yeicor]

I ran a quick test to check how pytest handles sys.exit() and stdout/stderr during test execution.

If I add the following to the end of e.g. vase_algebra.py:

import sys
print("This is some error message that is useful for debugging")
sys.exit(1)

And now I run all example tests with python -m pytest tests/test_examples.py, I get the following results:

[...]
collected 77 items                                          

tests/test_examples.py .............................. [ 38%]
.............................................F.                          [100%]

=================================== FAILURES ===================================
______________________ TestExamples.test_vase_algebra_py _______________________
[...]
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

>   ???
E   SystemExit: 1

<string>:47: SystemExit
----------------------------- Captured stdout call -----------------------------
This is some error message that is useful for debugging
[...]

As you can see, it includes the stdout and the exit code of the failed test. Hence, it seems that Copilot's suggestion adds unnecessary "complexity" that is already handled by pytest.

[fischman]

FWIW the use of subprocess in the checked-in state was intentional, in order to run each example in isolation, to ensure that they remained individually valid (i.e. an example didn't silently rely on a symbol from another example, or any other observable state of the python process such as imported modules, etc). I think it would be valuable to retain this property on platforms that support it (i.e. non-WASM).

[jdegenstein]

@fischman if that seems valuable to you, I am ok with it -- can add a platform specific branch like:

if sys.platform == 'emscripten':
    # pyodide specific
else:
    # existing behavior with subprocess

What do you think?

[yeicor]

Thanks for the review @fischman. I believe that the exec isolation problem is not as bad as you make it seem: the trick is that I use an empty dictionary as the globals so that the examples do not modify the global namespace. I relied on chatgpt to find possible issues and I discuss them at the end.

====================

Yes, you can pass an empty dictionary ({}) to the globals parameter of Python 3’s exec() function to avoid modifying the actual global state, but with a few caveats and nuances you should be aware of:

---

✅ What Happens When You Pass {} to globals:

code = "a = 42"
exec(code, {})

• The code is executed with the given dictionary as the global namespace.

• This prevents pollution of your actual global namespace (e.g., variables or functions defined in your module).

• Any variables defined by the executed code go into that empty dictionary, not your global scope.

---

⚠️ But There Are Important Details:

1. Minimal Built-ins by Default:

   • If you just pass {} to globals, there will be no __builtins__, meaning things like print, len, range, etc., will be undefined unless you include them manually:

     safe_globals = {"__builtins__": __builtins__}
     exec("print('Hello')", safe_globals)  # Works
     exec("print('Hello')", {})  # Raises NameError
     

2. Local Scope Defaults to Global Scope If Omitted:

   • If you only pass globals, then locals defaults to the same dict. So the code will use the same dict for both local and global variables.

3. Does Not Prevent Side Effects:

   • Even if you isolate the namespace, the executed code can still perform side effects like:

     • Writing to disk

     • Importing modules

     • Modifying objects you pass in

   • So you’re only isolating namespace pollution, not necessarily security or system safety.

4. Code Can Still Modify Objects You Put in the Globals:

   • For example, if you pass in a global variable that refers to a mutable object, exec() code can mutate it.

====================

And these are my answers to the caveats:

1. This is not true (see the docs).
2. This is not a problem for the current use case.
3. Writing to disk would still be a problem with subprocess, importing modules seems to be ok as they are forgotten after exec() finishes (see demo of python repl below).
4. We are not sending any global that can be modified.

>>> exec("import math; a = math.sin(1)", {})
>>> a
Traceback (most recent call last):
  File "<python-input-1>", line 1, in <module>
    a
NameError: name 'a' is not defined
>>> math.sin(1)
Traceback (most recent call last):
  File "<python-input-2>", line 1, in <module>
    math.sin(1)
    ^^^^
NameError: name 'math' is not defined. Did you forget to import 'math'?

According to the demo above, I shouldn't even have to forget the ocp_vscode module as I did just in case in the patch. I can update this and write a comment about exec's behavior.

Assuming that the examples code is well-intentioned, using an isolated globals dict with exec() seems sufficient to avoid unintended global state changes, but correct me if I'm wrong. I’d personally prefer to avoid including Pyodide-specific code in build123d, but it’s not my call, and I’m fine with it if it's ultimately the chosen direction.

[fischman]

Thanks for looking into it, @yeicor. I think the overview generated by chatgpt is incomplete. For example imported modules are singletons in Python, and importing a module inside the exec context gets you a (mutable!) handle on the same module object outside the exec context, enabling leakage.

I sympathize with wanting to avoid pyodide-specific code in b123d.
One way to accomplish this would be to patch subprocess in the pyodide test runner to swap the stdlib's (C) fork/exec impl of subprocess with your (py) exec impl.

[yeicor]

Ok, I get it, things like this could have unintended effects:

>>> exec("import math; math.b = 'c'", {})
>>> import math
>>> math.b
'c'

I'm ok with any of the suggested solutions so far. I'll let the maintainers decide and update this pull request if required.