-
Notifications
You must be signed in to change notification settings - Fork 143
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
SCAN4NET-158 Make file indexing more resilient #2261
Conversation
52607c8
to
37bd0d6
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. The exception logging is wrong, and therefore, I request changes. The other comments are nitpicks.
Tests/SonarScanner.MSBuild.Shim.Test/AdditionalFilesServiceTest.cs
Outdated
Show resolved
Hide resolved
Tests/SonarScanner.MSBuild.Shim.Test/AdditionalFilesServiceTest.cs
Outdated
Show resolved
Hide resolved
Tests/SonarScanner.MSBuild.Shim.Test/AdditionalFilesServiceTest.cs
Outdated
Show resolved
Hide resolved
Tests/SonarScanner.MSBuild.Shim.Test/AdditionalFilesServiceTest.cs
Outdated
Show resolved
Hide resolved
Tests/SonarScanner.MSBuild.Shim.Test/AdditionalFilesServiceTest.cs
Outdated
Show resolved
Hide resolved
37bd0d6
to
8fa3850
Compare
8fa3850
to
a40ee0f
Compare
ServerSettings = [new("sonar.typescript.file.suffixes", ".ts,.tsx")] | ||
}; | ||
|
||
var sut = new AdditionalFilesService(directoryWrapper, logger); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you don't need to create the directoryWrapper
and the sut
, you can re-use the wrapper
and sut
initialized in the constructor (called once every method).
It's going to look like this, which is a bit simpler. Same in the second test.
wrapper.EnumerateDirectories(ProjectBaseDir, "*", SearchOption.AllDirectories).Throws(_ => new DirectoryNotFoundException("Error message"));
var analysisConfig = new AnalysisConfig
{
ScanAllAnalysis = true,
LocalSettings = [],
ServerSettings = [new("sonar.typescript.file.suffixes", ".ts,.tsx")]
};
var files = sut.AdditionalFiles(analysisConfig, ProjectBaseDir);
files.Sources.Should().BeEmpty();
files.Tests.Should().BeEmpty();
wrapper.Received(1).EnumerateDirectories(ProjectBaseDir, "*", SearchOption.AllDirectories);
logger.DebugMessages[0].Should().Be($"Reading directories from: '{ProjectBaseDir}'.");
logger.DebugMessages[1].Should().MatchEquivalentOf(@"HResult: -2147024893, Exception: System.IO.DirectoryNotFoundException: Error message
at NSubstitute.ExceptionExtensions.ExceptionExtensions.<>c__DisplayClass2_0.<Throws>b__0(CallInfo ci) *");
logger.DebugMessages[2].Should().Be($"Reading files from: '{ProjectBaseDir}'.");
logger.DebugMessages[3].Should().Be($"Found 0 files in: '{ProjectBaseDir}'.");
logger.AssertSingleWarningExists($"Failed to get directories from: '{ProjectBaseDir}'.");
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, left only a small suggestion, not a blocker.
Martin is off for two days, I am taking over the PR
Quality Gate passedIssues Measures |
Part of SCAN4NET-158