[PATCH 2/4] coverage/reports: Improve formatting and clarity
Chris Johns
chrisj at rtems.org
Mon Mar 15 01:06:52 UTC 2021
On 13/3/21 3:48 am, Alex White wrote:
> The coverage reports contain places where they display incorrect or
> vague information particularly when some statistic is unavailable. This
> has been fixed. The formatting and wording of various things has been
> improved as well.
> ---
> tester/covoar/ObjdumpProcessor.cc | 5 +
> tester/covoar/ReportsBase.cc | 22 ++++-
> tester/covoar/ReportsHtml.cc | 159 +++++++++++++++++-------------
> tester/covoar/ReportsText.cc | 76 ++++++++------
> tester/rt/coverage.py | 19 +++-
> 5 files changed, 174 insertions(+), 107 deletions(-)
>
> diff --git a/tester/covoar/ObjdumpProcessor.cc b/tester/covoar/ObjdumpProcessor.cc
> index a71b00d..9ef2390 100644
> --- a/tester/covoar/ObjdumpProcessor.cc
> +++ b/tester/covoar/ObjdumpProcessor.cc
> @@ -377,6 +377,11 @@ namespace Coverage {
> break;
> }
>
> + // Remove any extra line break
> + if (line.back() == '\n') {
> + line.erase(line.end() - 1);
> + }
> +
> lineInfo.line = line;
> lineInfo.address = 0xffffffff;
> lineInfo.isInstruction = false;
> diff --git a/tester/covoar/ReportsBase.cc b/tester/covoar/ReportsBase.cc
> index 0be5567..8252959 100644
> --- a/tester/covoar/ReportsBase.cc
> +++ b/tester/covoar/ReportsBase.cc
> @@ -161,6 +161,24 @@ void ReportsBase::CloseSymbolSummaryFile(
> CloseFile( aFile );
> }
>
> +void remove_tabs(const char* in, char* out, size_t max_len) {
Please change this interface and pass in a std::string reference. Playing with
the internal data of a string is probality not a good idea. I suggest you use
C++ and avoid falling back to C in these sorts of cases.
> + size_t i = 0;
> +
> + while (*in != 0 && i < max_len - 1) {
> + if (*in == '\t') {
> + in++;
> + out[i++] = ' ';
> + while (i % 4 != 0 && i < max_len - 1) {
> + out[i++] = ' ';
> + }
> + } else {
> + out[i++] = *in++;
> + }
> + }
> +
> + out[i] = 0;
> +}
> +
> /*
> * Write annotated report
> */
> @@ -237,7 +255,9 @@ void ReportsBase::WriteAnnotatedReport(
> }
> }
>
> - snprintf( textLine, LINE_LENGTH, "%-70s", itr->line.c_str() );
> + char textLineWithoutTabs[LINE_LENGTH];
> + remove_tabs( itr->line.c_str(), textLineWithoutTabs, LINE_LENGTH );
As called from here.
Chris
> + snprintf( textLine, LINE_LENGTH, "%-90s", textLineWithoutTabs );
> line = textLine + annotation;
>
> PutAnnotatedLine( aFile, state, line, id);
> diff --git a/tester/covoar/ReportsHtml.cc b/tester/covoar/ReportsHtml.cc
> index ebc6ee0..cce0a4f 100644
> --- a/tester/covoar/ReportsHtml.cc
> +++ b/tester/covoar/ReportsHtml.cc
> @@ -89,7 +89,7 @@ namespace Coverage {
> PRINT_ITEM( "Branch Report", "branch" );
> PRINT_ITEM( "Annotated Assembly", "annotated" );
> PRINT_ITEM( "Symbol Summary", "symbolSummary" );
> - PRINT_ITEM( "Size Report", "sizes" );
> + PRINT_ITEM( "Uncovered Range Size Report", "sizes" );
>
> PRINT_TEXT_ITEM( "Explanations Not Found", "ExplanationsNotFound.txt" );
>
> @@ -176,7 +176,7 @@ namespace Coverage {
> // Put header information into the file
> fprintf(
> aFile,
> - "<title>Branch Report</title\n"
> + "<title>Branch Report</title>\n"
> "<div class=\"heading-title\">"
> );
>
> @@ -321,7 +321,7 @@ namespace Coverage {
> // Put header information into the file
> fprintf(
> aFile,
> - "<title>Size Report</title>\n"
> + "<title>Uncovered Range Size Report</title>\n"
> "<div class=\"heading-title\">"
> );
>
> @@ -334,7 +334,7 @@ namespace Coverage {
>
> fprintf(
> aFile,
> - "Size Report</div>\n"
> + "Uncovered Range Size Report</div>\n"
> "<div class =\"datetime\">%s</div>\n"
> "<body>\n"
> "<table class=\"covoar table-autosort:0 table-autofilter table-stripeclass:covoar-tr-odd"
> @@ -490,7 +490,7 @@ namespace Coverage {
> FILE* report
> )
> {
> - if ( BranchInfoAvailable )
> + if (BranchInfoAvailable && SymbolsToAnalyze->getNumberBranchesFound() != 0)
> fprintf( report, "All branch paths taken.\n" );
> else
> fprintf( report, "No branch information found.\n" );
> @@ -861,90 +861,107 @@ namespace Coverage {
> symbol->first.c_str()
> );
>
> - // Total Size in Bytes
> - fprintf(
> - report,
> - "<td class=\"covoar-td\" align=\"center\">%d</td>\n",
> - symbol->second.stats.sizeInBytes
> - );
> -
> - // Total Size in Instructions
> - fprintf(
> - report,
> - "<td class=\"covoar-td\" align=\"center\">%d</td>\n",
> - symbol->second.stats.sizeInInstructions
> - );
> -
> - // Total Uncovered Ranges
> - fprintf(
> - report,
> - "<td class=\"covoar-td\" align=\"center\">%d</td>\n",
> - symbol->second.stats.uncoveredRanges
> - );
> -
> - // Uncovered Size in Bytes
> - fprintf(
> - report,
> - "<td class=\"covoar-td\" align=\"center\">%d</td>\n",
> - symbol->second.stats.uncoveredBytes
> - );
> -
> - // Uncovered Size in Instructions
> - fprintf(
> - report,
> - "<td class=\"covoar-td\" align=\"center\">%d</td>\n",
> - symbol->second.stats.uncoveredInstructions
> - );
> + if (symbol->second.stats.sizeInBytes == 0) {
> + // The symbol has never been seen. Write "unknown" for all columns.
> + fprintf(
> + report,
> + "<td class=\"covoar-td\" align=\"center\">unknown</td>\n"
> + "<td class=\"covoar-td\" align=\"center\">unknown</td>\n"
> + "<td class=\"covoar-td\" align=\"center\">unknown</td>\n"
> + "<td class=\"covoar-td\" align=\"center\">unknown</td>\n"
> + "<td class=\"covoar-td\" align=\"center\">unknown</td>\n"
> + "<td class=\"covoar-td\" align=\"center\">unknown</td>\n"
> + "<td class=\"covoar-td\" align=\"center\">unknown</td>\n"
> + "<td class=\"covoar-td\" align=\"center\">unknown</td>\n"
> + "<td class=\"covoar-td\" align=\"center\">unknown</td>\n"
> + "<td class=\"covoar-td\" align=\"center\">unknown</td>\n"
> + );
> + } else {
> + // Total Size in Bytes
> + fprintf(
> + report,
> + "<td class=\"covoar-td\" align=\"center\">%d</td>\n",
> + symbol->second.stats.sizeInBytes
> + );
>
> - // Total number of branches
> - fprintf(
> - report,
> - "<td class=\"covoar-td\" align=\"center\">%d</td>\n",
> - symbol->second.stats.branchesNotExecuted + symbol->second.stats.branchesExecuted
> - );
> + // Total Size in Instructions
> + fprintf(
> + report,
> + "<td class=\"covoar-td\" align=\"center\">%d</td>\n",
> + symbol->second.stats.sizeInInstructions
> + );
>
> - // Total Always Taken
> - fprintf(
> - report,
> - "<td class=\"covoar-td\" align=\"center\">%d</td>\n",
> - symbol->second.stats.branchesAlwaysTaken
> - );
> + // Total Uncovered Ranges
> + fprintf(
> + report,
> + "<td class=\"covoar-td\" align=\"center\">%d</td>\n",
> + symbol->second.stats.uncoveredRanges
> + );
>
> - // Total Never Taken
> - fprintf(
> - report,
> - "<td class=\"covoar-td\" align=\"center\">%d</td>\n",
> - symbol->second.stats.branchesNeverTaken
> - );
> + // Uncovered Size in Bytes
> + fprintf(
> + report,
> + "<td class=\"covoar-td\" align=\"center\">%d</td>\n",
> + symbol->second.stats.uncoveredBytes
> + );
>
> - // % Uncovered Instructions
> - if ( symbol->second.stats.sizeInInstructions == 0 )
> + // Uncovered Size in Instructions
> fprintf(
> report,
> - "<td class=\"covoar-td\" align=\"center\">100.00</td>\n"
> + "<td class=\"covoar-td\" align=\"center\">%d</td>\n",
> + symbol->second.stats.uncoveredInstructions
> );
> - else
> +
> + // Total number of branches
> fprintf(
> report,
> - "<td class=\"covoar-td\" align=\"center\">%.2f</td>\n",
> - (symbol->second.stats.uncoveredInstructions*100.0)/
> - symbol->second.stats.sizeInInstructions
> + "<td class=\"covoar-td\" align=\"center\">%d</td>\n",
> + symbol->second.stats.branchesNotExecuted + symbol->second.stats.branchesExecuted
> );
>
> - // % Uncovered Bytes
> - if ( symbol->second.stats.sizeInBytes == 0 )
> + // Total Always Taken
> fprintf(
> report,
> - "<td class=\"covoar-td\" align=\"center\">100.00</td>\n"
> + "<td class=\"covoar-td\" align=\"center\">%d</td>\n",
> + symbol->second.stats.branchesAlwaysTaken
> );
> - else
> +
> + // Total Never Taken
> fprintf(
> report,
> - "<td class=\"covoar-td\" align=\"center\">%.2f</td>\n",
> - (symbol->second.stats.uncoveredBytes*100.0)/
> - symbol->second.stats.sizeInBytes
> + "<td class=\"covoar-td\" align=\"center\">%d</td>\n",
> + symbol->second.stats.branchesNeverTaken
> );
>
> + // % Uncovered Instructions
> + if ( symbol->second.stats.sizeInInstructions == 0 )
> + fprintf(
> + report,
> + "<td class=\"covoar-td\" align=\"center\">100.00</td>\n"
> + );
> + else
> + fprintf(
> + report,
> + "<td class=\"covoar-td\" align=\"center\">%.2f</td>\n",
> + (symbol->second.stats.uncoveredInstructions*100.0)/
> + symbol->second.stats.sizeInInstructions
> + );
> +
> + // % Uncovered Bytes
> + if ( symbol->second.stats.sizeInBytes == 0 )
> + fprintf(
> + report,
> + "<td class=\"covoar-td\" align=\"center\">100.00</td>\n"
> + );
> + else
> + fprintf(
> + report,
> + "<td class=\"covoar-td\" align=\"center\">%.2f</td>\n",
> + (symbol->second.stats.uncoveredBytes*100.0)/
> + symbol->second.stats.sizeInBytes
> + );
> + }
> +
> fprintf( report, "</tr>\n");
> return true;
> }
> diff --git a/tester/covoar/ReportsText.cc b/tester/covoar/ReportsText.cc
> index 33d3509..a3923e6 100644
> --- a/tester/covoar/ReportsText.cc
> +++ b/tester/covoar/ReportsText.cc
> @@ -52,7 +52,7 @@ bool ReportsText::PutNoBranchInfo(
> FILE* report
> )
> {
> - if ( BranchInfoAvailable )
> + if ( BranchInfoAvailable && SymbolsToAnalyze->getNumberBranchesFound() != 0 )
> fprintf( report, "All branch paths taken.\n" );
> else
> fprintf( report, "No branch information found.\n" );
> @@ -235,38 +235,52 @@ bool ReportsText::PutSymbolSummaryLine(
> float uncoveredBytes;
> float uncoveredInstructions;
>
> - if ( symbol->second.stats.sizeInInstructions == 0 )
> - uncoveredInstructions = 0;
> - else
> - uncoveredInstructions = (symbol->second.stats.uncoveredInstructions*100.0)/
> - symbol->second.stats.sizeInInstructions;
> + if (symbol->second.stats.sizeInBytes == 0) {
> + fprintf(
> + report,
> + "============================================\n"
> + "Symbol : %s\n"
> + " *** NEVER REFERENCED ***\n\n"
> + "This symbol was never referenced by an analyzed executable.\n"
> + "Therefore there is no size or disassembly for this symbol.\n"
> + "This could be due to symbol misspelling or lack of a test for\n"
> + "this symbol.\n",
> + symbol->first.c_str()
> + );
> + } else {
> + if ( symbol->second.stats.sizeInInstructions == 0 )
> + uncoveredInstructions = 0;
> + else
> + uncoveredInstructions = (symbol->second.stats.uncoveredInstructions*100.0)/
> + symbol->second.stats.sizeInInstructions;
>
> - if ( symbol->second.stats.sizeInBytes == 0 )
> - uncoveredBytes = 0;
> - else
> - uncoveredBytes = (symbol->second.stats.uncoveredBytes*100.0)/
> - symbol->second.stats.sizeInBytes;
> + if ( symbol->second.stats.sizeInBytes == 0 )
> + uncoveredBytes = 0;
> + else
> + uncoveredBytes = (symbol->second.stats.uncoveredBytes*100.0)/
> + symbol->second.stats.sizeInBytes;
>
> - fprintf(
> - report,
> - "============================================\n"
> - "Symbol : %s\n"
> - "Total Size in Bytes : %d\n"
> - "Total Size in Instructions : %d\n"
> - "Total number Branches : %d\n"
> - "Total Always Taken : %d\n"
> - "Total Never Taken : %d\n"
> - "Percentage Uncovered Instructions : %.2f\n"
> - "Percentage Uncovered Bytes : %.2f\n",
> - symbol->first.c_str(),
> - symbol->second.stats.sizeInBytes,
> - symbol->second.stats.sizeInInstructions,
> - symbol->second.stats.branchesNotExecuted + symbol->second.stats.branchesExecuted,
> - symbol->second.stats.branchesAlwaysTaken,
> - symbol->second.stats.branchesNeverTaken,
> - uncoveredInstructions,
> - uncoveredBytes
> - );
> + fprintf(
> + report,
> + "============================================\n"
> + "Symbol : %s\n"
> + "Total Size in Bytes : %d\n"
> + "Total Size in Instructions : %d\n"
> + "Total number Branches : %d\n"
> + "Total Always Taken : %d\n"
> + "Total Never Taken : %d\n"
> + "Percentage Uncovered Instructions : %.2f\n"
> + "Percentage Uncovered Bytes : %.2f\n",
> + symbol->first.c_str(),
> + symbol->second.stats.sizeInBytes,
> + symbol->second.stats.sizeInInstructions,
> + symbol->second.stats.branchesNotExecuted + symbol->second.stats.branchesExecuted,
> + symbol->second.stats.branchesAlwaysTaken,
> + symbol->second.stats.branchesNeverTaken,
> + uncoveredInstructions,
> + uncoveredBytes
> + );
> + }
>
> fprintf(report, "============================================\n");
> return true;
> diff --git a/tester/rt/coverage.py b/tester/rt/coverage.py
> index 8d176c3..d62c853 100644
> --- a/tester/rt/coverage.py
> +++ b/tester/rt/coverage.py
> @@ -121,7 +121,7 @@ class report_gen_html:
>
> def _prepare_head_section(self):
> head_section = '<head>' + os.linesep
> - head_section += ' <title>RTEMS coverage report</title>' + os.linesep
> + head_section += ' <title>RTEMS Coverage Report</title>' + os.linesep
> head_section += ' <style type="text/css">' + os.linesep
> head_section += ' progress[value] {' + os.linesep
> head_section += ' -webkit-appearance: none;' + os.linesep
> @@ -129,17 +129,28 @@ class report_gen_html:
> head_section += ' width: 150px;' + os.linesep
> head_section += ' height: 15px;' + os.linesep
> head_section += ' }' + os.linesep
> + head_section += ' table, th, td {' + os.linesep
> + head_section += ' border: 1px solid black;' + os.linesep
> + head_section += ' border-collapse: collapse;' + os.linesep
> + head_section += ' }' + os.linesep
> + head_section += ' td {' + os.linesep
> + head_section += ' text-align:center;' + os.linesep
> + head_section += ' }' + os.linesep
> head_section += ' </style>' + os.linesep
> head_section += '</head>' + os.linesep
> return head_section
>
> def _prepare_index_content(self, partial_reports):
> - header = "<h1> RTEMS coverage analysis report </h1>" + os.linesep
> + header = "<h1> RTEMS Coverage Analysis Report </h1>" + os.linesep
> header += "<h3>Coverage reports by symbols sets:</h3>" + os.linesep
> - table = "<table>" + os.linesep
> + table = "<table class=\"covoar table-autosort:0 table-autofilter table-stripeclass:covoar-tr-odd\">" + os.linesep
> + table += "<thead>" + os.linesep
> table += self._header_row()
> - for symbol_set in partial_reports:
> + table += "</thead>" + os.linesep
> + table += "<tbody>" + os.linesep
> + for symbol_set in sorted(partial_reports.keys()):
> table += self._row(symbol_set, partial_reports[symbol_set])
> + table += "</tbody>" + os.linesep
> table += "</table> </br>"
> timestamp = "Analysis performed on " + datetime.datetime.now().ctime()
> return "<body>\n" + header + table + timestamp + "\n</body>"
>
More information about the devel
mailing list