From 0e71c9e3bbd78fad878fa4d09126a7b305d06fe2 Mon Sep 17 00:00:00 2001
From: David Peter <sharkdp@users.noreply.github.com>
Date: Tue, 26 Nov 2024 15:40:02 +0100
Subject: [PATCH] [red-knot] Fix unit tests in release mode (#14604)

## Summary

This is about the easiest patch that I can think of. It has a drawback
in that there is no real guarantee this won't happen again. I think this
might be acceptable, given that all of this is a temporary thing.

And we also add a new CI job to prevent regressions like this in the
future.

For the record though, I'm listing alternative approaches I thought of:

- We could get rid of the debug/release distinction and just add `@Todo`
type metadata everywhere. This has possible affects on runtime. The main
reason I didn't follow through with this is that the size of `Type`
increases. We would either have to adapt the `assert_eq_size!` test or
get rid of it. Even if we add messages everywhere and get rid of the
file-and-line-variant in the enum, it's not enough to get back to the
current release-mode size of `Type`.
- We could generally discard `@Todo` meta information when using it in
tests. I think this would be a huge drawback. I like that we can have
the actual messages in the mdtest. And make sure we get the expected
`@Todo` type, not just any `@Todo`. It's also helpful when debugging
tests.

closes #14594

## Test Plan

```rs
cargo nextest run --release
```
---
 .github/workflows/ci.yaml                     | 28 ++++++++++++++++++-
 .../src/types/infer.rs                        | 12 ++++++--
 crates/red_knot_test/src/matcher.rs           | 13 +++++++++
 3 files changed, 50 insertions(+), 3 deletions(-)

diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml
index a552de86d3..d73670fc90 100644
--- a/.github/workflows/ci.yaml
+++ b/.github/workflows/ci.yaml
@@ -157,6 +157,33 @@ jobs:
           name: ruff
           path: target/debug/ruff
 
+  cargo-test-linux-release:
+    name: "cargo test (linux, release)"
+    runs-on: depot-ubuntu-22.04-16
+    needs: determine_changes
+    if: ${{ needs.determine_changes.outputs.code == 'true' || github.ref == 'refs/heads/main' }}
+    timeout-minutes: 20
+    steps:
+      - uses: actions/checkout@v4
+      - name: "Install Rust toolchain"
+        run: rustup show
+      - name: "Install mold"
+        uses: rui314/setup-mold@v1
+      - name: "Install cargo nextest"
+        uses: taiki-e/install-action@v2
+        with:
+          tool: cargo-nextest
+      - name: "Install cargo insta"
+        uses: taiki-e/install-action@v2
+        with:
+          tool: cargo-insta
+      - uses: Swatinem/rust-cache@v2
+      - name: "Run tests"
+        shell: bash
+        env:
+          NEXTEST_PROFILE: "ci"
+        run: cargo insta test --release --all-features --unreferenced reject --test-runner nextest
+
   cargo-test-windows:
     name: "cargo test (windows)"
     runs-on: windows-latest-xlarge
@@ -212,7 +239,6 @@ jobs:
   cargo-build-release:
     name: "cargo build (release)"
     runs-on: macos-latest
-    needs: determine_changes
     if: ${{ github.ref == 'refs/heads/main' }}
     timeout-minutes: 20
     steps:
diff --git a/crates/red_knot_python_semantic/src/types/infer.rs b/crates/red_knot_python_semantic/src/types/infer.rs
index dbf6c0bbef..32104b0f07 100644
--- a/crates/red_knot_python_semantic/src/types/infer.rs
+++ b/crates/red_knot_python_semantic/src/types/infer.rs
@@ -5985,7 +5985,11 @@ mod tests {
             "src/a.py",
             &["foo", "<listcomp>"],
             "x",
-            "@Todo(async iterables/iterators)",
+            if cfg!(debug_assertions) {
+                "@Todo(async iterables/iterators)"
+            } else {
+                "@Todo"
+            },
         );
 
         Ok(())
@@ -6015,7 +6019,11 @@ mod tests {
             "src/a.py",
             &["foo", "<listcomp>"],
             "x",
-            "@Todo(async iterables/iterators)",
+            if cfg!(debug_assertions) {
+                "@Todo(async iterables/iterators)"
+            } else {
+                "@Todo"
+            },
         );
 
         Ok(())
diff --git a/crates/red_knot_test/src/matcher.rs b/crates/red_knot_test/src/matcher.rs
index 6d8b0488c4..5c8b7328ed 100644
--- a/crates/red_knot_test/src/matcher.rs
+++ b/crates/red_knot_test/src/matcher.rs
@@ -180,6 +180,16 @@ where
     }
 }
 
+/// Discard `@Todo`-type metadata from expected types, which is not available
+/// when running in release mode.
+#[cfg(not(debug_assertions))]
+fn discard_todo_metadata(ty: &str) -> std::borrow::Cow<'_, str> {
+    static TODO_METADATA_REGEX: std::sync::LazyLock<regex::Regex> =
+        std::sync::LazyLock::new(|| regex::Regex::new(r"@Todo\([^)]*\)").unwrap());
+
+    TODO_METADATA_REGEX.replace_all(ty, "@Todo")
+}
+
 struct Matcher {
     line_index: LineIndex,
     source: SourceText,
@@ -276,6 +286,9 @@ impl Matcher {
                 }
             }
             Assertion::Revealed(expected_type) => {
+                #[cfg(not(debug_assertions))]
+                let expected_type = discard_todo_metadata(&expected_type);
+
                 let mut matched_revealed_type = None;
                 let mut matched_undefined_reveal = None;
                 let expected_reveal_type_message = format!("Revealed type is `{expected_type}`");