Refactor/reporters

[gro1m]

This PR is mainly a refactoring of the reporters.py file. It shifts as much configuration and formatting into the base classes Reporter and DeltaReporter. The main reason for this is to keep the needed changes to this file when introducing new formats, which needs a Reporter as well as a DeltaReporter subclass, minimal! Furthermore, it sets a standard for all the output tables. Possible future supportable output formats include: JSON, YAML, CSV, TSV, NSV (new-line separated values sometimes also summarized as TSV).

Also, the refactoring aaims to show clearly the used data structure, which are named tuples at the moment and follow a row-based logic. Namedtuples are immutable, as the code contains options to include a missed_lines column, the analogue mutable data structure recordtype might be an option. Dictionaries could be an option as well (especially when trying to extend to JSON and YAML formats); the nature of dictionaries demand though to have unique keys and thus the code would have to be transformed in such a way as that it adheres to a column-wise implementation logic and then the output could be formatted as in the dictionary example from tabulate to have the headers as keys:

>>> print(tabulate({"Name": ["Alice", "Bob"],
...                 "Age": [24, 19]}, headers="keys"))
  Age  Name
-----  ------
   24  Alice
   19  Bob

[aconrad]

Can we rebase and resolve conflicts?

[gro1m]

Hi @aconrad This PR is a bit more complicated. Up to commit https://github.com/aconrad/pycobertura/pull/124/commits/bb1aa1a60e6a90e7a9592fba43fbd97cfca444e5 it is a simple refactoring. The following commits also make adjustments to the data structure, i.e. from named tuples to dictionaries. The problem is at the moment that I only could do the reporter classes but not the delta reporters. I have an implementation for the delta reporters but the test fails, because I have a hard time with the following lines:

• https://github.com/aconrad/pycobertura/blob/master/pycobertura/templates/html-delta.jinja2#L41
• https://github.com/aconrad/pycobertura/blob/master/pycobertura/templates/html-delta.jinja2#L46
• https://github.com/aconrad/pycobertura/blob/master/pycobertura/templates/html-delta.jinja2#L57

If we could use one or 2 functions for these lines I would be able to do it I guess, but like this it is difficult. If you could give some help there, that would be very much appreciated. Otherwise we have half of the classes using dictionaries (the reporters classes) and the other using tuples (the delta reporters classes). Unfortunately, I did not foresee this difficulty.

Another approach could be also to first try to merge the first 2 commits in this PR and then do the switch to dictionaries.

[gro1m]

Hi @aconrad I think now it is at a nice place (everything with dictionaries) and I resolved your review, but for the 2 non-resolved ones, which I commented above and are still open and would be good if you could go over the code again :)

[aconrad]

Hey @gro1m, thanks again for your contributions. I'm away from a laptop for a couple of days, will take a look at your work next week.

[gro1m]

Hi @aconrad Thanks for the info, I should have now resolved addressed all of your comments above.

As an additional feedback: it would be nice if we could make the html report tests formatting-independent and if we could have a formatter for jinja2 files. The lines 37-51 in html-delta.jinja2 are quite confusing without proper indentation:

{%- for iter in range(lines["Filename"]|length) %}
          <tr>
            <td><a href="#{{ lines["Filename"][iter] }}">{{ lines["Filename"][iter]}}</a></td>
            <td>{{ lines["Stmts"][iter] }}</td>
            <td><span{% if lines["Miss"][iter] | is_not_equal_to_dash %} class="{{ lines["Miss"][iter] | misses_color }}"{% endif %}>{{ lines["Miss"][iter] }}</span></td>
            <td>{{ lines["Cover"][iter] }}</td>
{%- if show_source and show_missing %}
            <td>
{%- for missed_line in lines["Missing"][iter] %}
            {%- if not loop.first %}, {% endif %}<span class="{{ missed_line | misses_color }}">{{ missed_line }}</span>
{%- endfor %}
            </td>
{%- endif %}
          </tr>
{%- endfor %}

It took a long time for me to see that the missed_line loop is nested inside the first for loop and that the if statement is in the first for loop...

[aconrad]

It took a long time for me to see that the missed_line loop is nested inside the first for loop and that the if statement is in the first for loop...

Ugh, yes. If I recall, I was trying to be strict about the string output. The dashes you see in {%- are meant to suppress the whitespace before the jinja template. It's probably not the best way to test it and it's prone to breakage / fragile. Best would probably be to parse the HTML output in an in-memory DOM representation and then check the DOM itself or dump the DOM back to a string for comparison with a fixture.

[gro1m]

I will also do the CHANGES section again, once you are happy with the rest in this PR :)

[aconrad]

Merged! 🎉

Thanks for all your work on this! Great clean-up of the reporters.py file. This lays the ground for easier-to-implement output formats!