Skip to content

Conversation

@sbarker2
Copy link
Contributor

adding defer test for the remaining transforms:

  • NadelServiceTypeFilterTransform
  • NadelRenameArgumentInputTypesTransform
  • also added a test where it throws an error with transforms

sbarker2 and others added 25 commits August 19, 2024 12:11
# Conflicts:
#	test/src/test/kotlin/graphql/nadel/tests/next/fixtures/defer/transforms/DeferredDeepRenameTestSnapshot.kt
#	test/src/test/kotlin/graphql/nadel/tests/next/fixtures/defer/transforms/RenamedTypeIsDeferredTestSnapshot.kt
Copy link
Contributor

@felipe-gdr felipe-gdr left a comment

Choose a reason for hiding this comment

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

The tests don't look right to me, can you please have a look at my comments?

result = """
| {
| "data": {
| "aErrors": {}
Copy link
Contributor

Choose a reason for hiding this comment

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

There's not incremental data in the assertion here as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In this test, there is not supposed to be incremental data, because there is no data in the aErrors field.

This test is to ensure that the BError type does not get returned in the aErrors field

Copy link
Contributor

@felipe-gdr felipe-gdr left a comment

Choose a reason for hiding this comment

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

I'm still missing the deferred behaviour in some tests.
If that's part of the scenario you're testing, could add a description to the test class explaining why nothing is being deferred?

| query (${'$'}v0: PathType!) {
| me {
| profilePicture {
| path(type: ${'$'}v0)
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is @path not being deferred?

service = "A",
query = """
| {
| aErrors {
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm confused by this one... What deferred behaviour are we testing?

@sbarker2 sbarker2 changed the title Add tests for remaining transforms with defer [Ignore] Add tests for remaining transforms with defer Nov 19, 2024
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.

3 participants