Fix some silent failures in our CI.

Some of the more complex GHA were invoking inline bash scripts without setting `-e`, meaning that failures would go unnoticed. #test-continuous PiperOrigin-RevId: 702413176
protocolbuffers · Dec 3, 2024 · 8f6afdc · 8f6afdc
1 parent 7b8796a
commit 8f6afdc
Show file tree

Hide file tree

Showing 4 changed files with 53 additions and 34 deletions.
diff --git a/.github/workflows/test_csharp.yml b/.github/workflows/test_csharp.yml
@@ -31,7 +31,7 @@ jobs:
           credentials: ${{ secrets.GAR_SERVICE_ACCOUNT }}
           entrypoint: /bin/bash
           command: >-
-            -c "
+            -cex "
             cd csharp &&
             dotnet restore src/Google.Protobuf.sln &&
             dotnet build -c Release src/Google.Protobuf.sln &&

diff --git a/.github/workflows/test_php.yml b/.github/workflows/test_php.yml
@@ -32,17 +32,17 @@ jobs:
           - name: 8.1 Optimized
             version: "8.1.14"
             version-short: "8.1"
-            command: composer test \&\& composer test_c
+            command: composer test && composer test_c
           - name: 8.1 Debug
             version: 8.1.14-dbg
             version-short: "8.1"
-            command: composer test \&\& composer test_c
+            command: composer test && composer test_c
             continuous-only: true
           - name: 8.1 Memory Leak
             version: 8.1.14-dbg
             version-short: "8.1"
             # Run specialized memory leak & multirequest tests.
-            command: composer test_c \&\& tests/multirequest.sh \&\& tests/memory_leak_test.sh
+            command: composer test_c && tests/multirequest.sh && tests/memory_leak_test.sh
             continuous-only: true
           - name: 8.1 Valgrind
             version: 8.1.14-dbg
@@ -52,7 +52,7 @@ jobs:
           - name: 8.3 Optimized
             version: "8.3.1"
             version-short: "8.3"
-            command: composer test \&\& composer test_c
+            command: composer test && composer test_c
 
     name: ${{ matrix.continuous-only && inputs.continuous-prefix || '' }} Linux ${{ matrix.name}}
     runs-on: ubuntu-22-4core
@@ -70,21 +70,20 @@ jobs:
           directory: php
       - name: Run tests
         if: ${{ !matrix.continuous-only || inputs.continuous-run }}
-        uses: protocolbuffers/protobuf-ci/docker@v3
+        uses: protocolbuffers/protobuf-ci/bazel-docker@v3
         with:
-          image: us-docker.pkg.dev/protobuf-build/containers/test/linux/php:6.4.0-${{ matrix.version }}-27cf7b86212020d7e552bc13b1e084abb971da75
-          credentials: ${{ secrets.GAR_SERVICE_ACCOUNT }}
-          extra-flags: -e COMPOSER_HOME=/workspace/composer-cache
-          command: ${{ matrix.command }}
-
-      - name: Run conformance tests
-        if: ${{ !matrix.continuous-only || inputs.continuous-run }}
-        uses: protocolbuffers/protobuf-ci/bazel@v3
-        with:
-          image: us-docker.pkg.dev/protobuf-build/containers/test/linux/php:6.4.0-${{ matrix.version }}-27cf7b86212020d7e552bc13b1e084abb971da75
+          image: us-docker.pkg.dev/protobuf-build/containers/test/linux/php:6.4.0-${{ matrix.version }}-90d207f4e749b54c8792bbe974dfc70323b6566e
           credentials: ${{ secrets.GAR_SERVICE_ACCOUNT }}
           bazel-cache: php_linux/${{ matrix.version }}
-          bazel: test //php:conformance_test //php:conformance_test_c --action_env=PATH --test_env=PATH
+          bash: |
+            set -ex
+            COMPOSER_HOME=/workspace/composer-cache
+            BAZEL_FLAGS=${{ env.BAZEL_FLAGS }}
+            pushd /workspace/php
+            composer update
+            ${{ matrix.command }}
+            popd
+            bazel test //php:conformance_test //php:conformance_test_c --action_env=PATH --test_env=PATH ${{ env.BAZEL_FLAGS }}
 
   linux-32bit:
     strategy:
@@ -179,19 +178,12 @@ jobs:
           credentials: ${{ secrets.GAR_SERVICE_ACCOUNT }}
           extra-flags: -e COMPOSER_HOME=/workspace/composer-cache -e PROTOC=/workspace/${{ steps.cross-compile.outputs.protoc }}
           command: >-
-            -c '
+            -cex '
             cd php;
             composer update --ignore-platform-reqs;
             composer test;
             composer test_c'
 
-      - name: Run conformance tests
-        uses: protocolbuffers/protobuf-ci/bazel@v3
-        with:
-          credentials: ${{ secrets.GAR_SERVICE_ACCOUNT }}
-          bazel-cache: php_linux/${{ matrix.version }}
-          bazel: test //php:conformance_test //php:conformance_test_c --action_env=PATH --test_env=PATH
-
   macos:
     strategy:
       fail-fast: false   # Don't cancel all jobs if one fails.

diff --git a/conformance/conformance_php.php b/conformance/conformance_php.php
@@ -36,6 +36,8 @@
     ini_set('date.timezone', 'UTC');
 }
 
+error_reporting(0);
+
 $test_count = 0;
 
 function doTest($request)
@@ -78,6 +80,9 @@ function doTest($request)
             } catch (Exception $e) {
                 $response->setParseError($e->getMessage());
                 return $response;
+            } catch (Error $e) {
+                $response->setParseError($e->getMessage());
+                return $response;
             }
             break;
 
@@ -104,6 +109,9 @@ function doTest($request)
             } catch (Exception $e) {
                 $response->setParseError($e->getMessage());
                 return $response;
+            } catch (Error $e) {
+                $response->setParseError($e->getMessage());
+                return $response;
             }
             break;
 
@@ -129,6 +137,9 @@ function doTest($request)
             } catch (Exception $e) {
                 $response->setSerializeError($e->getMessage());
                 return $response;
+            } catch (Error $e) {
+                $response->setSerializeError($e->getMessage());
+                return $response;
             }
     }
 

diff --git a/php/BUILD.bazel b/php/BUILD.bazel
@@ -96,10 +96,10 @@ genrule(
     ],
     outs = ["protobuf.so"],
     cmd = """
-        rm php/ext/google/protobuf/wkt.inc php/ext/google/protobuf/php-upb.h php/ext/google/protobuf/php-upb.c
-        cp $(location generated/ext/google/protobuf/wkt.inc) php/ext/google/protobuf
-        cp $(location generated/ext/google/protobuf/php-upb.h) php/ext/google/protobuf
-        cp $(location generated/ext/google/protobuf/php-upb.c) php/ext/google/protobuf
+        mkdir -p php/ext/google/protobuf
+        cp -f $(location generated/ext/google/protobuf/wkt.inc) php/ext/google/protobuf
+        cp -f $(location generated/ext/google/protobuf/php-upb.h) php/ext/google/protobuf
+        cp -f $(location generated/ext/google/protobuf/php-upb.c) php/ext/google/protobuf
         ./$(execpath :build_extension) $@
         cp php/ext/google/protobuf/modules/protobuf.so $(OUTS)
     """,
@@ -230,13 +230,18 @@ pkg_files(
 
 pkg_files(
     name = "php_ext_source_files",
-    srcs = glob([
-        "ext/google/protobuf/*.h",
-        "ext/google/protobuf/*.c",
-    ]) + [
+    srcs = glob(
+        [
+            "ext/google/protobuf/*.h",
+            "ext/google/protobuf/*.c",
+        ],
+        exclude = [
+            "*/php-upb.*",
+            "*/wkt.inc",
+        ],
+    ) + [
         "ext/google/protobuf/config.m4",
         "ext/google/protobuf/config.w32",
-        "ext/google/protobuf/wkt.inc",
         "//:LICENSE",
     ],
 )
@@ -250,9 +255,20 @@ pkg_files(
     prefix = "third_party/utf8_range",
 )
 
+pkg_files(
+    name = "generated_files",
+    srcs = [
+        "ext/google/protobuf/php-upb.c",
+        "ext/google/protobuf/php-upb.h",
+        "ext/google/protobuf/wkt.inc",
+    ],
+    prefix = "third_party/utf8_range",
+)
+
 pkg_filegroup(
     name = "pecl_release_files",
     srcs = [
+        ":generated_files",
         ":php_ext_source_files",
         ":utf8_range_files",
     ],