Skip to content

Conversation

@sheremet-va
Copy link
Member

@sheremet-va sheremet-va commented Mar 28, 2025

Description

An idea I had while working on #7738. It felt weird that onTestSuiteReady/onTestSuiteResult are never called if test files doesn't have any suites. This PR makes it so the test file is also treated as a suite by the reporter. It means:

  • testCase.parent will always be a TestSuite
  • testSuite.parent is now TestSuite | undefined. If it's undefined, then the current suite is the same as the TestModule
  • onTestSuiteReady/onTestSuiteResult is always called

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:.

@sheremet-va sheremet-va requested a review from AriPerkkio March 28, 2025 13:54
@netlify
Copy link

netlify bot commented Mar 28, 2025

Deploy Preview for vitest-dev ready!

Name Link
🔨 Latest commit 9830fbe
🔍 Latest deploy log https://app.netlify.com/sites/vitest-dev/deploys/67e6a9d681dd34000884e339
😎 Deploy Preview https://deploy-preview-7754--vitest-dev.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link
Member

@AriPerkkio AriPerkkio left a comment

Choose a reason for hiding this comment

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

I'm not sure I see the reason for this change. During #7069 we discussed this quite much and decided that suite hooks are intentionally optional and only explicitly called when suites are used.

What challenges did you run into that would require this change?

@sheremet-va
Copy link
Member Author

sheremet-va commented Mar 31, 2025

What challenges did you run into that would require this change?

I found a few issues with our current approach:

  1. The need to duplicate logic for onTestModuleStart and onTestSuiteReady if you want to work with tests in a single suite
  2. It's hard to notice why your code doesn't work when you just iterate over tests in a suite even for a person who designed the API (me) - the verbose reporter previously would just iterate over all suites (which includes "file" in the old API)
  3. The word "suite" in testing context can also mean the test file when talking about testing broadly, so I think it would make sense to separate the "test module" and "test suite"

@sheremet-va
Copy link
Member Author

Let's keep the API stable for now.

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