Skip to content

chore: add Darwin and Windows OS's to node test runner #6711

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

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

Conversation

jbronder
Copy link
Contributor

@jbronder jbronder commented Jun 4, 2025

Addresses #6636. This PR adds Windows and MacOS the matrix.os variable to expand the CI workflow of the Node test runner.

@jbronder jbronder requested a review from kt3k as a code owner June 4, 2025 22:46
Copy link

codecov bot commented Jun 4, 2025

Codecov Report

Attention: Patch coverage is 7.69231% with 12 lines in your changes missing coverage. Please review.

Project coverage is 94.62%. Comparing base (2001bae) to head (367b5c4).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
fs/_get_fs_flag.ts 7.69% 12 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6711      +/-   ##
==========================================
- Coverage   94.65%   94.62%   -0.03%     
==========================================
  Files         576      576              
  Lines       47059    47070      +11     
  Branches     6608     6608              
==========================================
- Hits        44543    44542       -1     
- Misses       2474     2485      +11     
- Partials       42       43       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@jbronder
Copy link
Contributor Author

jbronder commented Jun 5, 2025

Some of the @std/fs APIs are throwing when running tests with deno task test:node under the windows-latest. Should updates to those affected tests be addressed in separate, forthcoming PR(s)?

@kt3k
Copy link
Member

kt3k commented Jun 5, 2025

Should updates to those affected tests be addressed in separate, forthcoming PR(s)?

We generally don't merge PRs to main which makes CI check red. Can you try to address those issues in this PR?

@jbronder
Copy link
Contributor Author

jbronder commented Jun 5, 2025

That makes sense, I'll take a look.

@github-actions github-actions bot added the fs label Jun 5, 2025
@jbronder
Copy link
Contributor Author

Quick progress update: I decided to tackle writeFile(Sync) APIs. I'm addressing one API set at a time. I did end up making some modifications to the getWriteFsFlag function to handle Windows differently from MacOS and Linux. This is documented in the function in comments. I'm imagining that writeTextFile(Sync) would be similar.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants