Add EDGEDB_TEST_REPEATS env var for testing function cache (#7497)

When EDGEDB_TEST_REPEATS is set, the test suite duplicates transactional query tests, and the re-run version will typically use the function cache. There are some actual failures when I ran this that need to be investigated. Also resolve the issue returning tuple that only happens with binary output format, executing query like: select ((insert A), true); The fix is simply applying the returns_record fix to tuples: 070d74c#diff-d2fc37bc724ef58ac44d52da73c773483ba90dd9818f3acede2664abd700e850R1925-R1951 --------- Co-authored-by: Fantix King <[email protected]>
geldata · Jul 16, 2024 · e7f76a0 · e7f76a0
1 parent c809efc
commit e7f76a0
Show file tree

Hide file tree

Showing 7 changed files with 120 additions and 31 deletions.
diff --git a/.github/workflows.src/tests.tpl.yml b/.github/workflows.src/tests.tpl.yml
@@ -92,6 +92,7 @@ jobs:
     - name: Test
       env:
         SHARD: ${{ matrix.shard }}
+        EDGEDB_TEST_REPEATS: 1
       run: |
         mkdir -p .results/
         cp .tmp/time_stats.csv .results/running_times_${SHARD}.csv
@@ -118,6 +119,7 @@ jobs:
     - name: Generate complete list of tests for verification
       env:
         SHARD: ${{ matrix.shard }}
+        EDGEDB_TEST_REPEATS: 1
       run: |
         edb test --list > .tmp/all_tests.txt
 

diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml
diff --git a/edb/server/compiler/compiler.py b/edb/server/compiler/compiler.py
@@ -1996,7 +1996,15 @@ def _build_cache_function(
     return_type: tuple[str, ...] = ("unknown",)
     match ctx.output_format:
         case enums.OutputFormat.NONE:
-            return_type = ("void",)
+            # CONFIGURE commands are never cached; other queries are actually
+            # wrapped with a count() call in top_output_as_value(), so set the
+            # return_type to reflect that fact. This was set to `void`, leading
+            # to issues that certain exceptions are not raised as expected when
+            # wrapped with a function returning (setof) void - reproducible
+            # with test_edgeql_casts_json_12() and EDGEDB_TEST_REPEATS=1.
+            return_type = ("int",)
+            if ir.stype.is_object_type() or ir.stype.is_tuple(ir.schema):
+                returns_record = True
 
         case enums.OutputFormat.BINARY:
             if ir.stype.is_object_type():
@@ -2006,6 +2014,8 @@ def _build_cache_function(
                 return_type = pg_types.pg_type_from_ir_typeref(
                     ir.expr.typeref.base_type or ir.expr.typeref
                 )
+                if ir.stype.is_tuple(ir.schema):
+                    returns_record = True
 
         case enums.OutputFormat.JSON:
             return_type = ("json",)

diff --git a/edb/testbase/server.py b/edb/testbase/server.py
@@ -176,11 +176,15 @@ def _iter_methods(bases, ns):
             yield methname, meth
 
     @classmethod
-    def wrap(mcls, meth):
+    def wrap(mcls, meth, is_repeat=False):
         @functools.wraps(meth)
         def wrapper(self, *args, __meth__=meth, **kwargs):
             try_no = 1
 
+            if is_repeat and not getattr(self, 'TRANSACTION_ISOLATION', False):
+                raise unittest.SkipTest()
+
+            self.is_repeat = is_repeat
             while True:
                 try:
                     # There might be unobvious serializability
@@ -216,6 +220,16 @@ def wrapper(self, *args, __meth__=meth, **kwargs):
     def add_method(mcls, methname, ns, meth):
         ns[methname] = mcls.wrap(meth)
 
+        # If EDGEDB_TEST_REPEATS is set, duplicate all the tests.
+        # This is valuable because it should exercise the function
+        # cache.
+        if (
+            os.environ.get('EDGEDB_TEST_REPEATS', None)
+            and methname.startswith('test_')
+        ):
+            new = methname.replace('test_', 'test_zREPEAT_', 1)
+            ns[new] = mcls.wrap(meth, is_repeat=True)
+
     def __new__(mcls, name, bases, ns):
         for methname, meth in mcls._iter_methods(bases, ns.copy()):
             if methname in ns:
@@ -236,6 +250,7 @@ def __new__(mcls, name, bases, ns):
 
 
 class TestCase(unittest.TestCase, metaclass=TestCaseMeta):
+    is_repeat: bool = False
 
     @classmethod
     def setUpClass(cls):

diff --git a/tests/test_edgeql_datatypes.py b/tests/test_edgeql_datatypes.py
@@ -990,19 +990,41 @@ async def test_edgeql_dt_sequence_01(self):
             '''
         )
 
-        await self.assert_query_result(
-            r'''SELECT Obj { seq_prop } ORDER BY Obj.seq_prop;''',
-            [
-                {'seq_prop': 1}, {'seq_prop': 2}
-            ],
-        )
-
-        await self.assert_query_result(
-            r'''SELECT Obj2 { seq_prop };''',
-            [
-                {'seq_prop': 1}
-            ],
-        )
+        try:
+            await self.assert_query_result(
+                r'''SELECT Obj { seq_prop } ORDER BY Obj.seq_prop;''',
+                [
+                    {'seq_prop': 1}, {'seq_prop': 2}
+                ],
+            )
+        except AssertionError:
+            if self.is_repeat:
+                await self.assert_query_result(
+                    r'''SELECT Obj { seq_prop } ORDER BY Obj.seq_prop;''',
+                    [
+                        {'seq_prop': 3}, {'seq_prop': 4}
+                    ],
+                )
+            else:
+                raise
+
+        try:
+            await self.assert_query_result(
+                r'''SELECT Obj2 { seq_prop };''',
+                [
+                    {'seq_prop': 1},
+                ],
+            )
+        except AssertionError:
+            if self.is_repeat:
+                await self.assert_query_result(
+                    r'''SELECT Obj2 { seq_prop };''',
+                    [
+                        {'seq_prop': 2},
+                    ],
+                )
+            else:
+                raise
 
     async def test_edgeql_dt_enum_01(self):
         await self.assert_query_result(

diff --git a/tests/test_edgeql_insert.py b/tests/test_edgeql_insert.py
@@ -1947,12 +1947,23 @@ async def test_edgeql_insert_default_05(self):
             INSERT DefaultTest8;
         ''')
 
-        await self.assert_query_result(
-            r'''
-                SELECT DefaultTest8.number;
-            ''',
-            {1, 2, 3}
-        )
+        try:
+            await self.assert_query_result(
+                r'''
+                    SELECT DefaultTest8.number;
+                ''',
+                {1, 2, 3},
+            )
+        except AssertionError:
+            if self.is_repeat:
+                await self.assert_query_result(
+                    r'''
+                        SELECT DefaultTest8.number;
+                    ''',
+                    {4, 5, 6},
+                )
+            else:
+                raise
 
     async def test_edgeql_insert_default_06(self):
         res = await self.con.query(r'''

diff --git a/tests/test_protocol.py b/tests/test_protocol.py
@@ -222,7 +222,13 @@ async def test_proto_args_mismatch(self):
 
     async def test_proto_state(self):
         await self.con.connect()
+        try:
+            await self._test_proto_state()
+        finally:
+            await self.con.execute('DROP GLOBAL state_desc_1')
+            await self.con.execute('DROP GLOBAL state_desc_2')
 
+    async def _test_proto_state(self):
         # Create initial state schema
         await self._execute('CREATE GLOBAL state_desc_1 -> int32')
         sdd1 = await self.con.recv_match(protocol.StateDataDescription)
@@ -359,6 +365,7 @@ async def test_proto_state_desc_in_script(self):
             protocol.ReadyForCommand,
             transaction_state=protocol.TransactionState.NOT_IN_TRANSACTION,
         )
+        await self.con.execute('DROP GLOBAL state_desc_in_script')
 
     async def test_proto_desc_id_cardinality(self):
         await self.con.connect()
@@ -372,6 +379,13 @@ async def test_proto_desc_id_cardinality(self):
         )
         await self.con.recv_match(protocol.ReadyForCommand)
 
+        try:
+            await self._test_proto_desc_id_cardinality()
+        finally:
+            await self.con.execute('DROP TYPE CardTest')
+
+    async def _test_proto_desc_id_cardinality(self):
+
         await self._execute('SELECT CardTest { prop }', data=True)
         cdd1 = await self.con.recv_match(protocol.CommandDataDescription)
         await self.con.recv_match(
@@ -442,17 +456,20 @@ async def test_proto_parse_cardinality(self):
         )
         await self.con.recv_match(protocol.ReadyForCommand)
 
-        await self._parse("SELECT ParseCardTest")
-        await self.con.recv_match(
-            protocol.CommandDataDescription,
-            result_cardinality=compiler.Cardinality.MANY,
-        )
+        try:
+            await self._parse("SELECT ParseCardTest")
+            await self.con.recv_match(
+                protocol.CommandDataDescription,
+                result_cardinality=compiler.Cardinality.MANY,
+            )
 
-        await self._parse("SELECT ParseCardTest LIMIT 1")
-        await self.con.recv_match(
-            protocol.CommandDataDescription,
-            result_cardinality=compiler.Cardinality.AT_MOST_ONE,
-        )
+            await self._parse("SELECT ParseCardTest LIMIT 1")
+            await self.con.recv_match(
+                protocol.CommandDataDescription,
+                result_cardinality=compiler.Cardinality.AT_MOST_ONE,
+            )
+        finally:
+            await self.con.execute("DROP TYPE ParseCardTest")
 
     async def test_proto_state_concurrent_alter(self):
         con2 = await protocol.protocol.new_connection(
@@ -509,6 +526,7 @@ async def test_proto_state_concurrent_alter(self):
 
         finally:
             await con2.aclose()
+            await self.con.execute("DROP GLOBAL state_desc_3")
 
     async def _parse_execute(self, query, args):
         output_format = protocol.OutputFormat.BINARY
@@ -693,6 +711,15 @@ async def test_proto_state_change_in_tx(self):
         )
         await self.con.recv_match(protocol.ReadyForCommand)
 
+        try:
+            await self._test_proto_state_change_in_tx()
+        finally:
+            await self.con.execute('ROLLBACK')
+            await self.con.execute(
+                'DROP TYPE TestStateChangeInTx::StateChangeInTx')
+            await self.con.execute('DROP MODULE TestStateChangeInTx')
+
+    async def _test_proto_state_change_in_tx(self):
         # Collect states
         await self._execute('''
             SET MODULE TestStateChangeInTx