Skip to content
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

Fix context in directive tests #9025

Merged
merged 1 commit into from
Aug 9, 2023
Merged

Conversation

Josh-Walker-GM
Copy link
Collaborator

Problem
Context isolation was not behaving as expect when using directive tests. See https://community.redwoodjs.com/t/redwood-v6-0-0-upgrade-guide/5044/46 for more details.

Changes

  1. We set the global context to match the context passed in to the mockRedwoodDirective and then the mock directive gets the global context during execution.

Notes
It looks like in v5 we didn't pass through the context that was provided as a parameter to mockRedwoodDirective. It can be somewhat tricky to understand exactly what is happening due to a few layers and that in <v6 the context was not isolated between tests.

Outstanding

  1. People may not fully expect the side effect that passing in the context to the mock directive sets the global context?

@Josh-Walker-GM Josh-Walker-GM added the release:fix This PR is a fix label Aug 8, 2023
@Josh-Walker-GM Josh-Walker-GM added this to the next-release-patch milestone Aug 8, 2023
Copy link
Contributor

@dthyresson dthyresson left a comment

Choose a reason for hiding this comment

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

Josh and I paired on this fix yesterday. Approved!

@dthyresson dthyresson merged commit 2ce3189 into main Aug 9, 2023
27 of 30 checks passed
@dthyresson dthyresson deleted the jgmw-graphql/directive-context branch August 9, 2023 17:18
thedavidprice pushed a commit that referenced this pull request Aug 9, 2023
**Problem**
Context isolation was not behaving as expect when using directive tests.
See
https://community.redwoodjs.com/t/redwood-v6-0-0-upgrade-guide/5044/46
for more details.

**Changes**
1. We set the global context to match the context passed in to the
`mockRedwoodDirective` and then the mock directive gets the global
context during execution.

**Notes**
It looks like in v5 we didn't pass through the context that was provided
as a parameter to `mockRedwoodDirective`. It can be somewhat tricky to
understand exactly what is happening due to a few layers and that in <v6
the context was not isolated between tests.

**Outstanding**
1. People may not fully expect the side effect that passing in the
context to the mock directive sets the global context?
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release:fix This PR is a fix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants