fix: reuse python worker between tests (#52617)

curriculum/challenges/english/20-upcoming-python/learn-python-by-building-a-blackjack-game/64a5229b99ff0e8250cd9a72.md

@@ -20,6 +20,16 @@ Test 1
   } })
 ```
 
+Test 2
+
+```js
+({ input: ["Bob", "Carnes"], test: () => {
+  assert.equal( "Bob", __userGlobals.get("name"));
+  assert.equal( "Carnes", __userGlobals.get("last_name"));
+  } })
+```
+
+
 # --seed--
 
 ## --seed-contents--

curriculum/test/test-challenges.js

@@ -103,6 +103,8 @@ spinner.text = 'Populate tests.';
 
 let browser;
 let page;
+// This worker can be reused since it clears its environment between tests.
+let pythonWorker;
 
 setup()
   .then(runTests)
@@ -142,6 +144,10 @@ async function setup() {
     ]
   });
   global.Worker = createPseudoWorker(await newPageContext(browser));
+
+  pythonWorker = createWorker(pythonTestEvaluator, {
+    terminateWorker: false
+  });
   page = await newPageContext(browser);
   await page.setViewport({ width: 300, height: 150 });
 
@@ -551,21 +557,15 @@ async function createTestRunner(
   const runsInBrowser = buildChallenge === buildDOMChallenge;
   const runsInPythonWorker = buildChallenge === buildPythonChallenge;
 
-  const testEvaluator = runsInPythonWorker
-    ? pythonTestEvaluator
-    : javaScriptTestEvaluator;
-
-  // The python worker clears the globals between tests, so it should be fine
-  // to use the same evaluator for all tests. TODO: check if this is true for
-  // sys, since sys.modules is not being reset.
-  const workerConfig = {
-    testEvaluator,
-    options: { terminateWorker: !runsInPythonWorker }
-  };
-
   const evaluator = await (runsInBrowser
     ? getContextEvaluator(build, sources, code, loadEnzyme)
-    : getWorkerEvaluator(build, sources, code, removeComments, workerConfig));
+    : getWorkerEvaluator(
+        build,
+        sources,
+        code,
+        removeComments,
+        runsInPythonWorker
+      ));
 
   return async ({ text, testString }) => {
     try {
@@ -630,10 +630,14 @@ async function getWorkerEvaluator(
   sources,
   code,
   removeComments,
-  workerConfig
+  runsInPythonWorker
 ) {
-  const { testEvaluator, options } = workerConfig;
-  const testWorker = createWorker(testEvaluator, options);
+  // The python worker clears the globals between tests, so it should be fine
+  // to use the same evaluator for all tests. TODO: check if this is true for
+  // sys, since sys.modules is not being reset.
+  const testWorker = runsInPythonWorker
+    ? pythonWorker
+    : createWorker(javaScriptTestEvaluator, { terminateWorker: true });
   return {
     evaluate: async (testString, timeout) =>
       await testWorker.execute(

tools/client-plugins/browser-scripts/python-test-evaluator.ts

@@ -63,6 +63,11 @@ ctx.onmessage = async (e: PythonRunEvent) => {
 
   const assert = chai.assert;
   const __helpers = helpers;
+
+  // Create fresh globals for each test
+  // eslint-disable-next-line @typescript-eslint/no-unsafe-call
+  const __userGlobals = pyodide.globals.get('dict')() as PyProxy;
+
   /* eslint-enable @typescript-eslint/no-unused-vars */
   // uncomment the following line to inspect
   // the frame-runner as it runs tests
@@ -109,14 +114,24 @@ ctx.onmessage = async (e: PythonRunEvent) => {
       }
     };
 
+    // Clear out the old import otherwise it will use the old input/print
+    // functions
+    pyodide.runPython(`
+import sys
+try:
+  del sys.modules['jscustom']
+  del jscustom
+except (KeyError, NameError):
+  pass
+
+`);
+
     // Make input available to python (print is not used yet)
     pyodide.registerJsModule('jscustom', {
       input: testInput
       // print: () => {}
     });
-    // Create fresh globals for each test
-    // eslint-disable-next-line @typescript-eslint/no-unsafe-call
-    const __userGlobals = pyodide.globals.get('dict')() as PyProxy;
+
     // Some tests rely on __name__ being set to __main__ and we new dicts do not
     // have this set by default.
     // eslint-disable-next-line @typescript-eslint/no-unsafe-call
@@ -162,5 +177,7 @@ ctx.onmessage = async (e: PythonRunEvent) => {
         actual: (err as { actual?: string }).actual
       }
     });
+  } finally {
+    __userGlobals.destroy();
   }
 };

tools/client-plugins/browser-scripts/python-worker.ts

@@ -135,8 +135,8 @@ function initRunPython() {
       return ""
   `);
   // eslint-disable-next-line @typescript-eslint/no-unsafe-call
-  const getResetId = globals.get('__get_reset_id') as () => string;
-  return { runPython, getResetId };
+  const getResetId = globals.get('__get_reset_id') as PyProxy & (() => string);
+  return { runPython, getResetId, globals };
 }
 
 ctx.onmessage = (e: PythonRunEvent | ListenRequestEvent | CancelEvent) => {
@@ -165,7 +165,7 @@ function handleRunRequest(data: PythonRunEvent['data']) {
   // TODO: use reset-terminal for clarity?
   postMessage({ type: 'reset' });
 
-  const { runPython, getResetId } = initRunPython();
+  const { runPython, getResetId, globals } = initRunPython();
   // use pyodide.runPythonAsync if we want top-level await
   try {
     runPython(code);
@@ -184,5 +184,8 @@ function handleRunRequest(data: PythonRunEvent['data']) {
       // ...and tell the client that we're ignoring them.
       postMessage({ type: 'stopped', text: getResetId() });
     }
+  } finally {
+    getResetId.destroy();
+    globals.destroy();
   }
 }