-
Notifications
You must be signed in to change notification settings - Fork 41
SONARXML-218 Remove queries to non-persisted metrics (do not squash but rebase) #326
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
Conversation
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.
It looks good overall but let's keep the change to the min JDK into a separate PR
| } | ||
|
|
||
| private void verifyMetrics(String filename, int commentLinesNumber, int... linesOfCode) throws IOException { | ||
| private void verifyMetrics(String filename, int commentLinesNumber, int ncloc, String linesMetrics) throws IOException { |
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.
In the tests above, it looks like the only metric we check is ncloc_data. But we also pass ncloc as argument, which also always matches the number of entries under ncloc_data here. Should we split the test with one checking the metrics using the JSON and a second one checking the measures stored in the sensor storage (ncloc and comment_lines)?
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.
Ideally we should be able to pass the sensor storage to the FilesLinesContextTester so that everything is stored in the same place but splitting the test method in two might be enough for now
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.
For now, I only removed int ncloc and compute it from the FilesLinesContextTester storage using int expectedNcloc = metrics.get(CoreMetrics.NCLOC_DATA_KEY).size();. And because I had to split in different commits, sorry, the latest changes are already merged into the second commits.
44828a9 to
53cf5db
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 👍🏿 Thank you for spending some time on the topic





SONARXML-218