-
Notifications
You must be signed in to change notification settings - Fork 734
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: Updating test mock set method definitions #3089
Conversation
-Updated the definitions of the test mock _set method definitions -Updated MockObjectTemplate to generate mocks with new definitions
✅ Deploy Preview for apollo-ios-docs canceled.
|
} | ||
} | ||
""") : TemplateString(stringLiteral: "") | ||
) | ||
|
||
""" | ||
} | ||
|
||
private func getMockSetType(_ graphQLType: GraphQLType) -> String { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure I like the name of this function, open to other suggestions here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe just remove the get
- mockSetType
or something completely different in mockFunctionDescriptor
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is the right approach. Didn't look like we were going to get it right through typing alone.
} | ||
} | ||
""") : TemplateString(stringLiteral: "") | ||
) | ||
|
||
""" | ||
} | ||
|
||
private func getMockSetType(_ graphQLType: GraphQLType) -> String { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe just remove the get
- mockSetType
or something completely different in mockFunctionDescriptor
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, just one question on the changes to AnimalKingdomAPI
.
Sources/AnimalKingdomAPI/AnimalKingdomAPI/TestMocks/Bird+Mock.graphql.swift
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Two minor suggestions, but this looks great! Thanks so much @BobaFetters
case .entity(_): | ||
return "Entity" | ||
case .inputObject(_): | ||
preconditionFailure() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: might want to also add a message to this failure to help us identify the issue if a user ever encounters this and files a bug report.
-Updated the definitions of the test mock _set method definitions
-Updated MockObjectTemplate to generate mocks with new definitions
Closes #3023
Closes #3043