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

FIX #18 Remove the IntegrationTest scope from the GraphQLQueryPlugin #20

Merged
merged 6 commits into from
May 21, 2018

Conversation

muuki88
Copy link
Owner

@muuki88 muuki88 commented Mar 18, 2018

No description provided.

// TODO separate these into two auto plugins
override def projectSettings: Seq[Setting[_]] =
pluginSettings(Compile) ++ pluginSettings(IntegrationTest)
override def projectSettings: Seq[Setting[_]] = pluginSettings(Compile)

private def pluginSettings(config: Configuration): Seq[Setting[_]] =
inConfig(config)(
Copy link
Collaborator

Choose a reason for hiding this comment

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

The config can be removed from the following line below:

     graphqlQueryDirectory := (sourceDirectory in Compile).value / "graphql",

Copy link
Owner Author

Choose a reason for hiding this comment

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

Good catch. I'm thinking about putting things into the Test scope as these are actually tests.

I'm also less and less sure if the QueryPlugin is actually necessary. You can write simple unit
tests with scalatest and sangria pretty easy as well. WDYT?

Copy link
Collaborator

@jonas jonas Mar 20, 2018

Choose a reason for hiding this comment

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

The QueryPlugin helps to check that the generated schema and associated queries match. I guess that may be helpful in cases where you don’t us Scala end-to-end. In the project I used sangria in we had a scala.js client and was able to write unit tests that ran queries against the server-side and also tested the JSON serialization.

Copy link
Owner Author

Choose a reason for hiding this comment

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

Interesting point. Then let's keep it. I think the test scope is a better fit then, too.

@jonas
Copy link
Collaborator

jonas commented Mar 18, 2018

Refs #18

@@ -15,7 +15,6 @@ libraryDependencies ++= Seq(

// scripted test settings
scriptedLaunchOpts += "-Dproject.version=" + version.value
scriptedLaunchOpts += "-Dcodegen.samples.dir=" + ((baseDirectory in ThisBuild).value / "src/test/resources")
Copy link
Owner Author

Choose a reason for hiding this comment

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

I isolated the test cases as it was hard to refactor things due to the inter-dependencies.
Also the scripted tests acted as unit tests as well, which is too much IMHO.

@muuki88 muuki88 merged commit d20c890 into master May 21, 2018
@muuki88 muuki88 deleted the sbt-graphql-18 branch May 21, 2018 09:12
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.

2 participants