api-platform / api-platform/core

ResourceMetadataCompatibilityTest fails when api-platform/elasticsearch is autoloaded

Open
#8,495 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
PHP
Stars
2.6k
Forks
980
Avg merge
2d 5h
Merged PRs (30d)
48

Description

| Q | A
| -------------------- | ---
| API Platform version | 4.3, 4.4, main
| PHP version | 8.4 / 8.5

`ResourceMetadataCompatibilityTest` fails as soon as `ApiPlatform\Elasticsearch\State\Options` is autoloadable, which is the case for anyone running the component tests from the monorepo root:

```
$ vendor/bin/phpunit src/Metadata/Tests/Extractor/ResourceMetadataCompatibilityTest.php

There were 2 failures:
1) …::testValidMetadata#0 with data ('…XmlResourceExtractor', …XmlResourceAdapter)
Failed asserting that two objects are equal.
- 'stateOptions' => null
+ 'stateOptions' => ApiPlatform\Elasticsearch\State\Options Object (...)
```

Both data sets fail, and the rendered diff also shows unrelated keys such as `strictQueryParameterValidation`, which sends you looking in the wrong place — removing `src/Elasticsearch/State/Options.php` makes the whole test green again, so `stateOptions` is the only real difference.

### Cause

The fixture declares `stateOptions: {elasticsearchOptions: {index: foo_index}}`, and both extractors build the real object when the component is installed:

```php
// src/Metadata/Extractor/XmlResourceExtractor.php:477
if (isset($stateOptions->elasticsearchOptions) && class_exists(ElasticsearchOptions::class)) {
return new ElasticsearchOptions(...);
}
```

The expectation, on the other hand, returns `null` unconditionally:

```php
// src/Metadata/Tests/Extractor/ResourceMetadataCompatibilityTest.php:731
case 'elasticsearchOptions':
return null;
```

So the assertion only holds when `api-platform/elasticsearch` is *absent*. CI never notices, because `phpunit-components` runs each component from its own directory with its own `vendor/`, where the class does not exist. `$configuration` is even read and then unused, which suggests the `null` was a placeholder.

Beyond the failure, this means the `elasticsearchOptions` branch of `buildStateOptions()` has no assertion on it in any environment.

### Fix

Mirror the extractors: build the options when the class exists, keep returning `null` otherwise. The test then passes both from the monorepo root and in the isolated component job. PR follows.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.