From 250404485b8038f7697a56e1875010351ff0ff35 Mon Sep 17 00:00:00 2001 From: Olafur Pall Geirsson Date: Fri, 20 Mar 2020 08:21:33 +0000 Subject: [PATCH 1/6] Relax `assertEquals` strictness about type equality. Previously, `assertEquals` only allowed comparing values of the same type. Now, it's OK as long as the second argument is a subtype of the first argument. The reasoning for this change is that it's common that you want to test some generic value like `Option[Int]` obtained from a method we're testing against an expected hardcoded value like `Some[Int]`. --- docs/assertions.md | 17 ++++++++--------- .../src/main/scala/munit/Assertions.scala | 2 +- .../src/test/scala/munit/AssertionsSuite.scala | 11 +++++++++++ 3 files changed, 20 insertions(+), 10 deletions(-) diff --git a/docs/assertions.md b/docs/assertions.md index 094a3896..28efe004 100644 --- a/docs/assertions.md +++ b/docs/assertions.md @@ -83,19 +83,18 @@ Comparing two values of different types is a compile error. assertEquals(1, "") ``` -The two types must match exactly, it's a type error to compare two values even -if one value is a subtype of the other. +The "expected" value (second argument) must be a subtype of the "obtained" value +(first argument). It's a type error to compare two values even if one value is a +subtype of the other. -```scala mdoc:fail -assertEquals(Some(1), Option(1)) +```scala mdoc +assertEquals(Option(1), Some(1)) ``` -Upcast the subtype using a type ascription `subtype: Supertype` when you want to -compare a subtype with a supertype. +It's a compile error if you swap the order of the arguments. -```scala mdoc -// OK -assertEquals(Some(1): Option[Int], Option(1)) +```scala mdoc:fail +assertEquals(Some(1), Option(1)) ``` ## `assertNotEquals()` diff --git a/munit/shared/src/main/scala/munit/Assertions.scala b/munit/shared/src/main/scala/munit/Assertions.scala index e8db68d2..a7ba99d4 100644 --- a/munit/shared/src/main/scala/munit/Assertions.scala +++ b/munit/shared/src/main/scala/munit/Assertions.scala @@ -76,7 +76,7 @@ trait Assertions extends MacroCompat.CompileErrorMacro { obtained: A, expected: B, clue: => Any = "values are not the same" - )(implicit loc: Location, ev: A =:= B): Unit = { + )(implicit loc: Location, ev: B <:< A): Unit = { StackTraces.dropInside { if (obtained != expected) { Diffs.assertNoDiff( diff --git a/tests/shared/src/test/scala/munit/AssertionsSuite.scala b/tests/shared/src/test/scala/munit/AssertionsSuite.scala index b65e52d9..57f98f38 100644 --- a/tests/shared/src/test/scala/munit/AssertionsSuite.scala +++ b/tests/shared/src/test/scala/munit/AssertionsSuite.scala @@ -47,4 +47,15 @@ class AssertionsSuite extends BaseSuite { |} |""".stripMargin ) + + test("subtype".tag(NoDotty)) { + assertEquals(Option(1), Some(1)) + assertNoDiff( + compileErrors("assertEquals(Some(1), Option(1))"), + """|error: Cannot prove that Option[Int] <:< Some[Int]. + |assertEquals(Some(1), Option(1)) + | ^ + |""".stripMargin + ) + } } From 66262cc21d11f237264ebfd3f038e23c49ee9038 Mon Sep 17 00:00:00 2001 From: Olafur Pall Geirsson Date: Fri, 20 Mar 2020 08:39:05 +0000 Subject: [PATCH 2/6] Document `assertEquals[Any, Any]` workaround --- docs/assertions.md | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/docs/assertions.md b/docs/assertions.md index 28efe004..3afc24f8 100644 --- a/docs/assertions.md +++ b/docs/assertions.md @@ -97,6 +97,17 @@ It's a compile error if you swap the order of the arguments. assertEquals(Some(1), Option(1)) ``` +Use `assertEquals[Any, Any]` if you really want to compare two different types. + +```scala mdoc +val right1: Either[String , Int] = Right(42) +val right2: Either[List[String], Int] = Right(42) +``` + +```scala mdoc +assertEquals[Any, Any](right1, right2) +``` + ## `assertNotEquals()` Use `assertNotEqual()` to assert that two values are not the same. From 972f360498f6be559f32decd254f0abf4e9226db Mon Sep 17 00:00:00 2001 From: Olafur Pall Geirsson Date: Fri, 20 Mar 2020 08:58:00 +0000 Subject: [PATCH 3/6] Remove false claim from docs --- docs/assertions.md | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/docs/assertions.md b/docs/assertions.md index 3afc24f8..50f5f8e3 100644 --- a/docs/assertions.md +++ b/docs/assertions.md @@ -84,8 +84,7 @@ assertEquals(1, "") ``` The "expected" value (second argument) must be a subtype of the "obtained" value -(first argument). It's a type error to compare two values even if one value is a -subtype of the other. +(first argument). ```scala mdoc assertEquals(Option(1), Some(1)) From 88e1a4b382798732f23d6efb8281012aa240aa1f Mon Sep 17 00:00:00 2001 From: Olafur Pall Geirsson Date: Fri, 20 Mar 2020 10:10:50 +0000 Subject: [PATCH 4/6] Format codebase with JavaDoc-style docstrings --- .../src/main/scala/munit/internal/JSIO.scala | 38 +++++++++---------- .../src/main/scala/munit/GenericTest.scala | 12 +++--- .../src/main/scala/munit/Printable.scala | 4 +- .../shared/src/main/scala/munit/Printer.scala | 12 +++--- munit/shared/src/main/scala/munit/Suite.scala | 24 ++++++------ .../src/main/scala/munit/TestOptions.scala | 18 ++++----- .../src/main/scala/munit/TestValues.scala | 4 +- 7 files changed, 56 insertions(+), 56 deletions(-) diff --git a/munit/js/src/main/scala/munit/internal/JSIO.scala b/munit/js/src/main/scala/munit/internal/JSIO.scala index f36b443a..ac61eeb9 100644 --- a/munit/js/src/main/scala/munit/internal/JSIO.scala +++ b/munit/js/src/main/scala/munit/internal/JSIO.scala @@ -5,30 +5,30 @@ import scala.scalajs.js.annotation.JSImport import scala.scalajs.js.annotation.JSImport.Namespace /** Facade for the native nodejs process API - * - * The process object is a global that provides information about, and - * control over, the current Node.js process. As a global, it is always - * available to Node.js applications without using require(). - * - * @see https://nodejs.org/api/process.html - */ + * + * The process object is a global that provides information about, and + * control over, the current Node.js process. As a global, it is always + * available to Node.js applications without using require(). + * + * @see https://nodejs.org/api/process.html + */ @js.native trait JSProcess extends js.Any { def cwd(): String = js.native } /** Facade for native nodejs module "fs". - * - * @see https://nodejs.org/api/fs.html - */ + * + * @see https://nodejs.org/api/fs.html + */ @js.native @JSImport("fs", Namespace) object JSFs extends js.Any { /** Returns the file contents as Buffer using blocking apis. - * - * NOTE: The actual return value is a Node.js buffer and not js.Array[Int]. - * However, both support .length and angle bracket access (foo[1]). + * + * NOTE: The actual return value is a Node.js buffer and not js.Array[Int]. + * However, both support .length and angle bracket access (foo[1]). **/ def readFileSync(path: String): js.Array[Int] = js.native @@ -52,9 +52,9 @@ object JSFs extends js.Any { } /** Facade for nodejs class fs.Stats. - * - * @see https://nodejs.org/api/fs.html#fs_class_fs_stats - */ + * + * @see https://nodejs.org/api/fs.html#fs_class_fs_stats + */ @js.native @JSImport("fs", Namespace) class JSStats extends js.Any { @@ -63,9 +63,9 @@ class JSStats extends js.Any { } /** Facade for native nodejs module "path". - * - * @see https://nodejs.org/api/path.html - */ + * + * @see https://nodejs.org/api/path.html + */ @js.native @JSImport("path", Namespace) object JSPath extends js.Any { diff --git a/munit/shared/src/main/scala/munit/GenericTest.scala b/munit/shared/src/main/scala/munit/GenericTest.scala index 6ec3b904..32c6cafc 100644 --- a/munit/shared/src/main/scala/munit/GenericTest.scala +++ b/munit/shared/src/main/scala/munit/GenericTest.scala @@ -5,12 +5,12 @@ import java.lang.annotation.Annotation import scala.collection.mutable /** - * Metadata about a single test case. - * - * @param body the function to be evaluated for this test case. - * @param tags the annotated tags for this test case. - * @param location the file and line number where this test was defined. - */ + * Metadata about a single test case. + * + * @param body the function to be evaluated for this test case. + * @param tags the annotated tags for this test case. + * @param location the file and line number where this test was defined. + */ class GenericTest[T]( val name: String, val body: () => T, diff --git a/munit/shared/src/main/scala/munit/Printable.scala b/munit/shared/src/main/scala/munit/Printable.scala index 94d3e71c..6947b7a5 100644 --- a/munit/shared/src/main/scala/munit/Printable.scala +++ b/munit/shared/src/main/scala/munit/Printable.scala @@ -1,8 +1,8 @@ package munit /** - * Override this class to customize the default pretty-printer. - */ + * Override this class to customize the default pretty-printer. + */ trait Printable { def print(out: StringBuilder, indent: Int): Unit } diff --git a/munit/shared/src/main/scala/munit/Printer.scala b/munit/shared/src/main/scala/munit/Printer.scala index 2e09a668..743abd3f 100644 --- a/munit/shared/src/main/scala/munit/Printer.scala +++ b/munit/shared/src/main/scala/munit/Printer.scala @@ -1,15 +1,15 @@ package munit /** - * Implement this trait to customize the default printer - */ + * Implement this trait to customize the default printer + */ trait Printer { /** - * Pretty-print a single value during pretty printing. - * - * Returns true if this value has been printed, false if FunSuite should fallback to the default pretty-printer. - */ + * Pretty-print a single value during pretty printing. + * + * Returns true if this value has been printed, false if FunSuite should fallback to the default pretty-printer. + */ def print(value: Any, out: StringBuilder, indent: Int): Boolean def height: Int = 100 def isMultiline(string: String): Boolean = diff --git a/munit/shared/src/main/scala/munit/Suite.scala b/munit/shared/src/main/scala/munit/Suite.scala index f8557dec..aeb272b5 100644 --- a/munit/shared/src/main/scala/munit/Suite.scala +++ b/munit/shared/src/main/scala/munit/Suite.scala @@ -4,8 +4,8 @@ import org.junit.runner.RunWith import scala.concurrent.ExecutionContext /** The base class for all test suites. - * Extend this class if you don't need the functionality in FunSuite. - */ + * Extend this class if you don't need the functionality in FunSuite. + */ @RunWith(classOf[MUnitRunner]) abstract class Suite extends PlatformSuite { @@ -28,10 +28,10 @@ abstract class Suite extends PlatformSuite { def munitExecutionContext: ExecutionContext = parasiticExecutionContext /** - * - * @param name The name of this fixture, used for displaying an error message if - * `beforeAll()` or `afterAll()` fail. - */ + * + * @param name The name of this fixture, used for displaying an error message if + * `beforeAll()` or `afterAll()` fail. + */ abstract class Fixture[T](val fixtureName: String) { /** The value produced by this suite-local fixture that can be reused for all test cases. */ @@ -41,8 +41,8 @@ abstract class Suite extends PlatformSuite { def beforeAll(): Unit = () /** Runs before each individual test case. - * An error in this method aborts the test case. - */ + * An error in this method aborts the test case. + */ def beforeEach(context: BeforeEach): Unit = () /** Runs after each individual test case. */ @@ -54,16 +54,16 @@ abstract class Suite extends PlatformSuite { } /** Runs once before all test cases and before all suite-local fixtures are setup. - * An error in this method aborts the test suite. - */ + * An error in this method aborts the test suite. + */ def beforeAll(): Unit = () /** Runs once after all test cases and after all suite-local fixtures have been tear down. */ def afterAll(): Unit = () /** Runs before each individual test case. - * An error in this method aborts the test case. - */ + * An error in this method aborts the test case. + */ def beforeEach(context: BeforeEach): Unit = () /** Runs after each individual test case. */ diff --git a/munit/shared/src/main/scala/munit/TestOptions.scala b/munit/shared/src/main/scala/munit/TestOptions.scala index 9ffbe4d7..30b3f7e6 100644 --- a/munit/shared/src/main/scala/munit/TestOptions.scala +++ b/munit/shared/src/main/scala/munit/TestOptions.scala @@ -1,12 +1,12 @@ package munit /** - * Options used when running a test. It can be built implicitly from a [[String]] - * (@see [[munit.TestOptionsConversions]]) - * - * @param name the test name, used in the UI and to select it with testOnly - * @param tags a set of [[munit.Tag]], used to attach semantic information to a test - */ + * Options used when running a test. It can be built implicitly from a [[String]] + * (@see [[munit.TestOptionsConversions]]) + * + * @param name the test name, used in the UI and to select it with testOnly + * @param tags a set of [[munit.Tag]], used to attach semantic information to a test + */ final class TestOptions( val name: String, val tags: Set[Tag], @@ -40,9 +40,9 @@ final class TestOptions( trait TestOptionsConversions { /** - * Implicitly create a TestOptions given a test name. - * This allows writing `test("name") { ... }` even if `test` accepts a `TestOptions` - */ + * Implicitly create a TestOptions given a test name. + * This allows writing `test("name") { ... }` even if `test` accepts a `TestOptions` + */ implicit def testOptionsFromString( name: String )(implicit loc: Location): TestOptions = diff --git a/munit/shared/src/main/scala/munit/TestValues.scala b/munit/shared/src/main/scala/munit/TestValues.scala index 55fede6d..f0367ab9 100644 --- a/munit/shared/src/main/scala/munit/TestValues.scala +++ b/munit/shared/src/main/scala/munit/TestValues.scala @@ -3,8 +3,8 @@ package munit import scala.util.control.NoStackTrace /** - * Values that have special treatment when evaluating values produced by tests. - */ + * Values that have special treatment when evaluating values produced by tests. + */ object TestValues { /** The test failed with the given exception but was ignored but its marked as flaky */ From e60c1df25e6d809eb3f4ba480f01b2d82252e61b Mon Sep 17 00:00:00 2001 From: Olafur Pall Geirsson Date: Fri, 20 Mar 2020 10:10:15 +0000 Subject: [PATCH 5/6] Add docstring for `assertEquals`. Users navigating to the source via IDEs can then learn about the `assertEquals[Any, Any]` escape hatch. --- .scalafmt.conf | 1 + .../src/main/scala/munit/Assertions.scala | 19 +++++++++++++++++++ 2 files changed, 20 insertions(+) diff --git a/.scalafmt.conf b/.scalafmt.conf index e84da2f7..cef838bb 100644 --- a/.scalafmt.conf +++ b/.scalafmt.conf @@ -1,5 +1,6 @@ version = "2.3.2" assumeStandardLibraryStripMargin = true +docstrings = JavaDoc project.git=true project.excludeFilters = [ ".*scala-3" diff --git a/munit/shared/src/main/scala/munit/Assertions.scala b/munit/shared/src/main/scala/munit/Assertions.scala index a7ba99d4..d09a5d1e 100644 --- a/munit/shared/src/main/scala/munit/Assertions.scala +++ b/munit/shared/src/main/scala/munit/Assertions.scala @@ -72,6 +72,25 @@ trait Assertions extends MacroCompat.CompileErrorMacro { } } + /** + * Asserts that two elements are equal using `==` equality. + * + * The "expected" value (second argument) must have the same type or be a + * subtype of the "obtained" value (first argument). For example: + * {{{ + * assertEquals(Option(1), Some(1)) // OK + * assertEquals(Some(1), Option(1)) // Error: Option[Int] is not a subtype of Some[Int] + * }}} + * + * Use `assertEquals[Any, Any](a, b)` as an escape hatch to compare two + * values of different types. For example: + * {{{ + * val a: Either[List[String], Int] = Right(42) + * val b: Either[String, Int] = Right(42) + * assertEquals[Any, Any](a, b) // OK + * assertEquals(a, b) // Error: Either[String, Int] is not a subtype of Either[List[String], Int] + * }}} + */ def assertEquals[A, B]( obtained: A, expected: B, From 1406f649b031ce45f5cb3d51ab91a6d0553553de Mon Sep 17 00:00:00 2001 From: Olafur Pall Geirsson Date: Fri, 20 Mar 2020 10:15:48 +0000 Subject: [PATCH 6/6] Use bash on Windows CI --- .gitattributes | 2 ++ .github/workflows/ci.yml | 3 ++- .jvmopts | 2 +- 3 files changed, 5 insertions(+), 2 deletions(-) create mode 100644 .gitattributes diff --git a/.gitattributes b/.gitattributes new file mode 100644 index 00000000..80f45d31 --- /dev/null +++ b/.gitattributes @@ -0,0 +1,2 @@ +* eol=lf +*.png eol=autocrlf diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 87411709..64112334 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -52,7 +52,8 @@ jobs: steps: - uses: actions/checkout@v2 - uses: olafurpg/setup-scala@v7 - - run: sbt +testsJVM/test + - run: csbt +testsJVM/test + shell: bash env: GOOGLE_APPLICATION_CREDENTIALS: ${{ secrets.GOOGLE_APPLICATION_CREDENTIALS }} diff --git a/.jvmopts b/.jvmopts index 778b9a75..daac33f6 100644 --- a/.jvmopts +++ b/.jvmopts @@ -1,4 +1,4 @@ --Xss4m +-Xss2m -Xms1G -Xmx2G -XX:ReservedCodeCacheSize=1024m