test: stop requiring core.autocrlf=true on windows - #63102
Conversation
|
Review requested:
|
|
Note that we explicitly request it to be set to Lines 11 to 12 in f694c3a Can you split this into two PR so this can be reviewed and backported separately? |
867cffd to
a0a5162
Compare
I opened #63118 for the other test. |
|
This pull request has been marked as stale due to 90 days of inactivity. |
|
This is still relevant. Waiting for review. |
|
I think in order to land this, we would need some kind of linter or at least make sure some CI would fail if some test makes incorrect assumption about the |
0a4578f to
3066cdd
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #63102 +/- ##
==========================================
- Coverage 90.30% 90.30% -0.01%
==========================================
Files 759 759
Lines 247629 247629
Branches 46677 46692 +15
==========================================
- Hits 223628 223619 -9
+ Misses 15470 15461 -9
- Partials 8531 8549 +18 🚀 New features to boost your workflow:
|
|
With autocrlf: Without autocrlf: But the issue with the coverage job is that it doesn't report test failures, so we would likely still miss regressions |
|
@aduh95 Oh, you're right. Sorry about that. [EDIT]: |
9d7e7a7 to
d4903ca
Compare
Signed-off-by: Nad Alaba <37968805+nadalaba@users.noreply.github.com>
d4903ca to
5d03015
Compare
Some tests on Windows (
test\parallel\test-snapshot-typescript) require the repo to be checked-out with the git configcore.autocrlfset totrue.However, a lot of users prefer to use modern editors that can handle LF and keep core.autocrlf set to false to minimize headaches.
This PR fixes the tests for them without breaking it for others.