Removing deprecated methods from tests - part 1 #2930
Conversation
| ] | ||
| when: | ||
| def result = GraphQL.newGraphQL(StarWarsSchema.starWarsSchema).build().execute(query, null, params).data | ||
| def result = GraphQL.newGraphQL(StarWarsSchema.starWarsSchema).build().execute(query, null, params).data // Retain deprecated method for test coverage |
There was a problem hiding this comment.
Sorry to be a pain but I think we should "retain the deprecated method coverage IN the test for that class
ex GraphQLTest say rather than some random other test
This was the scope of the "retained for deprecated reasons" will reside in the most logical spot
There was a problem hiding this comment.
That's a good idea, makes it easier to find deprecated uses later
| Object get(DataFetchingEnvironment environment) { | ||
| Person source = environment.getSource() | ||
| DataLoaderRegistry dlRegistry = environment.getContext() | ||
| DataLoaderRegistry dlRegistry = environment.getGraphQlContext() |
There was a problem hiding this comment.
How can this work??? a "context" could have been a DataLoaderRegistry but a GraphqlContext never is!??
There was a problem hiding this comment.
nvironment.getGraphQlContext().get("registry") ??
There was a problem hiding this comment.
Nope I was wrong, it should not work
| private void assertValidQuery(GraphQLSchema graphQLSchema, String query, Map variables = [:]) { | ||
| GraphQL graphQL = GraphQL.newGraphQL(graphQLSchema).build() | ||
| assert graphQL.execute(query, null, variables).errors.size() == 0 | ||
| assert graphQL.execute(query, null, variables).errors.size() == 0 // Retain deprecated method for test coverage |
There was a problem hiding this comment.
Move to its own home test - remove from here - make a new test in its home test
bbakerman
left a comment
bbakerman
left a comment
There was a problem hiding this comment.
See my comments about moving ghe deprecated methods to their "home test" as a geenral pattern
| * @deprecated use {@link #graphQLContext(GraphQLContext)} instead | ||
| */ | ||
| @Deprecated | ||
| @DeprecatedAt("2021-07-05") |
There was a problem hiding this comment.
The context getter was deprecated on this date, for consistency, also deprecating the builder
| ] | ||
| ] | ||
| when: | ||
| def result = GraphQL.newGraphQL(StarWarsSchema.starWarsSchema).build().execute(query, null, params).data // Retain deprecated method for test coverage |
There was a problem hiding this comment.
#2932 will remove execute, I'll remove execute tests after that PR is merged
| * @deprecated use {@link #graphQLContext(GraphQLContext)} instead | ||
| */ | ||
| @Deprecated | ||
| @DeprecatedAt("2021-07-05") |
There was a problem hiding this comment.
Nice because this is when it was deprecated - albeit missed
| .query(query) | ||
| .root(new MutationSchema.SubscriptionRoot(6)) | ||
|
|
||
| def executionResult = GraphQL.newGraphQL(MutationSchema.schema).build().execute(executionInput) |
There was a problem hiding this comment.
THere will be some merge conflicts here
| def executionInput = ExecutionInput.newExecutionInput() | ||
| .query(query) | ||
| .root(GarfieldSchema.john) | ||
| def executionResult = GraphQL.newGraphQL(GarfieldSchema.GarfieldSchema).build().execute(executionInput) |
There was a problem hiding this comment.
merge conflicts here as well I think
# Conflicts: # src/test/groovy/graphql/MutationTest.groovy # src/test/groovy/graphql/StarWarsQueryTest.groovy # src/test/groovy/graphql/UnionTest.groovy # src/test/groovy/graphql/normalized/ExecutableNormalizedOperationFactoryTest.groovy
dondonz
added
not release related
Updating tests to move away from deprecated methods. I've left at least 1 usage to maintain coverage in the method's "home" test file.
Also deprecates the
ExecutionContextBuildercontextbuilder for consistency. The getter was deprecated long ago.This is part 1, more to come in another PR.