Skip to content

Commit

Permalink
[MCHECKSTYLE-389] Partial revert of MCHECKSTYLE-365 severity change b…
Browse files Browse the repository at this point in the history
…ack to 'null' default

The severity setting even on original logic is incorrect.  It must remain null if it is to work properly at all.  While counts in fact are off, the entire section will be lost if it doesn't contain issues in at least 'error' due to the change when using default configurations.  This has to be reverted as not originally fixed properly so that the rules aggregate will display.  Example case has no 'error' type, only 'warning'.  Changing this to 'warning' will pick that section up.  Using 'info' or 'error' fails to do so.  I didn't bother with checking 'ignore' as that most likely is not good use either.  There is definitely a bug here as originally designed and more care needs taken to fix properly.
  • Loading branch information
hazendaz authored and eolivelli committed Feb 6, 2020
1 parent c427a30 commit de614e3
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 5 deletions.
8 changes: 6 additions & 2 deletions src/it/MCHECKSTYLE-365/verify.groovy
Original file line number Diff line number Diff line change
Expand Up @@ -32,14 +32,18 @@ assert htmlReportFile.exists();
*
* A more robust solution would be to parse the HTML, but that's
* a lot more effort than required to actually verify the behaviour.
*
* TODO: The original fix of MCHECKSTYLE-365 does not take into account when 'error'
* type doesn't exist and therefore breaks 'rules' aggregate reporting. As a result, counts
* set back to 3 and code is reverted. A better fix needs to be implemented.
*/

// Case with no custom messages
def numberOfOccurancesOfFileTabCharacter = htmlReportFile.text.count(">FileTabCharacter<")
assert 2 == numberOfOccurancesOfFileTabCharacter;
assert 3 == numberOfOccurancesOfFileTabCharacter;

// Case with custom messages
def numberOfOccurancesOfRegexpSingleline = htmlReportFile.text.count(">RegexpSingleline<");
assert 2 == numberOfOccurancesOfRegexpSingleline;
assert 3 == numberOfOccurancesOfRegexpSingleline;

return true;
Original file line number Diff line number Diff line change
Expand Up @@ -807,9 +807,10 @@ private void sortConfiguration( List<ConfReference> result, Configuration config
else
{
String fixedmessage = getConfigAttribute( childConfig, null, "message", null );
// Grab the severity from the rule configuration, use "error" as default value (as is
// done where the rule severity is displayed otherwise we miscount error violations)
String configSeverity = getConfigAttribute( childConfig, null, "severity", "error" );
// Grab the severity from the rule configuration. Do not set default value here as
// it breaks our rule aggregate section entirely. The counts are off but this is
// not appropriate fix location per MCHECKSTYLE-365.
String configSeverity = getConfigAttribute( childConfig, null, "severity", null );

// count rule violations
long violations = 0;
Expand Down

0 comments on commit de614e3

Please sign in to comment.