Skip to content

Conversation

@illusionalsagacity
Copy link
Contributor

These have the test context passed as an argument, and preserves the
array's tuple to be passed in as the first argument. This removes the
need for the -N suffix bindings and allows use of the non-BuiltIn
bindings

@illusionalsagacity
Copy link
Contributor Author

@cometkim Would you have some time soon to review this? Unfortunately test.each does not provide the test context so you have to open Vitest.Bindings.BuiltIn today if you use those style of tests.

@cometkim
Copy link
Owner

Looks great.

I don't think we need to keep both each and for, so maybe I will remove the Each module in the next major.

)
}

module type ForType = {
Copy link
Owner

Choose a reason for hiding this comment

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

Can you add describe sets too?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure!

It looks like it was introduced in vitest v3: vitest-dev/vitest#7253, should I add it with a warning that it's not compatible with v2?

Copy link
Owner

@cometkim cometkim May 23, 2025

Choose a reason for hiding this comment

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

Maybe I should bump vitest version in the next release.

It doesn't have breaking changes that could affect rescript-vitest users? Let me check peerDeps and histories first.

Copy link
Owner

Choose a reason for hiding this comment

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

https://vitest.dev/guide/migration#vitest-3

There are some breaking changes, but mostly I can handle

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, when you go to bump it for devDependencies I forgot to push these tests:

For.describe(sumObj, "sum $a+$b=$sum", (i, t) => {
  expect(t, i["a"] + i["b"])->Expect.toBe(i["sum"])
  expect(t, i["a"] + i["b"] + 1)->Expect.not->Expect.toBe(i["sum"])
})

For.describe(sum2, "%i=%s", ((a, b), t) => {
  expect(t, a->Js.Int.toString)->Expect.toBe(b)
  expect(t, (a + 1)->Js.Int.toString)->Expect.not->Expect.toBe(b)
})

For.describe(sum3, "sum %i+%i=%s", ((a, b, sum), t) => {
  expect(t, (a + b)->Js.Int.toString)->Expect.toBe(sum)
  expect(t, (a + b + 1)->Js.Int.toString)->Expect.not->Expect.toBe(sum)
})

For.describe(sum4, "sum %i+%i+%i=%s", ((a, b, c, sum), t) => {
  expect(t, (a + b + c)->Js.Int.toString)->Expect.toBe(sum)
  expect(t, (a + b + c + 1)->Js.Int.toString)->Expect.not->Expect.toBe(sum)
})

For.describe(sum5, "sum %i+%i+%i+%i=%s", ((a, b, c, d, sum), t) => {
  expect(t, (a + b + c + d)->Js.Int.toString)->Expect.toBe(sum)
  expect(t, (a + b + c + d + 1)->Js.Int.toString)->Expect.not->Expect.toBe(sum)
})

For.describeAsync(sumObj, "sum $a+$b=$sum", async (i, t) => {
  expect(t, i["a"] + i["b"])->Expect.toBe(i["sum"])
  expect(t, i["a"] + i["b"] + 1)->Expect.not->Expect.toBe(i["sum"])
})

For.describeAsync(sum2, "%i=%s", async ((a, b), t) => {
  expect(t, a->Js.Int.toString)->Expect.toBe(b)
  expect(t, (a + 1)->Js.Int.toString)->Expect.not->Expect.toBe(b)
})

For.describeAsync(sum3, "sum %i+%i=%s", async ((a, b, sum), t) => {
  expect(t, (a + b)->Js.Int.toString)->Expect.toBe(sum)
  expect(t, (a + b + 1)->Js.Int.toString)->Expect.not->Expect.toBe(sum)
})

For.describeAsync(sum4, "sum %i+%i+%i=%s", async ((a, b, c, sum), t) => {
  expect(t, (a + b + c)->Js.Int.toString)->Expect.toBe(sum)
  expect(t, (a + b + c + 1)->Js.Int.toString)->Expect.not->Expect.toBe(sum)
})

For.describeAsync(sum5, "sum %i+%i+%i+%i=%s", async ((a, b, c, d, sum), t) => {
  expect(t, (a + b + c + d)->Js.Int.toString)->Expect.toBe(sum)
  expect(t, (a + b + c + d + 1)->Js.Int.toString)->Expect.not->Expect.toBe(sum)
})

Copy link
Owner

Choose a reason for hiding this comment

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

btw, descritbe has no test context

These have the test context passed as an argument, and preserves the
array's tuple to be passed in as the first argument. This removes the
need for the -N suffix bindings and allows use of the non-BuiltIn
bindings
@illusionalsagacity illusionalsagacity force-pushed the add-for-test-context-bindings branch from 752ed1b to 3e4012a Compare May 23, 2025 19:51
@illusionalsagacity illusionalsagacity changed the title Adds test.for and it.for bindings Adds test.for, it.for, and describe.for bindings May 23, 2025
@cometkim cometkim merged commit 7ddcfeb into cometkim:main May 23, 2025
2 checks passed
@illusionalsagacity illusionalsagacity deleted the add-for-test-context-bindings branch May 23, 2025 20:05
cometkim added a commit that referenced this pull request May 24, 2025
introduced by #28

they shouldn't pass textCtx
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