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

Port tests to munit #943

Merged
merged 3 commits into from
Nov 11, 2020
Merged

Port tests to munit #943

merged 3 commits into from
Nov 11, 2020

Conversation

cquiroz
Copy link
Collaborator

@cquiroz cquiroz commented Nov 5, 2020

This PR ports all tests to munit. I had to disable a few tests that I'll point in the comments.
This also upgrades discipline-core

@julien-truffaut
Copy link
Member

julien-truffaut commented Nov 7, 2020

Looks great. I didn't see any test disabled.

@@ -20,7 +20,7 @@ class StringsSpec extends MonocleSuite {
checkAll("String to Int", PrismTests(stringToInt))
checkAll("String to Long", PrismTests(stringToLong))
checkAll("String to UUID", PrismTests(stringToUUID))
checkAll("String to URI", PrismTests(stringToURI))
// checkAll("String to URI", PrismTests(stringToURI))
Copy link
Member

Choose a reason for hiding this comment

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

Here is one.

}

test("monocle.std.all._ brings all polymorphic Optic instances in scope for standard Scala classes") {
import monocle.function.all._

// do not compile because Head instance for HList is not in scope
illTyped("""head[Int :: HNil, Int].modify(1 :: HNil, _ + 1) shouldEqual (2 :: HNil)""")
illTyped("""assertEquals(head[Int :: HNil, Int].modify(1 :: HNil, _ + 1), (2 :: HNil))""")
Copy link
Member

Choose a reason for hiding this comment

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

On a second pass, we may want to use compileErrors from munit instead of illTyped

@julien-truffaut
Copy link
Member

I propose that we merge it and a few tickets to tackle the commented tests. WDYT?

@cquiroz
Copy link
Collaborator Author

cquiroz commented Nov 11, 2020

Sounds good, I think munit will make it easier to support dotty

@cquiroz cquiroz merged commit aed2aaf into optics-dev:master Nov 11, 2020
@cquiroz cquiroz deleted the munit branch November 11, 2020 11:09
@julien-truffaut
Copy link
Member

Sounds good, I think munit will make it easier to support dotty

Definitely!

}

test("GenIso nullary equality") {
GenIso.unit[Nullary] shouldEqual _nullary
test("GenIso nullary equality".ignore) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

These are not working anymore, but TBH I don't know how could that have worked in the past

@@ -20,7 +20,7 @@ class StringsSpec extends MonocleSuite {
checkAll("String to Int", PrismTests(stringToInt))
checkAll("String to Long", PrismTests(stringToLong))
checkAll("String to UUID", PrismTests(stringToUUID))
checkAll("String to URI", PrismTests(stringToURI))
// checkAll("String to URI", PrismTests(stringToURI))
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This one is odd. I think it is related to changes on Scalia check but I can't pinpoint the reason. Enabling it triggers an error during the generation of URIs

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