feat: reinstate support for object syntax (#409)

* Revert "fix: handle invalid python syntax (#408)"

This reverts commit 2cb62d8.

* Revert "feat: drop support for object form python tests (#401)"

This reverts commit d92cf4a.

* feat: stop supporting simple tests + input arrays

Author: ojeytonwilliams

packages/python-evaluator/src/python-test-evaluator.ts

@@ -22,6 +22,10 @@ import { format } from "../../shared/src/format";
 import { ProxyConsole } from "../../shared/src/proxy-console";
 import { createAsyncIife } from "../../shared/src/async-iife";
 
+type EvaluatedTeststring = {
+  test: () => Promise<unknown>;
+};
+
 const READY_MESSAGE: ReadyEvent["data"] = { type: "ready" };
 
 function isProxy(raw: unknown): raw is PyProxy {
@@ -98,30 +102,64 @@ class PythonTestEvaluator implements TestEvaluator {
 
       /* eslint-enable @typescript-eslint/no-unused-vars */
 
-      try {
-        // If input isn't reassigned, it will throw when called during testing.
-        runPython(`
+      // If input is not faked, tests can fail with io exceptions.
+      runPython(`
 def __fake_input(arg=None):
   return ""
 
 input = __fake_input
 `);
-        eval(opts.hooks?.beforeEach ?? "");
 
-        const iifeTest = createAsyncIife(testString);
+      try {
+        eval(opts.hooks?.beforeEach ?? "");
+        // Eval test string to get the dummy input and actual test
+        const evaluatedTestString = await new Promise<unknown>(
+          (resolve, reject) => {
+            try {
+              const test: unknown = eval(testString);
+              resolve(test);
+            } catch (err) {
+              const isUsingTopLevelAwait =
+                err instanceof SyntaxError &&
+                err.message.includes(
+                  "await is only valid in async functions and the top level bodies of modules",
+                );
+
+              if (isUsingTopLevelAwait) {
+                const iifeTest = createAsyncIife(testString);
+
+                // eslint-disable-next-line @typescript-eslint/no-unsafe-call, @typescript-eslint/no-unsafe-member-access
+                eval(iifeTest).then(resolve).catch(reject);
+              } else {
+                // eslint-disable-next-line @typescript-eslint/prefer-promise-reject-errors
+                reject(err);
+              }
+            }
+          },
+        );
+
+        // If the test string does not evaluate to an object, then we assume that
+        // it's a standard JS test and any assertions have already passed.
+        if (typeof evaluatedTestString !== "object") {
+          // Execute afterEach hook if it exists
+          if (opts.hooks?.afterEach) eval(opts.hooks.afterEach);
 
-        // Evaluates the learner's code so that any variables they
-        // define are available to the test.
+          return { pass: true, ...this.#proxyConsole.flush() };
+        }
 
-        try {
-          runPython(opts.source ?? "");
-        } catch {
-          // Tests should not automatically fail if there's an error in the
-          // source. Various tests are only interested in using regexes on the
-          // code.
+        if (!evaluatedTestString || !("test" in evaluatedTestString)) {
+          throw Error(
+            "Test string did not evaluate to an object with the 'test' property",
+          );
         }
 
-        await eval(iifeTest);
+        const { test } = evaluatedTestString as EvaluatedTeststring;
+
+        // Evaluates the learner's code so that any variables they define are
+        // available to the test.
+        runPython(opts.source ?? "");
+
+        await test();
 
         return { pass: true, ...this.#proxyConsole.flush() };
       } catch (err) {

packages/tests/integration-tests/index.test.ts

@@ -1205,7 +1205,11 @@ assert(mocked.find('.greeting').length === 1);
           type: "python",
           source,
         });
-        return runner?.runTest(`assert.equal(runPython('get_five()'), 5)`);
+        return runner?.runTest(
+          `({
+test: () => assert.equal(runPython('get_five()'), 5),
+						})`,
+        );
       }, source);
 
       expect(result).toEqual({ pass: true });
@@ -1224,7 +1228,11 @@ assert(mocked.find('.greeting').length === 1);
       const result = await page.evaluate(async () => {
         const runner = window.FCCTestRunner.getRunner("python");
         await runner?.init({}, 1000);
-        return runner?.runTest(`assert.equal(runPython('get_five()'), 5)`);
+        return runner?.runTest(
+          `({
+test: () => assert.equal(runPython('get_five()'), 5),
+						})`,
+        );
       });
 
       expect(result).toMatchObject({
@@ -1243,7 +1251,9 @@ assert(mocked.find('.greeting').length === 1);
           type: "python",
         });
         return runner?.runTest(
-          `assert.equal(runPython('__name__'), '__main__')`,
+          `({
+test: () => assert.equal(runPython('__name__'), '__main__'),
+						})`,
         );
       });
 
@@ -1274,6 +1284,26 @@ assert(mocked.find('.greeting').length === 1);
       });
     });
 
+    it("should reject testStrings that evaluate to an invalid object", async () => {
+      const result = await page.evaluate(async () => {
+        const runner = await window.FCCTestRunner.createTestRunner({
+          type: "python",
+        });
+        return runner?.runTest(`({ invalid: 'test' })`);
+      });
+
+      expect(result).toEqual({
+        err: {
+          message:
+            "Test string did not evaluate to an object with the 'test' property",
+          // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
+          stack: expect.stringContaining(
+            "Error: Test string did not evaluate to an object with the 'test' property",
+          ),
+        },
+      });
+    });
+
     it("should make user code available to the python code as the _code variable", async () => {
       const result = await page.evaluate(async () => {
         const runner = await window.FCCTestRunner.createTestRunner({
@@ -1283,9 +1313,9 @@ assert(mocked.find('.greeting').length === 1);
           },
         });
 
-        return runner?.runTest(
-          `assert.equal(runPython('_code'), "test = 'value'")`,
-        );
+        return runner?.runTest(`({
+  test: () => assert.equal(runPython('_code'), "test = 'value'")
+})`);
       });
 
       expect(result).toEqual({ pass: true });
@@ -1296,9 +1326,9 @@ assert(mocked.find('.greeting').length === 1);
         const runner = await window.FCCTestRunner.createTestRunner({
           type: "python",
         });
-        return runner?.runTest(
-          `assert.equal(runPython('_Node("x = 1").get_variable("x")'), 1)`,
-        );
+        return runner?.runTest(`({
+  test: () => assert.equal(runPython('_Node("x = 1").get_variable("x")'), 1)
+})`);
       });
 
       expect(result).toEqual({ pass: true });
@@ -1316,7 +1346,9 @@ assert(mocked.find('.greeting').length === 1);
           },
         });
 
-        return runner?.runTest("");
+        return runner?.runTest(`({
+  test: () => {}
+})`);
       });
 
       expect(result).toEqual({ pass: true });
@@ -1342,7 +1374,9 @@ assert(mocked.find('.greeting').length === 1);
         const runner = await window.FCCTestRunner.createTestRunner({
           type: "python",
         });
-        return runner?.runTest(`assert.equal(runPython('1 + "1"'), 2)`);
+        return runner?.runTest(`({
+	test: () => assert.equal(runPython('1 + "1"'), 2)
+})`);
       });
       expect(result).toEqual({
         err: {
@@ -1368,7 +1402,9 @@ pattern = re.compile('l+')
           type: "python",
           source,
         });
-        return runner?.runTest(`assert.equal(runPython('str(pattern)'), "l+")`);
+        return runner?.runTest(`({
+	test: () => assert.equal(runPython('str(pattern)'), "l+")
+})`);
       }, source);
       expect(result).toEqual({
         err: {
@@ -1395,9 +1431,9 @@ pattern = re.compile('l+')`;
         });
         // Since the comparison includes a PyProxy object, that will be
         // posted back to the caller and cause a DataCloneError
-        return runner?.runTest(
-          `assert.equal(__userGlobals.get("pattern"), "l+")`,
-        );
+        return runner?.runTest(`
+  ({ test: () => assert.equal(__userGlobals.get("pattern"), "l+") })
+`);
       }, source);
 
       expect(result).toEqual({
@@ -1415,39 +1451,14 @@ pattern = re.compile('l+')`;
       });
     });
 
-    it("should support runPython in simple tests", async () => {
-      const result = await page.evaluate(async () => {
-        const runner = await window.FCCTestRunner.createTestRunner({
-          type: "python",
-        });
-        return runner?.runTest(`assert.equal(runPython('1 + 1'), 2)`);
-      });
-      expect(result).toEqual({ pass: true });
-    });
-
-    it("should evaluate user code before running tests", async () => {
-      const result = await page.evaluate(async () => {
-        const runner = await window.FCCTestRunner.createTestRunner({
-          type: "python",
-          source: "x = 3",
-        });
-        return runner?.runTest(`assert.equal(runPython('x + 1'), 5)`);
-      });
-
-      expect(result).toMatchObject({
-        err: {
-          actual: 4,
-          expected: 5,
-        },
-      });
-    });
-
-    it("should not throw when input is called during a test", async () => {
+    it("should not throw io exceptions when input is called in a test", async () => {
       const result = await page.evaluate(async () => {
         const runner = await window.FCCTestRunner.createTestRunner({
           type: "python",
         });
-        return runner?.runTest(`assert.equal(runPython('input("test")'), "")`);
+        return runner?.runTest(
+          `({ test: () => assert.equal(runPython('input("test")'), "") })`,
+        );
       });
 
       expect(result).toEqual({ pass: true });
@@ -1463,7 +1474,7 @@ pattern = re.compile('l+')`;
           },
         });
         return runner?.runTest(
-          `assert.equal(runPython('input()'), "test input")`,
+          `({ test: () => assert.equal(runPython('input()'), "test input") })`,
         );
       }, beforeEach);
 
@@ -1483,7 +1494,7 @@ pattern = re.compile('l+')`;
             },
           });
           return runner?.runTest(
-            `assert.equal(runPython('name'), "mocked input")`,
+            `({ test: () => assert.equal(runPython('name'), "mocked input") })`,
           );
         },
         source,
@@ -1492,50 +1503,5 @@ pattern = re.compile('l+')`;
 
       expect(result).toEqual({ pass: true });
     });
-
-    it("should not automatically fail tests if the source raises an error", async () => {
-      const source = `
-def func():
-pass
-      `;
-      const result = await page.evaluate(async (source) => {
-        const runner = await window.FCCTestRunner.createTestRunner({
-          type: "python",
-          source,
-        });
-        return runner?.runTest(`assert.equal(1, 2)`);
-      }, source);
-
-      expect(result).toMatchObject({
-        err: {
-          expected: 2,
-          actual: 1,
-        },
-      });
-    });
-
-    it("should be possible to re-run user code inside a test to detect errors", async () => {
-      const source = `
-def func():
-pass
-      `;
-
-      const result = await page.evaluate(async (source) => {
-        const runner = await window.FCCTestRunner.createTestRunner({
-          type: "python",
-          source,
-          code: {
-            contents: source,
-          },
-        });
-        return runner?.runTest(`runPython(code)`);
-      }, source);
-
-      expect(result).toMatchObject({
-        err: {
-          type: "IndentationError",
-        },
-      });
-    });
   });
 });

packages/tests/integration-tests/timeout.test.ts

@@ -86,7 +86,9 @@ def run():
           source,
         });
         const result = await runner?.runTest(
-          `assert.equal(runPython('loop()'), 1)`,
+          `({
+						test: () => assert.equal(runPython('loop()'), 1)
+				})`,
           10,
         );
         return result;
@@ -100,7 +102,11 @@ def run():
           },
           source,
         });
-        return runner?.runTest(`assert.equal(runPython('run()'), 1)`);
+        return runner?.runTest(
+          `({
+						test: () => assert.equal(runPython('run()'), 1)
+				})`,
+        );
       }, source);
 
       expect(timeoutResult).toEqual({
@@ -126,7 +132,12 @@ while True:
           },
           source,
         });
-        return runner?.runTest(`assert.equal(runPython('1'), 1)`, 10);
+        return runner?.runTest(
+          `({
+						test: () => assert.equal(runPython('1'), 1)
+				})`,
+          10,
+        );
       }, source);
 
       expect(result).toEqual({