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

Remove assert. #69

Merged
merged 1 commit into from
Oct 11, 2024
Merged

Remove assert. #69

merged 1 commit into from
Oct 11, 2024

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