-
Notifications
You must be signed in to change notification settings - Fork 226
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
SQ Plugin: Should align logging for not indexed files #9571
Conversation
@andrei-epure-sonarsource I assigned you as a secondary reviewer. Please confirm that the PR solves the issue. |
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
...dotnet-shared-library/src/main/java/org/sonar/plugins/dotnet/tests/DotCoverReportParser.java
Outdated
Show resolved
Hide resolved
...r-dotnet-shared-library/src/main/java/org/sonar/plugins/dotnet/tests/ScannerFileService.java
Outdated
Show resolved
Hide resolved
assertThat(logTester.logs(Level.DEBUG).get(0)).isEqualTo("Did not find deterministic source path in 'C:\\_\\some\\path\\file.cs'." + | ||
" Will skip this coverage entry. Verify sonar.sources in .sonarqube\\out\\sonar-project.properties."); | ||
assertThat(logTester.logs(Level.DEBUG).get(0)).isEqualTo( | ||
"The file 'C:\\_\\some\\path\\file.cs' is not indexed or does not have the supported language." |
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.
this can fail either because there is a mismatch between the deterministic source path (due to some tooling issue) or because the file hasn't been indexed.
how can we make both possible cases clear? is it worth it? what would it help if you were the developer investigating such a case?
Did you look at the integration tests we have for deterministic code coverage, to get more context?
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.
I refactored it a bit to improve the situation. I hope the messages make sense now.
5e8e9f0
to
225ea37
Compare
225ea37
to
60d5774
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.
LGTM! Left one remark.
...tnet-shared-library/src/test/java/org/sonar/plugins/dotnet/tests/ScannerFileServiceTest.java
Outdated
Show resolved
Hide resolved
f9968c3
to
70cf878
Compare
Quality Gate passed for 'Sonar .NET Java Plugin'Issues Measures |
Quality Gate passed for 'SonarAnalyzer for .NET'Issues Measures |
Fixes #8503