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

feat(runner): provide test context to test.each #4989

Open
wants to merge 27 commits into
base: main
Choose a base branch
from

Conversation

hi-ogawa
Copy link
Contributor

@hi-ogawa hi-ogawa commented Jan 17, 2024

Description

I made a few prototypes here in this PR based on the suggestions in the issue.

todo

  • finalize api (Now this PR includes only each7)
    • last argument? issuecomment-1871372078
      • see test.each5 and test.each7
      • currently fixture extraction looks for first argument, so it needs to adjust getUsedProps slightly, but it should be simple.
      • the usage of spreading (...args, { expect }) => ... is not possible, but overall, this feels least breaking and least surprising in terms of API.
    • first argument? issuecomment-1859974766
      • see test.each6
      • based on context's usage, the user needs to switch between ({ expect }, arg) => ... and (arg) => ..., which might be un-intuitive than having it in the last argument. Also, if Vitest would change the default to be { context: true } in the future, then all users (regardless of context/fixture usage) would be impacted by breaking change. Also this would break Jest compatibility.
    • inside test context object? issuecomment-1894155135
      • see test.each3
      • the downside would be similar to "first argument" explained above. But if ever, breaking change is possible, then this API feels best with Vitest's context concept.
    • where to boolean flag?
      • test.each([...])("title", (...) => { ... }, { context: true }) issuecomment-1894155135
        • see test.each7
      • test.each([...], { context: true }) issuecomment-1859974766
        • see test.each5
        • it cannot support string literal usage test.each`${0} | ${1}`
  • ts function overload
  • support TestOption.context at the 2nd position feat(vitest): "test" accepts options object as the second parameter #5142
  • test + type testing
  • docs

Please don't delete this checklist! Before submitting the PR, please make sure you do the following:

  • It's really useful if your PR references an issue where it is discussed ahead of time. If the feature is substantial or introduces breaking changes without a discussion, PR might be closed.
  • Ideally, include a test that fails without this PR but passes with it.
  • Please, don't make changes to pnpm-lock.yaml unless you introduce a new test example.

Tests

  • Run the tests with pnpm test:ci.

Documentation

  • If you introduce new functionality, document it. You can run documentation with pnpm run docs command.

Changesets

  • Changes in changelog are generated from PR name. Please, make sure that it explains your changes in an understandable manner. Please, prefix changeset messages with feat:, fix:, perf:, docs:, or chore:.

Copy link

netlify bot commented Jan 17, 2024

Deploy Preview for fastidious-cascaron-4ded94 canceled.

Name Link
🔨 Latest commit c6eae14
🔍 Latest deploy log https://app.netlify.com/sites/fastidious-cascaron-4ded94/deploys/65cac8aa1e10050008ef26ea

@hi-ogawa hi-ogawa marked this pull request as ready for review January 18, 2024 03:19
@hi-ogawa hi-ogawa marked this pull request as draft January 19, 2024 00:46
@@ -192,7 +212,6 @@ export interface TestOptions {
}

interface ExtendedAPI<ExtraContext> {
each: TestEachFunction
Copy link
Contributor Author

Choose a reason for hiding this comment

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

each in ExtendedAPI (here) and SuiteAPI (below) seems to be redundant.

@hi-ogawa hi-ogawa marked this pull request as ready for review February 12, 2024 10:33
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.

Provide test context to test.each()
3 participants