Skip to content

Conversation

@Steveb-p
Copy link
Collaborator

@Steveb-p Steveb-p commented Sep 8, 2025

🎫 Issue IBX-XXXXX

Description:

This PR adds ErrorMessage (internal://ibexa/rest/ErrorMessage) as schema usable for tests.

It also changes how Response validation is done, allowing expected HTTP code and expected schema to be customized.

For QA:

Documentation:

@Steveb-p Steveb-p force-pushed the error-message+schema-location branch from a35d1f6 to 32c9a42 Compare September 8, 2025 10:37
@Steveb-p Steveb-p changed the base branch from main to 4.6 September 8, 2025 10:39
@Steveb-p Steveb-p requested review from alongosz and Copilot September 8, 2025 10:40
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds ErrorMessage schema support for REST API testing and refactors response validation to allow customization of expected HTTP codes and schemas. The changes improve test flexibility by enabling custom schema locations and removing redundant type assertions.

Key changes:

  • Added ErrorSchemaProvider for ibexa/rest/ErrorMessage schema validation
  • Refactored response validation to accept EndpointRequestDefinition instead of separate parameters
  • Added schemaLocation property to EndpointRequestDefinition for custom schema paths

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
src/lib/Schema/ErrorSchemaProvider.php Implements schema provider for ErrorMessage with required properties
src/contracts/Request/Value/EndpointRequestDefinition.php Adds schemaLocation property and getter/setter methods
src/contracts/BaseRestWebTestCase.php Refactors validation methods to use EndpointRequestDefinition parameter
src/bundle/Resources/config/services/rest_schema.yaml Registers ErrorSchemaProvider as tagged service
phpstan-baseline.neon Removes baseline entry for redundant string type assertion

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

{
public function provideSchemas(): iterable
{
yield 'ibexa/rest/ErrorMessage' => (object)[
Copy link

Copilot AI Sep 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider using an array instead of casting to object with (object). The schema structure is already defined as an associative array, and the object cast adds unnecessary complexity without clear benefit.

Suggested change
yield 'ibexa/rest/ErrorMessage' => (object)[
yield 'ibexa/rest/ErrorMessage' => [

Copilot uses AI. Check for mistakes.
$expectedStatusCode,
"Expected HTTP $expectedStatusCode, got HTTP $actualStatusCode status code"
);
self::assertResponseStatusCodeSame($expectedStatusCode);
Copy link

Copilot AI Sep 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The refactoring from manual status code assertion to assertResponseStatusCodeSame() is good, but the custom error message providing context about expected vs actual status codes has been lost. Consider preserving the descriptive error message for better test failure diagnostics.

Suggested change
self::assertResponseStatusCodeSame($expectedStatusCode);
self::assertResponseStatusCodeSame(
$expectedStatusCode,
sprintf(
'Expected status code %d but got %d for endpoint [%s %s]',
$expectedStatusCode,
$this->client?->getResponse()?->getStatusCode(),
$endpointDefinition->getMethod(),
$endpointDefinition->getUri()
)
);

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant