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

Replace expecty with clue #64

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

zainab-ali
Copy link
Contributor

@zainab-ali zainab-ali commented Sep 16, 2024

This PR replaces expecty with an munit-like clue function.

It uses macros to generate clues. Unlike munit, clues are stored locally to the expect call. This avoids the need for a global lock, and means tests can run concurrently, and add clues, without relying on any extra concurrency primitives.

It assumes that the code within an expect call is synchronous.

The behaviour is identical for Scala 2 and 3, but the macros are implemented differently.

Note that the signatures for expect aren't identical for Scala 2 and 3. The Scala 3 expect functions use context functions.

Behaviour

A user can write:

val x = 1
val y = 2
expect(clue(x) == clue(y))

and will see the following on test failure:

Clues {
  x: Int = 1
  y: Int = 2
}

They can also nest calls to clue:

val x = 1
val y = 2
expect(clue(List(clue(x))) == clue(List(clue(y))))

Scala 2 implementation

The Scala 2 implementation has a couple of macros:

  • The ClueMacro generates a clue for a value expression. This is identical to munit's clue macro.
  • The ExpectMacro constructs a local collection of Clues. Calls to the clue function are rewritten to call the addClue function of the local collection.

Scala 3 implementation

In Scala 3, we can use context functions to provide the Clues collection. It still has a couple of macros:

  • The ClueMacro generates a clue for a value expression using simpler logic for obtaining the source code and type.
  • The ExpectMacro gets the source location to ensure that the tracing behaviour of expect is the same as it was previously. Calls to clue don't need to be rewritten.

@zainab-ali zainab-ali marked this pull request as draft September 16, 2024 13:24
@zainab-ali zainab-ali force-pushed the munit-clue-integration branch 3 times, most recently from 6f24200 to 1bda9bb Compare September 17, 2024 14:16
@zainab-ali zainab-ali marked this pull request as ready for review September 17, 2024 14:27
@zainab-ali zainab-ali changed the title WIP: Basic clue implementation. Replace expecty with clue Sep 17, 2024
@zainab-ali zainab-ali mentioned this pull request Sep 19, 2024
14 tasks
import c.universe._

def fromContext: Tree = {
def fromContext: c.Tree = {
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍

"This function can only be used within `expect` or `assert`.")
final def clue[A](@unused a: Clue[A]): A = {
// This function is removed as part of the `expect` macro expansion.
throw new Error("compileTimeOnly annotation not respected! This is likely to be a bug in weaver-test. Report it at https://github.com/typelevel/weaver-test/issues/new")
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍

override def transform(input: Tree): Tree = input match {
case c.universe.Apply(fun, List(clueValue))
if fun.symbol == clueMethodSymbol =>
val transformedClueValue = super.transform(clueValue)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm curious about this transformation here. Can you explain ?

@Baccata
Copy link
Collaborator

Baccata commented Oct 1, 2024

This is great ! Thank you so much for taking that on !🥇

I just have a minor question, but I'm happy with the code, tests, and documentation.

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