SONARXML-218 Remove queries to non-persisted metrics (do not squash but rebase)#326
Conversation
dorian-burihabwa-sonarsource
left a comment
dorian-burihabwa-sonarsource
left a comment
There was a problem hiding this comment.
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.
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.
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.
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.
alban-auzeill
force-pushed
the
alban/SONARXML-218
branch
from
44828a9 to
53cf5db
Compare
|
alban-auzeill
changed the title
dorian-burihabwa-sonarsource
left a comment
dorian-burihabwa-sonarsource
left a comment
There was a problem hiding this comment.
LGTM 👍🏿 Thank you for spending some time on the topic
2 participants
SONARXML-218