Remove CPP from test suite, take 2 #880
Conversation
| - env: ARGS="--resolver lts-10 --stack-yaml stack_lts-10.yaml" | ||
| - env: ARGS="--resolver lts-11" | ||
| - env: ARGS="--resolver lts-12" | ||
| - env: ARGS="--resolver lts-13" |
There was a problem hiding this comment.
Hooray for simpler CI!
There was a problem hiding this comment.
What's the reason for leaving in 6 and 9, but not the other ones between 6 and 11?
(EDIT: n/m I must have looked at a specific commit or something)
There was a problem hiding this comment.
They're all resolvers for distinct GHC versions. 7.10.3, 8.0.2, 8.2, 8.4, and 8.6 are represented here. More resolvers seemed redundant to me.
| , mtl | ||
| , containers | ||
| , blaze-html | ||
| , time |
There was a problem hiding this comment.
The deps are somewhat sloppy. They could be weedered now that we don't have so much CPP.
| {-# LANGUAGE QuasiQuotes #-} | ||
| {-# LANGUAGE TemplateHaskell #-} | ||
| {-# LANGUAGE TypeFamilies #-} | ||
| module EmbedTestMongo (specs) where |
There was a problem hiding this comment.
Some of these tests were just really hard to unentangle, and this was one of them. Instead of detangling it I just copied it.
| PersistentTest.cleanDB | ||
|
|
||
| hspec $ do | ||
| RenameTest.specsWith db |
There was a problem hiding this comment.
All of the test suites are now parameterized by a database runner function: type DbRunner backend m = ReaderT bakend m () -> IO (). This,combined with mpsGeneric = True, allows us to have most of these tests be generic over the backend.
| {-# LANGUAGE TemplateHaskell #-} | ||
| {-# LANGUAGE TypeFamilies #-} | ||
|
|
||
| module ArrayAggTest where |
There was a problem hiding this comment.
The tests that were enabled for a single backend were factored into the specific test suite.
| @@ -0,0 +1,153 @@ | |||
| {-# LANGUAGE QuasiQuotes, GADTs, OverloadedStrings #-} | |||
There was a problem hiding this comment.
This test suite is annoying. We can't have a circular dependency, so in order to depend on persistent-sqlite to have a driver to run these tests against, we have to not depend on persistent-test
| @@ -1,53 +0,0 @@ | |||
| {-# LANGUAGE EmptyDataDecls #-} | |||
There was a problem hiding this comment.
Was incorproated into the other test suite.
|
I'm gonna merge this on Wednesday if no one has any objections |
|
Will try to take a look at it tonight. |
|
Couldn't look in too much detail, and you've moved mountains, so from a somewhat intensive skimming, I'd say: "If it compiles, and tests, should probably be ok?" |
|
Yeah, the only thing that's really changed is that the MongoDB tests have totally different behavior on upsert. I'm tempted to call that a bug in the MongoDB library, but it may just be that they need to have totally different specifications factored out. |
| {-# LANGUAGE ScopedTypeVariables #-} | ||
| {-# LANGUAGE CPP #-} | ||
| {-# LANGUAGE DeriveGeneric #-} | ||
| {-# LANGUAGE EmptyDataDecls #-} |
There was a problem hiding this comment.
I get the feeling most of these pragmas in lots of files are completely superfluous, no?
I'll make a PR and go through pragmas and dependencies after this is merged to clean up, I think.
| - env: ARGS="--resolver lts-10 --stack-yaml stack_lts-10.yaml" | ||
| - env: ARGS="--resolver lts-11" | ||
| - env: ARGS="--resolver lts-12" | ||
| - env: ARGS="--resolver lts-13" |
There was a problem hiding this comment.
What's the reason for leaving in 6 and 9, but not the other ones between 6 and 11?
(EDIT: n/m I must have looked at a specific commit or something)
This PR removes all the CPP from the test suite.
I've pinged a few of you on a request; it's a big PR though and I won't be mad if you don't look at all of it. Self-review icoming.