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

Add doobie-munit package to integrate Doobie with the shiny new MUnit #1233

Merged
merged 28 commits into from
May 23, 2021
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
9edad27
Add doobie-munit package to integrate Doobie with the shiny new MUnit
Jul 13, 2020
ff5e0ce
fix: header check requires the copyright to have expired to years ago
Jul 13, 2020
b11538c
Merge branch 'master' into munit
alejandrohdezma Nov 18, 2020
d641056
Update MUnit version
alejandrohdezma Nov 18, 2020
1d96dfe
Avoid `Auto-application to `()` is deprecated` error
alejandrohdezma Nov 18, 2020
662a4dc
Pass `Location` around so failure occurs at the `check` call
alejandrohdezma Nov 18, 2020
32f2de3
Update `setup-scala` Github Action to latest version to fix build fail
alejandrohdezma Nov 18, 2020
357e6d4
Re-create headers
alejandrohdezma Nov 18, 2020
0ecf3b5
Merge branch 'master' into munit
alejandrohdezma Jan 19, 2021
229e833
Conform `doobie-munit` with `doobie-specs2`
alejandrohdezma Jan 28, 2021
c50c210
Merge branch 'master' into munit
alejandrohdezma Jan 28, 2021
5693893
Update MUnit version so it works in Scala 3
alejandrohdezma Jan 28, 2021
991b1fd
`docs` should depend on `munit`
alejandrohdezma Jan 28, 2021
f89be9b
`munit` package has to be imported as `_root_.munit`
alejandrohdezma Jan 28, 2021
13ea0c0
Merge branch 'master' into munit
alejandrohdezma Feb 10, 2021
539e1da
Add failing assertion and conform with specs2 tests
alejandrohdezma Feb 13, 2021
6a3d5a3
Merge branch 'master' into munit
alejandrohdezma Feb 13, 2021
a16dda7
Merge branch 'master' into munit
alejandrohdezma Feb 16, 2021
b161a60
Merge branch 'master' into munit
jatcwang Feb 17, 2021
e426df8
Merge branch 'master' into munit
jatcwang Feb 19, 2021
936f03e
Merge branch 'master' into munit
alejandrohdezma Mar 16, 2021
1a281b8
Merge branch 'master' into munit
jatcwang Apr 5, 2021
4ec4730
Merge branch 'master' into munit
jatcwang Apr 11, 2021
5b488a2
Merge branch 'master' into munit
alejandrohdezma Apr 13, 2021
9955bee
Merge branch 'master' into munit
jatcwang Apr 13, 2021
703211c
Merge branch 'master' into munit
Kazark May 20, 2021
4b3b84b
Update build.sbt
Kazark May 21, 2021
176a5a1
Merge branch 'master' into munit
jatcwang May 23, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 19 additions & 1 deletion build.sbt
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ lazy val postgresVersion = "42.2.18"
lazy val refinedVersion = "0.9.19"
lazy val scalaCheckVersion = "1.15.1"
lazy val scalatestVersion = "3.2.3"
lazy val munitVersion = "0.7.21"
lazy val silencerVersion = "1.7.1"
lazy val specs2Version = "4.10.6"
lazy val scala212Version = "2.12.12"
Expand Down Expand Up @@ -162,6 +163,7 @@ lazy val doobie = project.in(file("."))
quill,
refined,
scalatest,
munit,
specs2,
)

Expand Down Expand Up @@ -390,6 +392,22 @@ lazy val scalatest = project
)
.settings(noDottySettings)

lazy val munit = project
.in(file("modules/munit"))
.enablePlugins(AutomateHeaderPlugin)
.dependsOn(core)
.settings(doobieSettings)
.settings(publishSettings)
.settings(
name := s"doobie-munit",
description := "MUnit support for doobie.",
testFrameworks += new TestFramework("munit.Framework"),
libraryDependencies ++= Seq(
"org.scalameta" %% "munit" % munitVersion,
"com.h2database" % "h2" % h2Version % "test"
)
)

lazy val bench = project
.in(file("modules/bench"))
.enablePlugins(AutomateHeaderPlugin)
Expand All @@ -400,7 +418,7 @@ lazy val bench = project

lazy val docs = project
.in(file("modules/docs"))
.dependsOn(core, postgres, specs2, hikari, h2, scalatest, quill)
.dependsOn(core, postgres, specs2, munit, hikari, h2, scalatest, quill)
.enablePlugins(ParadoxPlugin)
.enablePlugins(ParadoxSitePlugin)
.enablePlugins(GhpagesPlugin)
Expand Down
2 changes: 1 addition & 1 deletion modules/docs/src/main/mdoc/docs/06-Checking.md
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ def biggerThan2(minPop: Int) =
biggerThan2(0).check.unsafeRunSync()
```

**doobie** supports `check` for queries and updates in three ways: programmatically, via YOLO mode in the REPL, and via the `doobie-specs2` and `doobie-scalatest` packages, which allow checking to become part of your unit test suite. We will investigate this in the chapter on testing.
**doobie** supports `check` for queries and updates in four ways: programmatically, via YOLO mode in the REPL, and via the `doobie-specs2`, `doobie-scalatest` and `doobie-munit` packages, which allow checking to become part of your unit test suite. We will investigate this in the chapter on testing.

Choose a reason for hiding this comment

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

Take or leave it: might a bulleted list be easier to view?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think that's a reasonable idea; the general principle of the PR was to try to stick closely with what's there. We'll see what @tpolecat says.

Copy link
Collaborator

Choose a reason for hiding this comment

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

idm what's currently here, since it's featured prominantly in 13-Unit-Testing.md :)


### Working Around Bad Metadata

Expand Down
24 changes: 23 additions & 1 deletion modules/docs/src/main/mdoc/docs/13-Unit-Testing.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
## Unit Testing

The YOLO-mode query checking feature demonstated in an earlier chapter is also available as a trait you can mix into your [Specs2](http://etorreborre.github.io/specs2/) or [ScalaTest](http://www.scalatest.org/) unit tests.
The YOLO-mode query checking feature demonstated in an earlier chapter is also available as a trait you can mix into your [Specs2](http://etorreborre.github.io/specs2/), [ScalaTest](http://www.scalatest.org/) or [MUnit](https://scalameta.org/munit) unit tests.

### Setting Up

Expand Down Expand Up @@ -129,3 +129,25 @@ Details are shown for failing tests.
// Run a test programmatically. Usually you would do this from sbt, bloop, etc.
(new AnalysisTestScalaCheck).execute(color = false)
```

### The MUnit Package

The `doobie-munit` add-on provides a mix-in trait that we can add to any `Assertions` implementation (like `FunSuite`) much like the ScalaTest package above.

```scala mdoc:silent
import _root_.munit._

class AnalysisTestSuite extends FunSuite with doobie.munit.IOChecker {

override val colors = doobie.util.Colors.None // just for docs

val transactor = Transactor.fromDriverManager[IO](
"org.postgresql.Driver", "jdbc:postgresql:world", "postgres", ""
)

test("trivial") { check(trivial) }
test("biggerThan") { check(biggerThan(0)) }
test("update") { check(update("", "")) }

}
```
67 changes: 67 additions & 0 deletions modules/munit/src/main/scala/doobie/munit/analysisspec.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
// Copyright (c) 2013-2020 Rob Norris and Contributors
// This software is licensed under the MIT License (MIT).
// For more information see LICENSE or https://opensource.org/licenses/MIT

package doobie.munit

import cats.effect.{ Effect, IO }
import doobie.util.query.{Query, Query0}
import doobie.util.testing._
import org.tpolecat.typename._
import munit.Assertions
import munit.Location

/**
* Module with a mix-in trait for specifications that enables checking of doobie `Query` and `Update` values.
*
* {{{
* class ExampleSuite extends FunSuite with IOChecker {
*
* // The transactor to use for the tests.
* val transactor = Transactor.fromDriverManager[IO](
* "org.postgresql.Driver",
* "jdbc:postgresql:world",
* "postgres", ""
* )
*
* // Now just mention the queries. Arguments are not used.
* test("findByNameAndAge") { check(MyDaoModule.findByNameAndAge(null, 0)) }
* test("allWoozles") { check(MyDaoModule.allWoozles) }
*
* }
* }}}
*/
object analysisspec {

trait Checker[M[_]] extends CheckerBase[M] { this: Assertions =>

def check[A: Analyzable](a: A)(implicit loc: Location) = checkImpl(Analyzable.unpack(a))

def checkOutput[A: TypeName](q: Query0[A])(implicit loc: Location) =
checkImpl(AnalysisArgs(
s"Query0[${typeName[A]}]", q.pos, q.sql, q.outputAnalysis
))

def checkOutput[A: TypeName, B: TypeName](q: Query[A, B])(implicit loc: Location) =
checkImpl(AnalysisArgs(
s"Query[${typeName[A]}, ${typeName[B]}]", q.pos, q.sql, q.outputAnalysis
))

private def checkImpl(args: AnalysisArgs)(implicit loc: Location) = {
val report = analyzeIO(args, transactor).unsafeRunSync()
if (!report.succeeded) {
fail(
formatReport(args, report, colors)
.padLeft(" ")
.toString
)
}
Comment on lines +51 to +58
Copy link

@ValdemarGr ValdemarGr Mar 30, 2021

Choose a reason for hiding this comment

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

Wouldn't levering https://github.com/typelevel/munit-cats-effect be a better choice?
Something like.

Suggested change
val report = analyzeIO(args, transactor).unsafeRunSync()
if (!report.succeeded) {
fail(
formatReport(args, report, colors)
.padLeft(" ")
.toString
)
}
val report = analyzeIO(args, transactor)
report.flatMap{
case r if !r.succeeded =>
IO(fail(
formatReport(args, report, colors)
.padLeft(" ")
.toString
))
case _ => IO.unit
}

Choose a reason for hiding this comment

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

I have implemented the munit-cats-effect integration at
https://github.com/ValdemarGr/doobie/tree/munit
Can someone give any insight or consideration for the idea?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Sounds like a good idea. It'll also make CE3 migration easier

Choose a reason for hiding this comment

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

This implementation also changes the semantics of check; it is no longer eager, is this desired?
The scalatest and specs2 test utilities seem to be eager.

Copy link
Collaborator

Choose a reason for hiding this comment

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

If execution thread gets to check(..) then we're running the test already right? (This PR's impl seems eager to me, unless I'm missing something)

munit (and all other test frameworks that I know) all use call-by-name to allow skipping tests, so test("asdf") { check(..) } is still skippable even if check is eager.

Copy link

@ValdemarGr ValdemarGr Mar 30, 2021

Choose a reason for hiding this comment

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

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ah yeah I think the other integrations (scalatest/specs) run it eagerly. Perhaps it is worth keeping that behaviour to make it easier for people to migrate between test libs. (I know that in my codebase I have a few check(..) being called as if they're statements)

So I guess cats-effect munit won't help us here. Sorry!

}
}

/** Implementation of Checker[IO] */
trait IOChecker extends Checker[IO] {
self: Assertions =>
val M: Effect[IO] = implicitly
}
}
12 changes: 12 additions & 0 deletions modules/munit/src/main/scala/doobie/munit/package.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
// Copyright (c) 2013-2020 Rob Norris and Contributors
// This software is licensed under the MIT License (MIT).
// For more information see LICENSE or https://opensource.org/licenses/MIT

package doobie

package object munit {

type Checker[M[_]] = analysisspec.Checker[M]
type IOChecker = analysisspec.IOChecker

}
30 changes: 30 additions & 0 deletions modules/munit/src/test/scala/doobie/munit/CheckerTests.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
// Copyright (c) 2013-2020 Rob Norris and Contributors
// This software is licensed under the MIT License (MIT).
// For more information see LICENSE or https://opensource.org/licenses/MIT

package doobie.munit

import cats.effect.{ ContextShift, IO }
import doobie.syntax.string._
import doobie.util.transactor.Transactor
import munit._
import scala.concurrent.ExecutionContext

trait CheckerChecks[M[_]] extends FunSuite with Checker[M] {

implicit def contextShift: ContextShift[M]

lazy val transactor = Transactor.fromDriverManager[M](
"org.h2.Driver",
"jdbc:h2:mem:queryspec;DB_CLOSE_DELAY=-1",
"sa", ""
)

test("trivial") { check(sql"select 1".query[Int]) }
jatcwang marked this conversation as resolved.
Show resolved Hide resolved

}

class IOCheckerCheck extends CheckerChecks[IO] with IOChecker {
def contextShift: ContextShift[IO] =
IO.contextShift(ExecutionContext.global)
}