Skip to content

Conversation

zainab-ali
Copy link
Contributor

This PR resolves #67 by removing the assert call entirely.

An attempt was made to make assert throw in #68 , however the assert function didn't compose well. Existing users of assert would need to rewrite their code to use expect, or use assert but with poorer syntax. For example:

pureTest("some test") {
  assert(1 == 1)
}

would need to be written as

pureTest("some test") {
  assert(1 == 1)
  success
}

Instead, we can remove assert entirely in favour of expect. As a follow up, we can have a scalafix rule to help users migrate.

@Baccata
Copy link
Collaborator

Baccata commented Oct 11, 2024

For the record : I've tried to play with different encodings to try and make assert work, by changing the definition of the various test methods to allow for it.

The issue is that weaver has been using overloads since forever to allow for various usecases semi-transparently. Overloads don't work well with polymorphism, leading the compiler to ask for more information from the user as it's unable to infer types as well as the current state.

So, although we could make it work by deleting the overloads before the next release, it'd lead to a level of inconvenience to existing users that I'm just not comfortable triggering.

Therefore, removal it is.

@Baccata Baccata merged commit 372e02e into typelevel:main Oct 11, 2024
13 checks passed
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.

Make assert (and variants) throw
2 participants