Restore broken AutoCRLF tests #8041
Conversation
Groupped members by type: properties, public, protected, private, event handlers. No other changes of any kind
ghost
assigned 
|
OT: @gerhardol I noticed a peculiar behaviour that is quite confusing. I think you worked in this area not that long ago. Before committing files I have two different representations of files:
When files are committed and I look at the diff all "binary" files are shown similar to this: I can see how this can be confusing to users. Is this something we track somewhere, or I should raise an new issue for? |
Codecov Report
@@ Coverage Diff @@
## master #8041 +/- ##
==========================================
- Coverage 49.60% 49.58% -0.03%
==========================================
Files 844 846 +2
Lines 60849 60884 +35
Branches 10877 10899 +22
==========================================
+ Hits 30185 30189 +4
- Misses 28448 28480 +32
+ Partials 2216 2215 -1
|
There was a problem hiding this comment.
For those intimidated by the number of lines in this PR: See last commit and FileViewer.cs and review from there
I can see how this can be confusing to users. Is this something we track somewhere, or I should raise an new issue for?
Git do not show diff for files set to binary, why I suggest other attributes for these files. (That can be done in other ways too, but keeping lines down as we are not likely to merge these anyway).
For files where the line ending indicates that the file is a binary file, GE shows binary "diffs" the same as new.
GE may check Git attributes too for new files (not in all situations though...) why the binary status is detected for the new file.
I would propose to not change this behavior for diffs, it is an extra Git command that with viruses like Symantec could add 100 ms.
Sure we could add an issue, but I do not think that will help anyone.
Certainly not changing here. Just an observation at this stage. |
Resolves #8000 The tests got broken for few reasons: 1. Earlier versions of ApprovalTests considered differences in EOL, and these tests deal with precisely this. However recently we have updated to the newer version of ApprovalTests that no longer considered EOL differences as failures. 2. During migration to Arcade SDK (#7649) few approved files had their EOL accidentally converted to CRLF, that was unnoticed due to the reasons explained above. CRLF conversion is moved out of `FileViewer` because it has no real connection to it. The tests refactored and verified for correctness.
|
Hello @RussKie! Because this pull request has the p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (
|
ghost
merged commit 
Resolves #8000
Proposed changes
Restore broken AutoCRLF tests
The tests got broken for few reasons:
Earlier versions of ApprovalTests considered differences in EOL, and these tests deal with precisely this.
However recently we have updated to the newer version of ApprovalTests that no longer considered EOL differences as failures.
During migration to Arcade SDK (Use Microsoft Arcade to improve build pipelines #7649) few approved files had their EOL accidentally converted to CRLF, that was unnoticed due to the reasons explained above.
CRLF conversion is moved out of
FileViewerbecause it has no real connection to it. The tests refactored and verified for correctness.Move
FileViewermembers to give some logical orderRemove unused
FileViewermembersBest review commit-by-commit.
❗ Do not squash!