Skip to content

Commit

Permalink
Updated and simplified PHP testing structure (#8558)
Browse files Browse the repository at this point in the history
* Simplified PHP testing setup.

- Consolidated on a single autoloader, created by composer.
- Consolidated on a single phpunit invocation strategy: we run
  phpunit on a directory, which will run all tests matching *Test.php
  in that directory.
- We now rely on autoloading to import all test protos. require_once()
  calls for test protos are removed.
- For now the valgrind tests are removed. A follow-up PR will re-enable
  them in a more robust way.

* More improvements to PHP testing.

1. Replace custom PHPUnit-selection logic in test.sh with generic
   composer version selection.
2. Optimized both test proto generation and the custom extension
   build to avoid unnecessary work when the files are already up
   to date.

* Added assertions to verify that the C test doesn't use PHP sources.

* Updated tests.sh for the new PHP testing commands.

* Removed obsolete rules from tests.sh.

* Fixed generate_test_protos.sh for when tmp does not exist.

Also removed undefined_test.php and fixed Makefile.am.

* Added php8.0_all again which is still used.

* Added missing file to Makefile.am.

* Re-added php_all_32 rule which is also still used.

* Updated testing commands for macOS and download composer.

* Use /usr/local/bin on mac instead of /usr/bin, since the latter is not writable.
  • Loading branch information
haberman committed May 4, 2021
1 parent 114efc4 commit 8b87075
Show file tree
Hide file tree
Showing 17 changed files with 105 additions and 1,313 deletions.
8 changes: 2 additions & 6 deletions Makefile.am
Expand Up @@ -824,7 +824,7 @@ php_EXTRA_DIST= \
php/ext/google/protobuf/protobuf.h \
php/ext/google/protobuf/wkt.inc \
php/generate_descriptor_protos.sh \
php/phpunit.xml \
php/generate_test_protos.sh \
php/prepare_c_extension.sh \
php/release.sh \
php/src/GPBMetadata/Google/Protobuf/Any.php \
Expand Down Expand Up @@ -953,14 +953,12 @@ php_EXTRA_DIST= \
php/src/Google/Protobuf/Value.php \
php/src/phpdoc.dist.xml \
php/tests/ArrayTest.php \
php/tests/autoload.php \
php/tests/bootstrap_phpunit.php \
php/tests/compatibility_test.sh \
php/tests/compile_extension.sh \
php/tests/DescriptorsTest.php \
php/tests/EncodeDecodeTest.php \
php/tests/force_c_ext.php \
php/tests/gdb_test.sh \
php/tests/generate_protos.sh \
php/tests/GeneratedClassTest.php \
php/tests/GeneratedPhpdocTest.php \
php/tests/GeneratedServiceTest.php \
Expand All @@ -987,10 +985,8 @@ php_EXTRA_DIST= \
php/tests/proto/test_service.proto \
php/tests/proto/test_service_namespace.proto \
php/tests/proto/test_wrapper_type_setters.proto \
php/tests/test.sh \
php/tests/test_base.php \
php/tests/test_util.php \
php/tests/undefined_test.php \
php/tests/valgrind.supp \
php/tests/WellKnownTest.php \
php/tests/WrapperTypeSettersTest.php
Expand Down
9 changes: 5 additions & 4 deletions php/composer.json
Expand Up @@ -6,7 +6,7 @@
"homepage": "https://developers.google.com/protocol-buffers/",
"license": "BSD-3-Clause",
"require": {
"php": ">=5.5.0"
"php": ">=7.0.0"
},
"require-dev": {
"phpunit/phpunit": ">=5.0.0"
Expand All @@ -19,11 +19,12 @@
},
"autoload-dev": {
"psr-4": {
"": "tests/generated"
"": "tmp"
}
},
"scripts": {
"test": "tests/generate_protos.sh && vendor/bin/phpunit",
"aggregate_metadata_test": "tests/generate_protos.sh --aggregate_metadata && vendor/bin/phpunit"
"test_c": "./generate_test_protos.sh && ./tests/compile_extension.sh && php -dextension=ext/google/protobuf/modules/protobuf.so vendor/bin/phpunit --bootstrap tests/force_c_ext.php tests",
"test": "./generate_test_protos.sh && vendor/bin/phpunit tests",
"aggregate_metadata_test": "./generate_test_protos.sh --aggregate_metadata && vendor/bin/phpunit tests"
}
}
27 changes: 27 additions & 0 deletions php/generate_test_protos.sh
@@ -0,0 +1,27 @@
#!/bin/bash

set -e

cd `dirname $0`

if [[ -d tmp && -z $(find tests/proto ../src/protoc -newer tmp) ]]; then
# Generated protos are already present and up to date, so we can skip protoc.
#
# Protoc is very fast, but sometimes it is not available (like if we haven't
# built it in Docker). Skipping it helps us proceed in this case.
echo "Test protos are up-to-date, skipping protoc."
exit 0
fi

rm -rf tmp
mkdir -p tmp

find tests/proto -type f -name "*.proto"| xargs ../src/protoc --php_out=tmp -I../src -Itests

if [ "$1" = "--aggregate_metadata" ]; then
# Overwrite some of the files to use aggregation.
AGGREGATED_FILES="tests/proto/test.proto tests/proto/test_include.proto tests/proto/test_import_descriptor_proto.proto"
../src/protoc --php_out=aggregate_metadata=foo#bar:tmp -I../src -Itests $AGGREGATED_FILES
fi

echo "Generated test protos from tests/proto -> tmp"
18 changes: 0 additions & 18 deletions php/phpunit.xml

This file was deleted.

2 changes: 0 additions & 2 deletions php/tests/DescriptorsTest.php
@@ -1,7 +1,5 @@
<?php

require_once('generated/Descriptors/TestDescriptorsEnum.php');
require_once('generated/Descriptors/TestDescriptorsMessage.php');
require_once('test_base.php');
require_once('test_util.php');

Expand Down
10 changes: 4 additions & 6 deletions php/tests/GeneratedClassTest.php
@@ -1,7 +1,5 @@
<?php

require_once('generated/NoNamespaceEnum.php');
require_once('generated/NoNamespaceMessage.php');
require_once('test_base.php');
require_once('test_util.php');

Expand Down Expand Up @@ -759,10 +757,10 @@ public function testMessageMergeFrom()
public function testMessageWithoutNamespace()
{
$m = new TestMessage();
$n = new NoNameSpaceMessage();
$n = new NoNamespaceMessage();
$m->setOptionalNoNamespaceMessage($n);
$repeatedNoNamespaceMessage = $m->getRepeatedNoNamespaceMessage();
$repeatedNoNamespaceMessage[] = new NoNameSpaceMessage();
$repeatedNoNamespaceMessage[] = new NoNamespaceMessage();
$m->setRepeatedNoNamespaceMessage($repeatedNoNamespaceMessage);

// test nested messages
Expand All @@ -775,9 +773,9 @@ public function testMessageWithoutNamespace()
public function testEnumWithoutNamespace()
{
$m = new TestMessage();
$m->setOptionalNoNamespaceEnum(NoNameSpaceEnum::VALUE_A);
$m->setOptionalNoNamespaceEnum(NoNamespaceEnum::VALUE_A);
$repeatedNoNamespaceEnum = $m->getRepeatedNoNamespaceEnum();
$repeatedNoNamespaceEnum[] = NoNameSpaceEnum::VALUE_A;
$repeatedNoNamespaceEnum[] = NoNamespaceEnum::VALUE_A;
$m->setRepeatedNoNamespaceEnum($repeatedNoNamespaceEnum);
$this->assertTrue(true);
}
Expand Down
2 changes: 0 additions & 2 deletions php/tests/GeneratedPhpdocTest.php
@@ -1,7 +1,5 @@
<?php

require_once('generated/NoNamespaceEnum.php');
require_once('generated/NoNamespaceMessage.php');
require_once('test_base.php');
require_once('test_util.php');

Expand Down
2 changes: 1 addition & 1 deletion php/tests/PhpImplementationTest.php
Expand Up @@ -18,7 +18,7 @@
* Please note, this test is only intended to be run without the protobuf C
* extension.
*/
class ImplementationTest extends TestBase
class PhpImplementationTest extends TestBase
{
/**
* Avoid calling setUp, which has void return type (not avalialbe in php7.0).
Expand Down
27 changes: 0 additions & 27 deletions php/tests/autoload.php

This file was deleted.

5 changes: 0 additions & 5 deletions php/tests/bootstrap_phpunit.php

This file was deleted.

30 changes: 18 additions & 12 deletions php/tests/compile_extension.sh
@@ -1,20 +1,26 @@
#!/bin/bash

set -ex
set -e

cd $(dirname $0)

../prepare_c_extension.sh
pushd ../ext/google/protobuf
phpize --clean
rm -f configure.in configure.ac
phpize
if [ "$1" = "--release" ]; then
./configure --with-php-config=$(which php-config)
else
# To get debugging symbols in PHP itself, build PHP with:
# $ ./configure --enable-debug CFLAGS='-g -O0'
./configure --with-php-config=$(which php-config) CFLAGS="-g -O0 -Wall"
pushd ../ext/google/protobuf > /dev/null

CONFIGURE_OPTIONS=("./configure" "--with-php-config=$(which php-config)")

if [ "$1" != "--release" ]; then
CONFIGURE_OPTIONS+=("CFLAGS=-g -O0 -Wall")
fi

# If the PHP interpreter we are building against or the arguments
# have changed, we must regenerated the Makefile.
if [[ ! -f Makefile ]] || [[ "$(grep ' \$ ./configure' config.log)" != " $ ${CONFIGURE_OPTIONS[@]}" ]]; then
phpize --clean
rm -f configure.in configure.ac
phpize
"${CONFIGURE_OPTIONS[@]}"
fi

make
popd
popd > /dev/null
14 changes: 14 additions & 0 deletions php/tests/force_c_ext.php
@@ -0,0 +1,14 @@
<?php

// We have to test this because the command-line argument will fail silently
// if the extension could not be loaded:
// php -dextension=ext/google/protobuf/modules/protouf.so
if (!extension_loaded("protobuf")) {
throw new Exception("Protobuf extension not loaded");
}

spl_autoload_register(function($class) {
if (strpos($class, "Google\\Protobuf") === 0) {
throw new Exception("When using the C extension, we should not load runtime class: " . $class);
}
}, true, true);
16 changes: 0 additions & 16 deletions php/tests/generate_protos.sh

This file was deleted.

52 changes: 1 addition & 51 deletions php/tests/memory_leak_test.php
Expand Up @@ -2,57 +2,7 @@

# phpunit has memory leak by itself. Thus, it cannot be used to test memory leak.

require_once('generated/NoNamespaceEnum.php');
require_once('generated/NoNamespaceMessage.php');
require_once('generated/NoNamespaceMessage/NestedEnum.php');
require_once('generated/NoNamespaceMessage/NestedMessage.php');
require_once('generated/PrefixEmpty.php');
require_once('generated/PrefixTestPrefix.php');
require_once('generated/PrefixTestPrefix/PrefixNestedEnum.php');
require_once('generated/PrefixTestPrefix/PrefixNestedMessage.php');
require_once('generated/TestEmptyNamespace.php');
require_once('generated/TestEmptyNamespace/NestedEnum.php');
require_once('generated/TestEmptyNamespace/NestedMessage.php');
require_once('generated/Bar/TestInclude.php');
require_once('generated/Bar/TestLegacyMessage.php');
require_once('generated/Bar/TestLegacyMessage/NestedEnum.php');
require_once('generated/Bar/TestLegacyMessage/NestedMessage.php');
require_once('generated/Foo/PBARRAY.php');
require_once('generated/Foo/PBEmpty.php');
require_once('generated/Foo/TestAny.php');
require_once('generated/Foo/TestBoolValue.php');
require_once('generated/Foo/TestBytesValue.php');
require_once('generated/Foo/TestEnum.php');
require_once('generated/Foo/TestIncludeNamespaceMessage.php');
require_once('generated/Foo/TestIncludePrefixMessage.php');
require_once('generated/Foo/TestInt32Value.php');
require_once('generated/Foo/TestInt64Value.php');
require_once('generated/Foo/TestMessage.php');
require_once('generated/Foo/TestMessage/PBEmpty.php');
require_once('generated/Foo/TestMessage/NestedEnum.php');
require_once('generated/Foo/TestMessage/Sub.php');
require_once('generated/Foo/TestPackedMessage.php');
require_once('generated/Foo/TestPhpDoc.php');
require_once('generated/Foo/TestRandomFieldOrder.php');
require_once('generated/Foo/TestReverseFieldOrder.php');
require_once('generated/Foo/TestStringValue.php');
require_once('generated/Foo/TestUInt32Value.php');
require_once('generated/Foo/TestUInt64Value.php');
require_once('generated/Foo/TestUnpackedMessage.php');
require_once('generated/Foo/testLowerCaseMessage.php');
require_once('generated/Foo/testLowerCaseEnum.php');
require_once('generated/GPBMetadata/Proto/Test.php');
require_once('generated/TestEmptyPhpNamespace.php');
require_once('generated/GPBMetadata/Proto/TestInclude.php');
require_once('generated/TestNoNamespace.php');
require_once('generated/Metadata/Php/Test/TestPhpNamespace.php');
require_once('generated/GPBMetadata/Proto/TestPrefix.php');
require_once('generated/Php/Test/TestNamespace.php');
require_once('generated/Php/Test/TestNamespace/PBEmpty.php');
require_once('generated/Php/Test/TestNamespace/PBEmpty/NestedEnum.php');
require_once('generated/Php/Test/TestNamespace/PBEmpty/NestedMessage.php');
require_once('generated/Php/Test/TestNamespace/NestedEnum.php');
require_once('generated/Php/Test/TestNamespace/NestedMessage.php');
require_once('../vendor/autoload.php');
require_once('test_util.php');

use Google\Protobuf\Internal\RepeatedField;
Expand Down

0 comments on commit 8b87075

Please sign in to comment.