apollographql / apollo-kotlin

:rocket:  A strongly-typed, caching GraphQL client for the JVM, Android, and Kotlin multiplatform.
https://www.apollographql.com/docs/kotlin
MIT License
3.76k stars 653 forks source link

Automate updating IR test files for new codegen version #225

Closed BenSchwab closed 7 years ago

BenSchwab commented 7 years ago

As apollo-codegen changes, we will need to update our IR test suite. We should automate this. We can use the form: -test/graphql/com/example/test_name --- TestQuery.json --- TestQuery.graphl --- (schema.json) --- TestQueryExpected.java

The codegen will run on a general schema file, unless a custom one is present in the test's folder.

This will only need to run occasionally, so I am leaning on requiring apollo-codegen to be to installed and having gradle call a command line.

digitalbuddha commented 7 years ago

The requiring code gen to be installed part... Will that work with Travis?

BenSchwab commented 7 years ago

I am envisioning this being run very infrequently. So the TestQuery.json files would be checked-in.

We could automate this to occur on every build, and follow a similar approach to ApolloIRGenTask.

This has the upside that every time we update the codegen version, we won't forget to regenerate the test files. Downside is that it is another point of potential windows npm pain.

marwanad commented 7 years ago

We currently do so with the gradle plugin integration tests (https://github.com/apollographql/apollo-android/tree/master/apollo-gradle-plugin/src/test/testProject/android/basic/src/main/graphql/com/example). That is having the schema files and .graphql files and then applying the plugin and running gradle build.

The tests under apollo-compiler don't have any logic associate with them apart from checking that the generated files compile correctly iirc.

Correct me if I'm wrong but I think if bumping the apollo-codegen introduces breaking changes, they will be caught by the integration tests because either apollo-compiler will fail on a malformed IR or javac will fail.

BenSchwab commented 7 years ago

@marwanad The apollo-compiler implicitly depends on the results of apollo-codegen. Currently, we are checking in the IR files that we use to generate code, and then assert that it matches an expected generated file.

Here's why I think this would be helpful -- I need to bump the apollo-codegen version so we have access to field arguments to use for caching. Ultimately, I will edit the compiler to add the field arguments in the generated code. If I were to update the TestQueryExpected.java, the tests will fail because the IR files are "frozen" from an old version with no arguments. So I would have to go by hand calling apollo-codgen and replacing the IR file.

In terms of bumping-codgen introducing breaking changes, I would argue that the tests in gradle plugin integration tests are not sufficient. There are lots of parts of the GraphQL spec that aren't exercised in those tests, but which are contained in the compiler tests. (type names, directives, complex arguments etc.)

marwanad commented 7 years ago

@BenSchwab Ahh, that makes sense now that you mention that example. Thanks!