Fix: Unit test CTE failures not being captured #5081

VaggelisD · 2025-07-31T13:27:42Z

In unittest internals TestCase::addSubTest appends the result of that subtest directly to self.errors or self.failures:

    def addSubTest(self, test, subtest, err):
        """Called at the end of a subtest.
        'err' is None if the subtest ended successfully, otherwise it's a
        tuple of values as returned by sys.exc_info().
        """
        # By default, we don't do anything with successful subtests, but
        # more sophisticated test results might want to record them.
        if err is not None:
            if getattr(self, 'failfast', False):
                self.stop()
            if issubclass(err[0], test.failureException):
                errors = self.failures
            else:
                errors = self.errors
            errors.append((subtest, self._exc_info_to_string(err, test)))
            self._mirrorOutput = True

However, our custom classes expected that this'd happen through addError(...) or addFailure(...) so that we can intercept these exceptions (since they get turned into strings before being stored in their parent classes).

To solve this regression, ModelTestTextResult::addSubTest now follows the parent implementation and instead calls the respective APIs which do fill our own original_failures & original_error structures.

The output for that looks like:

❯ sqlmesh test
F
======================================================================

----------------------------------------------------------------------
FAIL: test_example_full_model (/Users/vaggelisd/Desktop/tobiko/test_dir/tests/test.yaml)
----------------------------------------------------------------------
         Data mismatch (CTE "filtered_orders_cte")          
┏━━━━━━━━━━┳━━━━━━━━━━━━━━━━━━━━━━━━━┳━━━━━━━━━━━━━━━━━━━━━┓
┃   Row    ┃      id: Expected       ┃     id: Actual      ┃
┡━━━━━━━━━━╇━━━━━━━━━━━━━━━━━━━━━━━━━╇━━━━━━━━━━━━━━━━━━━━━┩
│    0     │            5            │          1          │
└──────────┴─────────────────────────┴─────────────────────┘

----------------------------------------------------------------------
Test Failure Summary
======================================================================
Ran 1 tests against duckdb in 0.09 seconds. 

Failed tests (1):
 •<path>::test_example_full_model
======================================================================

Docs

unittest.TestResult | unittest.TestCase | unittest.runner

VaggelisD · 2025-07-31T13:28:47Z

sqlmesh/core/test/definition.py

+            failed_subtest = ""
+
+            if subtest := getattr(self, "_subtest", None):
+                if cte := subtest.params.get("cte"):
+                    failed_subtest = f" (CTE {cte})"


This is to differentiate the CTE mismatch from the output one, e.g if both subtests fail in a single test that'd be the output:

FF ====================================================================== ---------------------------------------------------------------------- FAIL: test_example_full_model (/Users/vaggelisd/Desktop/tobiko/test_dir/tests/test.yaml) ---------------------------------------------------------------------- Data mismatch (CTE "filtered_orders_cte") ┏━━━━━━━━━━┳━━━━━━━━━━━━━━━━━━━━━━━━━┳━━━━━━━━━━━━━━━━━━━━━┓ ┃ Row ┃ id: Expected ┃ id: Actual ┃ ┡━━━━━━━━━━╇━━━━━━━━━━━━━━━━━━━━━━━━━╇━━━━━━━━━━━━━━━━━━━━━┩ │ 0 │ 5 │ 1 │ └──────────┴─────────────────────────┴─────────────────────┘ ---------------------------------------------------------------------- FAIL: test_example_full_model (/Users/vaggelisd/Desktop/tobiko/test_dir/tests/test.yaml) ---------------------------------------------------------------------- Data mismatch ┏━━━━━━━━━━┳━━━━━━━━━━━━━━━━━━━━━━━━━┳━━━━━━━━━━━━━━━━━━━━━┓ ┃ Row ┃ id: Expected ┃ id: Actual ┃ ┡━━━━━━━━━━╇━━━━━━━━━━━━━━━━━━━━━━━━━╇━━━━━━━━━━━━━━━━━━━━━┩ │ 0 │ 2 │ 1 │ └──────────┴─────────────────────────┴─────────────────────┘ ---------------------------------------------------------------------- Test Failure Summary ====================================================================== Ran 1 tests against duckdb in 0.14 seconds. Failed tests (1): • /Users/vaggelisd/Desktop/tobiko/test_dir/tests/test.yaml::test_example_full_model ======================================================================

VaggelisD · 2025-07-31T13:29:12Z

sqlmesh/core/test/result.py

+    def get_fail_and_error_tests(self) -> t.List[ModelTest]:
+        # If tests contain failed subtests (e.g testing CTE outputs) we don't want
+        # to report it as different test failures
+        test_name_to_test = {
+            test.test_name: test
+            for test, _ in self.failures + self.errors
+            if isinstance(test, ModelTest)
+        }
+        return list(test_name_to_test.values())


Simple abstraction to simplify console.py

VaggelisD · 2025-07-31T13:29:56Z

tests/core/test_test.py

+        copy_test_file(original_test_file, tmp_path / "tests" / f"test_success_{i}.yaml", i)
+        copy_test_file(new_test_file, tmp_path / "tests" / f"test_failure_{i}.yaml", i)


Previously we copied these files directly but unit tests don't work well if the name is duplicated, I'll follow up with a UX fix

VaggelisD · 2025-07-31T13:30:29Z

sqlmesh/core/console.py

-        errors = result.errors
-        failures = result.failures
-        skipped = result.skipped
-
-        infos = []
-        if failures:
-            infos.append(f"failures={len(failures)}")
-        if errors:
-            infos.append(f"errors={len(errors)}")
-        if skipped:
-            infos.append(f"skipped={skipped}")


dead code, infos is no longer used

izeigerman · 2025-07-31T16:18:29Z

sqlmesh/core/test/definition.py

+
+            failed_subtest = ""
+
+            if subtest := getattr(self, "_subtest", None):


I wonder if it even makes sense at this point to rely on subtests rather than write our own context manager which sets the current CTE and then resets it.

Are there any other benefits that subtests offer us here?

I had the same thought as I was working on this, I think it's merit is that it triggers the callback addSubTest when it finishes, otherwise we'd have to schedule that ourselves.

Not really a strong argument for it, but I'm not sure if there's a strong argument against it either. Happy to reconsider if you had a better thought with the custom CM

VaggelisD commented Jul 31, 2025

View reviewed changes

VaggelisD added 2 commits July 31, 2025 16:45

Fix: Unit test CTE failures not being captured

ba16b22

Fix test

681d328

VaggelisD force-pushed the vaggelisd/fix_unit_test_cte branch from a726d55 to 681d328 Compare July 31, 2025 13:51

izeigerman reviewed Jul 31, 2025

View reviewed changes

izeigerman approved these changes Jul 31, 2025

View reviewed changes

VaggelisD merged commit a2f8a05 into main Aug 1, 2025
27 checks passed

VaggelisD deleted the vaggelisd/fix_unit_test_cte branch August 1, 2025 12:02

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fix: Unit test CTE failures not being captured #5081

Fix: Unit test CTE failures not being captured #5081

Uh oh!

VaggelisD commented Jul 31, 2025

Uh oh!

VaggelisD Jul 31, 2025

Uh oh!

VaggelisD Jul 31, 2025

Uh oh!

VaggelisD Jul 31, 2025

Uh oh!

VaggelisD Jul 31, 2025

Uh oh!

izeigerman Jul 31, 2025

Uh oh!

izeigerman Jul 31, 2025

Uh oh!

VaggelisD Jul 31, 2025

Uh oh!

Uh oh!

Uh oh!

		copy_test_file(original_test_file, tmp_path / "tests" / f"test_success_{i}.yaml", i)
		copy_test_file(new_test_file, tmp_path / "tests" / f"test_failure_{i}.yaml", i)


		failed_subtest = ""

		if subtest := getattr(self, "_subtest", None):

Fix: Unit test CTE failures not being captured #5081

Fix: Unit test CTE failures not being captured #5081

Uh oh!

Conversation

VaggelisD commented Jul 31, 2025

Docs

Uh oh!

VaggelisD Jul 31, 2025

Choose a reason for hiding this comment

Uh oh!

VaggelisD Jul 31, 2025

Choose a reason for hiding this comment

Uh oh!

VaggelisD Jul 31, 2025

Choose a reason for hiding this comment

Uh oh!

VaggelisD Jul 31, 2025

Choose a reason for hiding this comment

Uh oh!

izeigerman Jul 31, 2025

Choose a reason for hiding this comment

Uh oh!

izeigerman Jul 31, 2025

Choose a reason for hiding this comment

Uh oh!

VaggelisD Jul 31, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!