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

test: use default import for fs-extra #11543

Merged

Conversation

sapphi-red
Copy link
Member

Description

This PR fixes this CI fail.
https://github.com/vitejs/vite/actions/runs/3811060325/jobs/6483511814
This fail is caused by vitest-dev/vitest#2512, a breaking change in vitest 0.26.0. (Because we now use vitest 0.25.x the test is passing.)
This test was relying on an invalid named exports. This PR changes that part to use the default export.

#11419 seems not to work even with this PR. I guess it's microsoft/playwright#19661.

Additional context


What is the purpose of this pull request?

  • Bug fix
  • New Feature
  • Documentation update
  • Other

Before submitting the PR, please make sure you do the following

  • Read the Contributing Guidelines.
  • Read the Pull Request Guidelines and follow the PR Title Convention.
  • Check that there isn't already a PR that solves the problem the same way to avoid creating a duplicate.
  • Provide a description in this PR that addresses what the PR is solving, or reference the issue that it solves (e.g. fixes #123).
  • Ideally, include relevant tests that fail without this PR but pass with it.

@sapphi-red sapphi-red added the p1-chore Doesn't change code behavior (priority) label Dec 31, 2022
@sapphi-red sapphi-red enabled auto-merge (squash) December 31, 2022 17:38
bluwy
bluwy previously approved these changes Jan 1, 2023
@sapphi-red
Copy link
Member Author

Ah, no, github actions is working correctly.

The "Build&Test" jobs are not running because #11294 splitted the CI into two jobs.
Maybe we need to change the required jobs or make "Build&Test" job itself run but skip the actual tests running.

ArnaudBarre
ArnaudBarre previously approved these changes Jan 1, 2023
@ArnaudBarre
Copy link
Member

ArnaudBarre commented Jan 1, 2023

Oh sorry didn't account for required jobs when splitting the CI.

The doc for this case is here but that's so bad...

I only see two others solutions: removing require check or removing path filtering (which is useful for this kind of small PR)

Edit: Third solution: #11548

@sapphi-red sapphi-red dismissed stale reviews from ArnaudBarre and bluwy via 6d1b721 January 4, 2023 08:20
@sapphi-red sapphi-red force-pushed the test/use-default-import-for-fs-extra branch from 0dac13a to 6d1b721 Compare January 4, 2023 08:20
@sapphi-red sapphi-red merged commit d3bed53 into vitejs:main Jan 4, 2023
@sapphi-red sapphi-red deleted the test/use-default-import-for-fs-extra branch January 4, 2023 11:01
futurGH pushed a commit to futurGH/vite that referenced this pull request Feb 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
p1-chore Doesn't change code behavior (priority)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants