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

Move to MUnit and whale-tail #408

Merged
merged 2 commits into from
Jun 6, 2023
Merged

Move to MUnit and whale-tail #408

merged 2 commits into from
Jun 6, 2023

Conversation

milessabin
Copy link
Member

  • All tests reworked in terms of Munit.
  • All test (file)names normalised to "suite" from "spec".
  • Replaced test-containers with whale-tail to allow cross building for JS and native.

+ All tests reworked in terms of Munit.
+ All test (file)names normalised to "suite" from "spec".
+ Replaced test-containers with whale-tail to allow cross building for
  JS and native.
Copy link
Member

@armanbilge armanbilge left a comment

Choose a reason for hiding this comment

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

Admittedly only skimmed through an unfamiliar codebase but actually this seemed like a pretty straightforward migration, besides the whale tail stuff.

build.sbt Outdated Show resolved Hide resolved
Comment on lines 20 to +21
// Slow because each usage will open a new socket, but ok for now.
lazy val pool: Resource[IO, Session[IO]] =
lazy val pool: Resource[IO, Session[IO]] = {
Copy link
Member

Choose a reason for hiding this comment

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

We can replace this with a "resource fixture" that is shared for the entire suite in a follow-up PR.

password = Some(container.password),
database = container.databaseName,
// debug = true,
host = if (host == "0.0.0.0") "127.0.0.1" else host,
Copy link
Member

Choose a reason for hiding this comment

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

Note-to-self to look at this.

@armanbilge
Copy link
Member

After this merges I'll work on the JS and Native cross-builds. Thanks, these changes are a big help towards that!

@milessabin milessabin merged commit b0d5096 into main Jun 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants