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

Migrate tests to MUnit + other improvements #210

Merged
merged 11 commits into from
Mar 31, 2020
Merged

Migrate tests to MUnit + other improvements #210

merged 11 commits into from
Mar 31, 2020

Conversation

gabro
Copy link
Member

@gabro gabro commented Mar 27, 2020

This PR:

  • migrates all test to the MUnit library. Main advantages:
    • simpler and more consistent test definitions (each library was using a slightly different variant of ScalaTest suites)
    • nicer error reporting (highlighting of source line, advance diffing). See https://scalameta.org/munit/
  • resolves all mailo warnings (@calippo take a look) including the deprecations introduced by Akka 2.6
  • updates metarpheus to Scala.js 1.0
  • updates the version of Scala to 2.12.11 and of Sbt to 1.3.8
  • cleanup of some leftovers from the monorepo migration

@kanbanbot kanbanbot added the WIP label Mar 27, 2020
@gabro
Copy link
Member Author

gabro commented Mar 27, 2020

Still one mailo test failing (I'll investigate later)

@gabro gabro requested a review from calippo March 27, 2020 12:25
@kanbanbot kanbanbot added in review and removed WIP labels Mar 27, 2020
@gabro gabro requested a review from veej March 27, 2020 12:25
assert(Beer.Lager.isInstanceOf[Product])
assert(Beer.Lager.isInstanceOf[Serializable])
assert(Beer.Lager.isInstanceOf[Beer])
assertEquals(Beer.Lager, Beer.Lager)
Copy link
Member

@veej veej Mar 27, 2020

Choose a reason for hiding this comment

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

we lost an assertion here (Beer.Ale)

Copy link
Member Author

Choose a reason for hiding this comment

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

it was intentional, when I tried to rewrite the assertion it wouldn't even compile ("fruitless type test") and it's such an obvious test that I preferred to drop it.

@gabro gabro mentioned this pull request Mar 27, 2020
val mail2 = mail1.copy(subject = "2")

emailPersistanceActor ! SendEmail(mail1)
expectMsg(Queued)
Copy link
Member

Choose a reason for hiding this comment

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

This is failing in the CI, didn't investigate so much but we might try to specify duration like expectMsg(durationo)(). I see the defaultPatience is removed.

image

Copy link
Member 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 due to the change to the inmemory module of akka persistence (I've dropped the external library since it's not compatible with akka 2.6 and I'm using the internal one).
I'm aware of this and I'm trying to fix it (I can reproduce locally)

Copy link
Member Author

Choose a reason for hiding this comment

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

(patience was a ScalaTest thing, used for the eventually part which I have removed)

Copy link
Member Author

Choose a reason for hiding this comment

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

ok, fixed (it was a misconfiguration)

@calippo
Copy link
Member

calippo commented Mar 27, 2020

lgtm, fuck scalatest

@gabro
Copy link
Member Author

gabro commented Mar 31, 2020

Merging to unblock #211

@gabro gabro merged commit e6d2325 into master Mar 31, 2020
@gabro gabro deleted the munit branch March 31, 2020 10:13
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.

4 participants