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
Ignore file size limits for CSS #4316
Conversation
...in/src/test/java/org/sonar/plugins/javascript/filter/JavaScriptExclusionsFileFilterTest.java
Show resolved
Hide resolved
@@ -180,6 +180,9 @@ void should_exclude_huge_files() { | |||
assertThat(filter.accept(inputFile("name.java", syntheticJavaFileContent(size, "Foo")))) | |||
.withFailMessage("Wrong result for size " + size) | |||
.isTrue(); | |||
|
|||
// we ignore size limits for CSS files | |||
assertThat(filter.accept(inputFile("name.css", syntheticJsFileContent(size)))).isTrue(); |
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 reused syntheticJsFileContent()
, because the file content doesn't matter here.
I would keep assessors agnostic of the language of the file. Line 61 in ac74f3d
We should create different set of assessors for CSS and JS/TS files, and simply not include size assessor in the CSS files. Keep in mind that the order of "assessing" matters -> we should do cheap exclusions based on file properties before touching the content of the file. |
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.
see comment above
SonarQube Quality Gate |
I was wondering if it is problematic that the filter scope is only covering CssLanguage here contrary to how the CSSRuleSensor filters it. |
IMO, it's okay like this. We don't need to filter |
Fixes #4234