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

Simplify @effect/vitest Test Effect API #4163

Closed
QuentinJanuel opened this issue Dec 18, 2024 · 7 comments
Closed

Simplify @effect/vitest Test Effect API #4163

QuentinJanuel opened this issue Dec 18, 2024 · 7 comments
Labels
enhancement New feature or request

Comments

@QuentinJanuel
Copy link
Contributor

What is the problem this feature would solve?

At the moment, test effects in @effect/vitest are defined using the following pattern:

it.effect("message", () => someEffect)

Because someEffect is already a lazy computation, the arrow function feels redundant and does not simplify the developer experience.

What is the feature you are proposing to solve the problem?

Replace the current API with a direct call, removing the unnecessary arrow function:

it.effect("message", someEffect)

It would work on most cases, except when the function is actually used to pass an argument like in

it.effect.each(list)("message", item => someEffect)

but in these cases we could use an effect service like this:

it.effect.each(list)("message", E.gen(function* () {
  const item = yield* it.get // Consider a good naming here
  ...
}))

Furthermore, since all use cases I can think about would just use E.gen, we could streamline this further by requiring the generator function directly:

it.effect("message", function* () {
  // ...
})

I’m willing to implement this feature if it aligns with the project's direction. Before proceeding, I'd like to confirm whether this approach is desirable for @effect/vitest.

What alternatives have you considered?

No response

@QuentinJanuel QuentinJanuel added the enhancement New feature or request label Dec 18, 2024
@mikearnaldi
Copy link
Member

I kind of like:

it.effect("message", function* () {
  // ...
})

better than any alternative

@jessekelly881
Copy link
Contributor

jessekelly881 commented Dec 19, 2024

Don't forget test context. This might work well though.

it.effect("message", function* (ctx) {
  // ...
})

Or in the case of each or prop:

it.effect("message", function* (ctx, item) {
  // ...
})

@QuentinJanuel
Copy link
Contributor Author

I don't know if having the test context as a parameter of the generator function is possible, and if so that might just be the cleanest indeed, but otherwise we can just provide a service to fetch it too

@mikearnaldi
Copy link
Member

I don't know if having the test context as a parameter of the generator function is possible, and if so that might just be the cleanest indeed, but otherwise we can just provide a service to fetch it too

having it is indeed possible, what I am more worried about is instead pipeability, many times you need to add stuff like Effect.provide to the test.

Today with Effect.fn you can already do:

it.effect(
  "message",
  Effect.fn(function*(ctx) {
    ctx.expect(1).toBe(1)
  })
)

and piping would work like:

it.effect(
  "message",
  Effect.fn(
    function*(ctx) {
      ctx.expect(1).toBe(1)
    },
    Effect.timeout("1 second")
  )
)

if we were to support the generator directly we would need tons of overloads to support pipes

@mikearnaldi
Copy link
Member

I don't know if having the test context as a parameter of the generator function is possible, and if so that might just be the cleanest indeed, but otherwise we can just provide a service to fetch it too

having it is indeed possible, what I am more worried about is instead pipeability, many times you need to add stuff like Effect.provide to the test.

Today with Effect.fn you can already do:

it.effect(
  "message",
  Effect.fn(function*(ctx) {
    ctx.expect(1).toBe(1)
  })
)

and piping would work like:

it.effect(
  "message",
  Effect.fn(
    function*(ctx) {
      ctx.expect(1).toBe(1)
    },
    Effect.timeout("1 second")
  )
)

if we were to support the generator directly we would need tons of overloads to support pipes

And also we would not have a place where to specify test config like:

it.effect(
  "message",
  Effect.fn(
    function*(ctx) {
      ctx.expect(1).toBe(2)
    },
    Effect.timeout("1 second")
  ),
  { fails: true }
)

If this becomes:

it.effect(
  "message",
   function*(ctx) {
     ctx.expect(1).toBe(2)
   },
   Effect.timeout("1 second")
)

Where would we put { fails: true }?

@QuentinJanuel
Copy link
Contributor Author

Oh I didn't know about E.fn at all, this makes my suggestion feel way less interesting honestly
However it seems to me that E.fn first requires a name argument first and cannot be used like what you're describing. Am I missing something?

@QuentinJanuel
Copy link
Contributor Author

Oh alright I just learnt it's a new variant that got released today

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants