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

fix: apply retry and bail from test config file #4530

Merged
merged 3 commits into from
Nov 18, 2023

Conversation

hi-ogawa
Copy link
Contributor

@hi-ogawa hi-ogawa commented Nov 18, 2023

Description

Closes #4490

I found that retry and bail options from config file didn't have any effect since cac's default 0 value is preventing from user test config to overwrite the value.
I saw other option (e.g. --test-timeout introduced in #3019) is using "... (default: ...)" in help message instead of relying on { default: ... }, so this PR does similarly for retry and bail.

When retry config is introduced in #3603, the corresponding test is added as test/config/fixtures/retry which makes use of test.retry config, but this was not testing cli scenario since this test is executed via startVitest.

So, to verify this PR, I modified this existing test to use runVitestCli.

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

  • (N/A) 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 Nov 18, 2023

Deploy Preview for fastidious-cascaron-4ded94 canceled.

Name Link
🔨 Latest commit 654d825
🔍 Latest deploy log https://app.netlify.com/sites/fastidious-cascaron-4ded94/deploys/6558628ef8d5f000084bdad9

Comment on lines 127 to 137
function debug(obj: object) {
return Object.fromEntries(Object.entries(obj).filter(([k, _v]) => ['retry', 'bail', 'segfaultRetry'].includes(k)))
}
console.log('== configResolved (before)', [debug(options), debug(viteConfigTest)])
options = deepMerge(
{},
configDefaults,
viteConfigTest,
options,
)
console.log('== configResolved (after)', [debug(options)])
Copy link
Contributor Author

@hi-ogawa hi-ogawa Nov 18, 2023

Choose a reason for hiding this comment

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

When using cac's default value, this debug logging looks like this:

== configResolved (before) [ { segfaultRetry: 0, bail: 0, retry: 0 }, { retry: 3, bail: 4 } ]
== configResolved (after) [ { retry: 0, bail: 0, segfaultRetry: 0 } ]

It shows viteConfigTest has no effect for bail and retry since options from cli has precedence over viteConfigTest.

@hi-ogawa hi-ogawa changed the title fix: fix test config for retry and bail fix: apply retry and bail test config Nov 18, 2023
@hi-ogawa hi-ogawa changed the title fix: apply retry and bail test config fix: apply retry and bail from test config file Nov 18, 2023
@hi-ogawa hi-ogawa marked this pull request as ready for review November 18, 2023 07:17
@sheremet-va sheremet-va merged commit 94f9a3c into vitest-dev:main Nov 18, 2023
13 of 16 checks passed
@hi-ogawa hi-ogawa deleted the fix-global-retry-config branch November 18, 2023 08:37
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.

retry does not seem to work in global settings
2 participants