From 05a28b97f29342a3c82538574476acf244a78ef8 Mon Sep 17 00:00:00 2001
From: teor <teor@riseup.net>
Date: Wed, 6 Dec 2023 10:52:47 +1000
Subject: [PATCH] change(ci): Split experimental feature tests into their own
 step (#8039)

* Split experimental feature tests into their own step

* Only log a checkpoint error warning once
---
 .github/workflows/ci-unit-tests-docker.yml | 19 +++++++++++++-----
 zebra-consensus/src/router.rs              | 23 +++++++++-------------
 2 files changed, 23 insertions(+), 19 deletions(-)

diff --git a/.github/workflows/ci-unit-tests-docker.yml b/.github/workflows/ci-unit-tests-docker.yml
index 46b67c79f..a0a1e7c11 100644
--- a/.github/workflows/ci-unit-tests-docker.yml
+++ b/.github/workflows/ci-unit-tests-docker.yml
@@ -124,17 +124,26 @@ jobs:
       # If some tests hang, add "-- --nocapture" for just that test, or for all the tests.
       #
       # TODO: move this test command into entrypoint.sh
-      #       add a separate experimental workflow job if this job is slow
       - name: Run zebrad tests
+        env:
+          NETWORK: ${{ inputs.network || vars.ZCASH_NETWORK }}
         run: |
           docker pull ${{ vars.GAR_BASE }}/${{ vars.CI_IMAGE_NAME }}@${{ needs.build.outputs.image_digest }}
           docker run -e NETWORK --name zebrad-tests --tty ${{ vars.GAR_BASE }}/${{ vars.CI_IMAGE_NAME }}@${{ needs.build.outputs.image_digest }} cargo test --locked --release --features "${{ env.TEST_FEATURES }}" --workspace -- --include-ignored
-          # Currently GitHub doesn't allow empty variables
-          if [[ -n "${{ vars.RUST_EXPERIMENTAL_FEATURES }}" && "${{ vars.RUST_EXPERIMENTAL_FEATURES }}" != " " ]]; then
-            docker run -e NETWORK --name zebrad-tests-experimental --tty ${{ vars.GAR_BASE }}/${{ vars.CI_IMAGE_NAME }}@${{ needs.build.outputs.image_digest }} cargo test --locked --release --features "${{ env.EXPERIMENTAL_FEATURES }} " --workspace -- --include-ignored
-          fi
+
+      # Run unit, basic acceptance tests, and ignored tests with experimental features.
+      #
+      # TODO: move this test command into entrypoint.sh
+      - name: Run zebrad tests with experimental features
         env:
           NETWORK: ${{ inputs.network || vars.ZCASH_NETWORK }}
+        run: |
+          # GitHub doesn't allow empty variables
+          if [[ -n "${{ vars.RUST_EXPERIMENTAL_FEATURES }}" && "${{ vars.RUST_EXPERIMENTAL_FEATURES }}" != " " ]]; then
+            docker run -e NETWORK --name zebrad-tests-experimental --tty ${{ vars.GAR_BASE }}/${{ vars.CI_IMAGE_NAME }}@${{ needs.build.outputs.image_digest }} cargo test --locked --release --features "${{ env.EXPERIMENTAL_FEATURES }} " --workspace -- --include-ignored
+          else
+            echo "Experimental builds are disabled, set RUST_EXPERIMENTAL_FEATURES in GitHub actions variables to enable them"
+          fi
 
   # Run state tests with fake activation heights.
   #
diff --git a/zebra-consensus/src/router.rs b/zebra-consensus/src/router.rs
index 179dc487d..5c10e545b 100644
--- a/zebra-consensus/src/router.rs
+++ b/zebra-consensus/src/router.rs
@@ -264,6 +264,7 @@ where
             //
             // <https://zips.z.cash/protocol/protocol.pdf#blockchain>
             let full_checkpoints = CheckpointList::new(network);
+            let mut already_warned = false;
 
             for (height, checkpoint_hash) in full_checkpoints.iter() {
                 let checkpoint_state_service = checkpoint_state_service.clone();
@@ -298,20 +299,14 @@ where
                         unreachable!("unexpected response type: {response:?} from state request")
                     }
                     Err(e) => {
-                        #[cfg(not(test))]
-                        tracing::warn!(
-                            "unexpected error: {e:?} in state request while verifying previous \
-                             state checkpoints. Is Zebra shutting down?"
-                        );
-                        // This error happens a lot in some tests.
-                        //
-                        // TODO: fix the tests so they don't cause this error,
-                        //       or change the tracing filter
-                        #[cfg(test)]
-                        tracing::debug!(
-                            "unexpected error: {e:?} in state request while verifying previous \
-                             state checkpoints. Is Zebra shutting down?"
-                        );
+                        // This error happens a lot in some tests, and it could happen to users.
+                        if !already_warned {
+                            tracing::warn!(
+                                "unexpected error: {e:?} in state request while verifying previous \
+                                 state checkpoints. Is Zebra shutting down?"
+                            );
+                            already_warned = true;
+                        }
                     }
                 }
             }