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

The Hub UT framework should send what it received #206

Merged
merged 1 commit into from
Oct 23, 2024

Conversation

marckhouzam
Copy link
Contributor

@marckhouzam marckhouzam commented Aug 13, 2024

What this PR does / why we need it

Before this PR the UT framework would send the registered operation. This was not useful for unit tests as it is those same tests that registered the operation, so they are aware of what it is. What the tests need is for the framework to send the actual received GraphQL request so that it can be verified.

This PR does this.
NOTE: This change is not backwards-compatible. However, I feel it is early enough in the use of the UT framework and that the only ones using it are probably the CLI team, that we can easily adapt.

The unit tests of the UT framework are updated to check that the sent value is the actual query received by the mock server.

Which issue(s) this PR fixes

Follow-up to #202

Describe testing done for PR

I have an internal set of unit tests that now detects more failures by comparing the received graphql request with what the tests expect.

Release note

Hub unit test framework now sends back the GraphQL request it received for more thorough testing.  This change breaks backwards-compatibility of the API used to write unit test for GraphQL: the `Responder` and `EventGenerator` types have a slightly different signature.

Additional information

Special notes for your reviewer

Before this commit the UT framework would send the registered operation.
This was not useful for unit tests as it is those same tests that
registered the operation, so they are aware of what it is.  What is the
tests need is for the framework to send the actual received GraphQL
request so that it can be verified. This commit does this.

Signed-off-by: Marc Khouzam <[email protected]>
@marckhouzam marckhouzam marked this pull request as ready for review October 19, 2024 12:38
@marckhouzam marckhouzam requested a review from a team as a code owner October 19, 2024 12:38
@marckhouzam marckhouzam changed the title The Hub UT framework must send what it received The Hub UT framework should send what it received Oct 20, 2024
Copy link
Contributor

@anujc25 anujc25 left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for the improvement. This is going to be really useful for writing dynamic unit tests based on request data.

client/hub/client_test.go Show resolved Hide resolved
@anujc25
Copy link
Contributor

anujc25 commented Oct 23, 2024

@marckhouzam Can you mention about the breaking change in this unit test framework as part of the ReleaseNotes as well?

@marckhouzam marckhouzam added this to the v1.5.0 milestone Oct 23, 2024
@marckhouzam marckhouzam merged commit b5f3499 into vmware-tanzu:main Oct 23, 2024
4 checks passed
@marckhouzam marckhouzam deleted the fix/graphqlTesting branch October 23, 2024 12:30
anujc25 pushed a commit to anujc25/tanzu-plugin-runtime that referenced this pull request Nov 5, 2024
Before this commit the UT framework would send the registered operation.
This was not useful for unit tests as it is those same tests that
registered the operation, so they are already aware of what it is.  What the
tests need is for the framework to send the actual received GraphQL
request so that it can be verified. This commit does this.

Signed-off-by: Marc Khouzam <[email protected]>
anujc25 pushed a commit to anujc25/tanzu-plugin-runtime that referenced this pull request Nov 5, 2024
Before this commit the UT framework would send the registered operation.
This was not useful for unit tests as it is those same tests that
registered the operation, so they are already aware of what it is.  What the
tests need is for the framework to send the actual received GraphQL
request so that it can be verified. This commit does this.

Signed-off-by: Marc Khouzam <[email protected]>
anujc25 added a commit that referenced this pull request Nov 5, 2024
Before this commit the UT framework would send the registered operation.
This was not useful for unit tests as it is those same tests that
registered the operation, so they are already aware of what it is.  What the
tests need is for the framework to send the actual received GraphQL
request so that it can be verified. This commit does this.

Signed-off-by: Marc Khouzam <[email protected]>
Co-authored-by: Marc Khouzam <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants