graphql-php icon indicating copy to clipboard operation
graphql-php copied to clipboard

Test on PHP 8.3

Open spawnia opened this issue 1 year ago • 4 comments

spawnia avatar Mar 20 '24 15:03 spawnia

@simPod 7 tests are failing for me locally. Do you have any idea why they work in CI but not locally? I have PHP 8.3.3 installed.

There were 7 failures:

1) GraphQL\Tests\Executor\AbstractTest::testWarnsAboutOrphanedTypesWhenMissingType
Failed asserting that null is an instance of class GraphQL\Error\Error.

/home/bfranke/projects/graphql-php/tests/Executor/AbstractTest.php:631

2) GraphQL\Tests\Executor\AbstractTest::testHintsOnConflictingTypeInstancesInResolveType
Failed asserting that null is an instance of class GraphQL\Error\Error.

/home/bfranke/projects/graphql-php/tests/Executor/AbstractTest.php:764

3) GraphQL\Tests\Executor\ExecutorLazySchemaTest::testHintsOnConflictingTypeInstancesInDefinitions
Failed asserting that two arrays are identical.
--- Expected
+++ Actual
@@ @@
 Array &0 [
     0 => 'Test',
-    1 => 'Test',
 ]

/home/bfranke/projects/graphql-php/tests/Executor/ExecutorLazySchemaTest.php:182

4) GraphQL\Tests\Executor\ExecutorLazySchemaTest::testSimpleQuery
Failed asserting that two arrays are identical.
--- Expected
+++ Actual
@@ @@
 Array &0 [
     0 => 'Query.fields',
-    1 => 'SomeObject',
-    2 => 'SomeObject.fields',
+    1 => 'SomeObject.fields',
 ]

/home/bfranke/projects/graphql-php/tests/Executor/ExecutorLazySchemaTest.php:218

5) GraphQL\Tests\Executor\ExecutorLazySchemaTest::testDeepQuery
Failed asserting that two arrays are identical.
--- Expected
+++ Actual
@@ @@
 Array &0 [
     0 => 'Query.fields',
-    1 => 'SomeObject',
-    2 => 'SomeObject.fields',
+    1 => 'SomeObject.fields',
 ]

/home/bfranke/projects/graphql-php/tests/Executor/ExecutorLazySchemaTest.php:385

6) GraphQL\Tests\Executor\ExecutorLazySchemaTest::testResolveUnion
Failed asserting that two arrays are identical.
--- Expected
+++ Actual
@@ @@
 Array &0 [
     0 => 'Query.fields',
-    1 => 'OtherObject',
-    2 => 'OtherObject.fields',
-    3 => 'SomeUnion',
-    4 => 'SomeUnion.resolveType',
-    5 => 'SomeUnion.types',
-    6 => 'DeeperObject',
-    7 => 'SomeScalar',
+    1 => 'OtherObject.fields',
+    2 => 'SomeUnion.resolveType',
+    3 => 'SomeUnion.types',
 ]

/home/bfranke/projects/graphql-php/tests/Executor/ExecutorLazySchemaTest.php:446

7) GraphQL\Tests\Type\DefinitionTest::testRejectsASchemaWithSameNamedObjectsImplementingAnInterface
Failed asserting that exception with message matching "/Schema must contain unique named types but contains multiple types named "BadObject"/" is thrown

spawnia avatar Mar 20 '24 15:03 spawnia

FTR, works for me but one other test fails
> phpunitser test
PHPUnit 10.5.13 by Sebastian Bergmann and contributors.

Runtime:       PHP 8.3.4
Configuration: /Users/neo/src/graphql-php/phpunit.xml.dist

........................................................W....   61 / 1790 (  3%)
.............................................................  122 / 1790 (  6%)
.............................................................  183 / 1790 ( 10%)
.............................................................  244 / 1790 ( 13%)
.............................................................  305 / 1790 ( 17%)
.............................................................  366 / 1790 ( 20%)
.............S...............................................  427 / 1790 ( 23%)
........S....................................................  488 / 1790 ( 27%)
.............................................................  549 / 1790 ( 30%)
.............................................................  610 / 1790 ( 34%)
............................................S................  671 / 1790 ( 37%)
...................F.........................................  732 / 1790 ( 40%)
......................................................S......  793 / 1790 ( 44%)
..................................................I..........  854 / 1790 ( 47%)
.............................................................  915 / 1790 ( 51%)
.........................................................I...  976 / 1790 ( 54%)
............................................I..III........... 1037 / 1790 ( 57%)
................S............................................ 1098 / 1790 ( 61%)
................................S.....S...................... 1159 / 1790 ( 64%)
............................................................. 1220 / 1790 ( 68%)
............................................................. 1281 / 1790 ( 71%)
........................................................S.... 1342 / 1790 ( 74%)
............................................................. 1403 / 1790 ( 78%)
............................................................. 1464 / 1790 ( 81%)
............................................................. 1525 / 1790 ( 85%)
............................................................. 1586 / 1790 ( 88%)
............................................................. 1647 / 1790 ( 92%)
............................................................. 1708 / 1790 ( 95%)
............................................................. 1769 / 1790 ( 98%)
.....................                                         1790 / 1790 (100%)

Time: 00:05.725, Memory: 50.00 MB

There was 1 failure:

1) GraphQL\Tests\Type\EnumTypeTest::testAllowsSimpleArrayAsValues
Failed asserting that an array has the subset Array &0 [
    'data' => Array &1 [
        'first' => 'ONE',
        'second' => 'TWO',
        'third' => null,
    ],
    'errors' => Array &2 [
        0 => Array &3 [
            'locations' => Array &4 [
                0 => Array &5 [
                    'line' => 4,
                    'column' => 13,
                ],
            ],
            'extensions' => Array &6 [
                'debugMessage' => 'Expected a value of type SimpleEnum but received: "WRONG". Cannot serialize value as enum: "WRONG"',
                'trace' => Array &7 [
                    0 => Array &8 [
                        'call' => 'GraphQL\Type\Definition\EnumType::serialize()',
                    ],
                ],
            ],
        ],
    ],
].
--- Expected
+++ Actual
@@ @@
           array (
             'file' => '/Users/neo/src/graphql-php/src/Executor/ReferenceExecutor.php',
             'line' => 1011,
-            'call' => 'GraphQL\\Type\\Definition\\EnumType::serialize()',
+            'call' => 'GraphQL\\Type\\Definition\\EnumType::serialize(\'WRONG\')',
           ),
           1 => 
           array (

/Users/neo/src/graphql-php/vendor/dms/phpunit-arraysubset-asserts/src/Constraint/ArraySubset.php:100
/Users/neo/src/graphql-php/vendor/dms/phpunit-arraysubset-asserts/src/ArraySubsetAsserts.php:83
/Users/neo/src/graphql-php/tests/Type/EnumTypeTest.php:581

FAILURES!
Tests: 1790, Assertions: 20924, Failures: 1, Warnings: 1, Skipped: 8, Incomplete: 7.
Script phpunit handling the test event returned with error code 1
⏱  2024-03-20 20:13:00 📁 ~/src/graphql-php ⎇ test-php8.3
+ $ 


mfn avatar Mar 20 '24 19:03 mfn

😄

For me only EnumTypeTest::testAllowsSimpleArrayAsValues fails locally but it has been failing like 2y already.

image

simPod avatar Mar 21 '24 09:03 simPod

Ah!

That's because the php.ini used by default by setup-php is production, but the ones you get locally (e.g. via brew) is development (or so, but definitely not production).

For this particular case, -dzend.exception_ignore_args=On and the test will not fail locally anymore.

See https://github.com/shivammathur/setup-php?tab=readme-ov-file#ini-file-optional

  • Specify the base php.ini file.
  • Accepts production, development or none.
  • By default, production php.ini file is used.

I bet if you change ini-file: development, then the github action will fail too.

mfn avatar Mar 21 '24 10:03 mfn

Thanks for digging in. Do you think that specifying the INI settings in composer test makes sense the way i did it?

spawnia avatar Mar 24 '24 11:03 spawnia

I can't remember seeing this, but it looks ~good~ smart and ensures compatibility no matter the environment => 👍🏼

mfn avatar Mar 24 '24 15:03 mfn