From 8f6afdc0efbc026ff7dcb1863d5591bd68da1209 Mon Sep 17 00:00:00 2001 From: Mike Kruskal Date: Tue, 3 Dec 2024 11:35:35 -0800 Subject: [PATCH] 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 --- .github/workflows/test_csharp.yml | 2 +- .github/workflows/test_php.yml | 40 +++++++++++++------------------ conformance/conformance_php.php | 11 +++++++++ php/BUILD.bazel | 34 +++++++++++++++++++------- 4 files changed, 53 insertions(+), 34 deletions(-) diff --git a/.github/workflows/test_csharp.yml b/.github/workflows/test_csharp.yml index a573288d2d4c..b41a18a54777 100644 --- 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 index e06cf58f2b29..7c05eb5c4e2b 100644 --- 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 index 683eae537a61..66af4731e44d 100644 --- 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 index 1845e1397496..f1a0728d1883 100644 --- 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", ],