From 00085b8a5c2e936b6bdb45384154e92069ad66c1 Mon Sep 17 00:00:00 2001
From: Thomas Sibley <tsibley@fredhutch.org>
Date: Fri, 14 Jun 2024 15:45:17 -0700
Subject: [PATCH] pathogen-repo-ci: Use status functions in conditionals
 instead of continue-on-error: true

While "continue-on-error: true" resulted in the behaviour we wanted, it
also meant the job step status indicators were misleading: even when an
ingest/phylogenetic/nextclade step failed, the UI showed a check mark
for them in the logs.  Instead of masking failures to keep going in the
face of errors, override the default success() conditional by specifying
our own conditions on a status expression.

Related-to: <https://github.com/nextstrain/.github/pull/98#issuecomment-2168846812>
---
 .github/workflows/pathogen-repo-ci.yaml    | 17 +++++++++--------
 .github/workflows/pathogen-repo-ci.yaml.in | 17 +++++++++--------
 2 files changed, 18 insertions(+), 16 deletions(-)

diff --git a/.github/workflows/pathogen-repo-ci.yaml b/.github/workflows/pathogen-repo-ci.yaml
index 90282f7..9924078 100644
--- a/.github/workflows/pathogen-repo-ci.yaml
+++ b/.github/workflows/pathogen-repo-ci.yaml
@@ -286,18 +286,16 @@ jobs:
         if: hashFiles('ingest/Snakefile') && hashFiles('ingest/build-configs/ci/config.yaml')
         id: ingest
         run: nextstrain build ingest --configfile build-configs/ci/config.yaml
-        continue-on-error: true
       - name: Run phylogenetic
-        if: hashFiles('phylogenetic/Snakefile') && hashFiles('phylogenetic/build-configs/ci/config.yaml')
+        if: hashFiles('phylogenetic/Snakefile') && hashFiles('phylogenetic/build-configs/ci/config.yaml') && !cancelled()
         id: phylogenetic
         run: nextstrain build phylogenetic --configfile build-configs/ci/config.yaml
-        continue-on-error: true
       - name: Run nextclade
-        if: hashFiles('nextclade/Snakefile') && hashFiles('nextclade/build-configs/ci/config.yaml')
+        if: hashFiles('nextclade/Snakefile') && hashFiles('nextclade/build-configs/ci/config.yaml') && !cancelled()
         id: nextclade
         run: nextstrain build nextclade --configfile build-configs/ci/config.yaml
-        continue-on-error: true
-      - uses: actions/upload-artifact@v4
+      - if: always()
+        uses: actions/upload-artifact@v4
         with:
           name: ${{ inputs.artifact-name }}-${{ matrix.runtime }}
           if-no-files-found: ignore
@@ -320,10 +318,13 @@ jobs:
             nextclade/benchmarks/
             nextclade/logs/
             nextclade/results/
-      - name: Verify a workflow ran
+      - if: always()
+        name: Verify a workflow ran
         env:
           # "outcome" is success/failure/cancelled/skipped _before_
-          # "continue-on-error" is applied to calculate "conclusion"
+          # "continue-on-error" is applied to calculate "conclusion"; we no
+          # longer use continue-on-error for these steps, but even so,
+          # conceptually here what we want is outcome not conclusion.
           ingest: ${{ steps.ingest.outcome }}
           phylogenetic: ${{ steps.phylogenetic.outcome }}
           nextclade: ${{ steps.nextclade.outcome }}
diff --git a/.github/workflows/pathogen-repo-ci.yaml.in b/.github/workflows/pathogen-repo-ci.yaml.in
index 99ac3cc..904034f 100644
--- a/.github/workflows/pathogen-repo-ci.yaml.in
+++ b/.github/workflows/pathogen-repo-ci.yaml.in
@@ -263,21 +263,19 @@ jobs:
         if: hashFiles('ingest/Snakefile') && hashFiles('ingest/build-configs/ci/config.yaml')
         id: ingest
         run: nextstrain build ingest --configfile build-configs/ci/config.yaml
-        continue-on-error: true
 
       - name: Run phylogenetic
-        if: hashFiles('phylogenetic/Snakefile') && hashFiles('phylogenetic/build-configs/ci/config.yaml')
+        if: hashFiles('phylogenetic/Snakefile') && hashFiles('phylogenetic/build-configs/ci/config.yaml') && !cancelled()
         id: phylogenetic
         run: nextstrain build phylogenetic --configfile build-configs/ci/config.yaml
-        continue-on-error: true
 
       - name: Run nextclade
-        if: hashFiles('nextclade/Snakefile') && hashFiles('nextclade/build-configs/ci/config.yaml')
+        if: hashFiles('nextclade/Snakefile') && hashFiles('nextclade/build-configs/ci/config.yaml') && !cancelled()
         id: nextclade
         run: nextstrain build nextclade --configfile build-configs/ci/config.yaml
-        continue-on-error: true
 
-      - uses: actions/upload-artifact@v4
+      - if: always()
+        uses: actions/upload-artifact@v4
         with:
           name: ${{ inputs.artifact-name }}-${{ matrix.runtime }}
           if-no-files-found: ignore
@@ -301,10 +299,13 @@ jobs:
             nextclade/logs/
             nextclade/results/
 
-      - name: Verify a workflow ran
+      - if: always()
+        name: Verify a workflow ran
         env:
           # "outcome" is success/failure/cancelled/skipped _before_
-          # "continue-on-error" is applied to calculate "conclusion"
+          # "continue-on-error" is applied to calculate "conclusion"; we no
+          # longer use continue-on-error for these steps, but even so,
+          # conceptually here what we want is outcome not conclusion.
           ingest: ${{ steps.ingest.outcome }}
           phylogenetic: ${{ steps.phylogenetic.outcome }}
           nextclade: ${{ steps.nextclade.outcome }}