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

Audit all tests that require WindowsOnlyFact or WindowsOnlyTheory and make them work on linux #4586

Open
jmarolf opened this issue Dec 17, 2020 · 2 comments
Labels
help wanted The issue is up-for-grabs, and can be claimed by commenting Test
Milestone

Comments

@jmarolf
Copy link
Contributor

jmarolf commented Dec 17, 2020

the WindowsOnlyFact and WindowsOnlyTheory attributes was applied to several tests in #4444 to unblock merging. We should audit the remaining tests and figure out how to enable them again on linux.

@jmarolf jmarolf changed the title Audit all tests that require WindowsOnlyFact and make them work on linux Audit all tests that require WindowsOnlyFact or `WindowsOnlyTheory and make them work on linux Dec 17, 2020
@jmarolf jmarolf changed the title Audit all tests that require WindowsOnlyFact or `WindowsOnlyTheory and make them work on linux Audit all tests that require WindowsOnlyFact or WindowsOnlyTheory and make them work on linux Dec 17, 2020
mavasani added a commit to mavasani/roslyn-analyzers that referenced this issue Dec 18, 2020
These tests are failing intermittently on newly added Unix queues.
dotnet#4586 tracks fixing and enabling these tests on these queues.
@mavasani mavasani added help wanted The issue is up-for-grabs, and can be claimed by commenting Test labels Dec 18, 2020
@mavasani mavasani added this to the Unknown milestone Dec 18, 2020
@Youssef1313
Copy link
Member

Youssef1313 commented Feb 5, 2021

The failing test for ReleaseTrackingAnalyzer seems like a product bug (not a critical one though).

The fix isn't using consistent line endings. The following constants are using '\r\n':

internal const string ShippedAnalyzerReleaseTrackingFileDefaultContent = @"; Shipped analyzer releases
" + CommonAnalyzerReleaseTrackingContent;
internal const string UnshippedAnalyzerReleaseTrackingFileDefaultContent = @"; Unshipped analyzer release
" + CommonAnalyzerReleaseTrackingContent;
private const string CommonAnalyzerReleaseTrackingContent = @"; https://github.com/dotnet/roslyn-analyzers/blob/master/src/Microsoft.CodeAnalysis.Analyzers/ReleaseTrackingAnalyzers.Help.md

But then, AddOrUpdateEntriesToUnshippedFileAsync uses StringBuilder.AppendLine() which internally uses Environment.NewLine. This might need a fix similar to #4774.

Note: There are "hacky" ways to make tests path without changing in the fixer itself. This shouldn't be done claiming to fix the issue.

@Youssef1313
Copy link
Member

Interestingly, release tracking test doesn't actually fail on Linux builds. I was expecting them to fail due to my previous reasoning 😕.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted The issue is up-for-grabs, and can be claimed by commenting Test
Projects
None yet
Development

No branches or pull requests

3 participants