Skip to content

fix: Use normalisation context when none is provided in tests #6157

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 16, 2024

Conversation

kevin-macquer-omnilog
Copy link
Contributor

@kevin-macquer-omnilog kevin-macquer-omnilog commented Feb 12, 2024

Q A
Branch? main
Tickets Closes #6146
License MIT
Doc PR

In #6098 the behavious was changed in the test trait.
Before when you didn't provide a serialization context, the SerializationFactory would initialize it to the normalization context of the operation.
With this change the normalization context is an empty array instead so we loose the normalization context.

Solution: use the normalization context of the operation if the serialization context is null.

@devzenfr
Copy link

I tested this change and that resolves the issues I had inside #6146. Thanks @kevin-macquer-omnilog, I hope this gets merged.

@kevin-macquer-omnilog
Copy link
Contributor Author

The test that is failing here https://github.com/api-platform/core/actions/runs/7872836937/job/21481776163?pr=6157 is also failing on the main branch in my machine, it doesn't seems linked to those changes.

@soyuka
Copy link
Member

soyuka commented Feb 12, 2024

we should target 3.2 also is it possible to add a non-reg test in the ApiTestCaseTest?

@kevin-macquer-omnilog
Copy link
Contributor Author

we should target 3.2 also is it possible to add a non-reg test in the ApiTestCaseTest?

Hi soyuka, I'm working on the test today, will post an update when it's done.

@kevin-macquer-omnilog kevin-macquer-omnilog force-pushed the fix/testTraits branch 2 times, most recently from 2b62452 to 2707e03 Compare February 14, 2024 13:31
@kevin-macquer-omnilog
Copy link
Contributor Author

@soyuka I've added the required tests.
The test doesn't work on the main branch but work on this branch.

For targeting 3.2 it needs to be merged on the main branch right?

@kevin-macquer-omnilog kevin-macquer-omnilog changed the base branch from main to 3.2 February 15, 2024 08:41
@kevin-macquer-omnilog kevin-macquer-omnilog changed the base branch from 3.2 to main February 15, 2024 08:41
@kevin-macquer-omnilog kevin-macquer-omnilog changed the base branch from main to 3.2 February 15, 2024 08:48
@kevin-macquer-omnilog
Copy link
Contributor Author

@soyuka I've changed the target of this PR to 3.2.

@soyuka
Copy link
Member

soyuka commented Feb 16, 2024

amazing !

@soyuka soyuka merged commit 2629539 into api-platform:3.2 Feb 16, 2024
@kevin-macquer-omnilog kevin-macquer-omnilog deleted the fix/testTraits branch February 16, 2024 08:33
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.

Unit tests using assertMatchesJsonSchema fail for ManyToOne properties
3 participants