Skip to content
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

Update number of error occurrences reported + Bug fix in HTML reporter #915

Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,13 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
- For the message display of E9989 `pep8-errors`, all "blank line" messages are now custom-rendered, i.e., blank lines
are now highlighted instead of function signatures and instruction strings are added for required blank lines that
are missing.
- The default value of `pyta-number-of-messages` is now 0. This automatically displays all occurrences of the same error.

### Bug Fixes

- Fixed bug where running `python3 -m python_ta --generate-config` yields a `FileNotFoundError`.
- Fixed bug where the HTML reporter would display all error occurrences of the same type despite stating that only a limited number was being shown.
david-yz-liu marked this conversation as resolved.
Show resolved Hide resolved
- Fixed bug where the JSON reporter was not limiting the number of error occurrences displayed with respect to `pyta-number-of-messages`.

### New checkers

Expand Down
2 changes: 1 addition & 1 deletion docs/usage/configuration.md
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
# Configuration

```{note}
This page is current under construction!
This page is currently under construction!
```

## Providing Your Own Configuration Settings
Expand Down
2 changes: 1 addition & 1 deletion python_ta/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -291,7 +291,7 @@ def reset_linter(
(
"pyta-number-of-messages",
{
"default": 5,
"default": 0, # If the value is 0, all messages are displayed.
"type": "int",
"metavar": "<number_messages>",
"help": "Display a certain number of messages to the user, without overwhelming them.",
david-yz-liu marked this conversation as resolved.
Show resolved Hide resolved
Expand Down
4 changes: 2 additions & 2 deletions python_ta/config/.pylintrc
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@

# Make sure to register custom options tuple first in `python_ta/__init__.py`
# ===========================================================
# Default max amount of messages for reporter to display.
pyta-number-of-messages = 5
# Default max amount of messages for reporter to display. If the value is 0, all messages are displayed.
pyta-number-of-messages = 0

# (DEPRECATED: Use output-format option below.) Set the [REPORTS] output-format option instead.
# pyta-reporter = HTMLReporter
Expand Down
22 changes: 21 additions & 1 deletion python_ta/reporters/json_reporter.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,28 @@ def display_messages(self, layout: BaseLayout) -> None:
output.append(
{
"filename": k,
"msgs": [msg.to_dict() for msg in msgs],
"msgs": self._output_messages(msgs),
}
)

self.writeln(json.dumps(output, indent=4))

def _output_messages(self, msgs: List[NewMessage]) -> List[Dict]:
"""Returns a list of dictionaries containing formatted error messages."""
max_messages = self.linter.config.pyta_number_of_messages
unique_msg_names = {msg.message.msg_id for msg in msgs}
david-yz-liu marked this conversation as resolved.
Show resolved Hide resolved
num_occurrences = {}
output_lst = []

for msg_name in unique_msg_names:
num_occurrences[msg_name] = 0

for msg in msgs:
if max_messages == 0 or num_occurrences[msg.message.msg_id] < max_messages:
output_lst.append(msg.to_dict())
num_occurrences[msg.message.msg_id] += 1

for msg_dict in output_lst:
msg_dict["number_of_occurrences"] = num_occurrences[msg_dict["msg_id"]]

return output_lst
8 changes: 6 additions & 2 deletions python_ta/reporters/plain_reporter.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,12 +59,16 @@ def _colour_messages_by_type(self, messages: Dict[str, List[NewMessage]]) -> str
result += self._colourify("bold", msg_id)
result += self._colourify("bold", " ({}) ".format(messages[msg_id][0].symbol))
result += "Number of occurrences: {}.".format(len(messages[msg_id]))
if max_messages != float("inf") and max_messages < len(messages[msg_id]):
if (
max_messages != 0
and max_messages != float("inf")
and max_messages < len(messages[msg_id])
):
result += " (First {} shown).".format(max_messages)
result += self._BREAK

for i, msg in enumerate(messages[msg_id]):
if i == max_messages:
if max_messages != 0 and i == max_messages:
break

# Use only explanation, without redundant accessory information
Expand Down
93 changes: 48 additions & 45 deletions python_ta/reporters/templates/template.html.jinja
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@
{% endif %}
</span>
<span class="shown">
{% if limit < num_occurrences %}
{% if 0 < limit < num_occurrences %}
(First {{ limit }} shown).
{% endif %}
</span>
Expand All @@ -141,27 +141,29 @@
</div>
<div class="message-container collapsible expanded">
{% for indiv in occurrences %}
<div class="message">
<p class="message-name">
[Line {{ indiv.line }}] {{ indiv.msg }}
<span class="slider">
<button style="float: right;">
<svg class="collapse-trigger" viewBox="0 1 10 10" width="60" height="20">
<g transform="scale(0.13 0.13)">
<path d="M 10 30 l 40 40 l 40 -40" stroke="#777777" stroke-width="15px" fill="transparent"/>
</g>
</svg>
</button>
</span>
</p>
{% if indiv.snippet != '' %}
<div class="collapsible expanded message-snippet-container">
<pre class="message-snippet">{{ indiv.snippet }}</pre>
</div>
{% else %}
<span class="empty-placeholder">{{ reporter.no_snippet }}</span>
{% endif %}
</div>
{% if limit == 0 or loop.index0 < limit %}
<div class="message">
<p class="message-name">
[Line {{ indiv.line }}] {{ indiv.msg }}
<span class="slider">
<button style="float: right;">
<svg class="collapse-trigger" viewBox="0 1 10 10" width="60" height="20">
<g transform="scale(0.13 0.13)">
<path d="M 10 30 l 40 40 l 40 -40" stroke="#777777" stroke-width="15px" fill="transparent"/>
</g>
</svg>
</button>
</span>
</p>
{% if indiv.snippet != '' %}
<div class="collapsible expanded message-snippet-container">
<pre class="message-snippet">{{ indiv.snippet }}</pre>
</div>
{% else %}
<span class="empty-placeholder">{{ reporter.no_snippet }}</span>
{% endif %}
</div>
{% endif %}
{% endfor %}
</div>
</div>
Expand Down Expand Up @@ -201,7 +203,7 @@
{% endif %}
</span>
<span class="shown">
{% if limit < num_occurrences %}
{% if 0 < limit < num_occurrences %}
(First {{ limit }} shown).
{% endif %}
</span>
Expand All @@ -215,30 +217,31 @@
</button>
</span>
</div>

<div class="message-container collapsible expanded">
{% for indiv in occurrences %}
<div class="message">
<p class="message-name">
[Line {{ indiv.line }}] {{ indiv.msg }}
<span class="slider">
<button style="float: right;">
<svg class="collapse-trigger" viewBox="0 1 10 10" width="60" height="20">
<g transform="scale(0.13 0.13)">
<path d="M 10 30 l 40 40 l 40 -40" stroke="#777777" stroke-width="15px" fill="transparent"/>
</g>
</svg>
</button>
</span>
</p>
<div class="collapsible expanded message-snippet-container">
{% if indiv.snippet != '' %}
<pre class="message-snippet">{{ indiv.snippet }}</pre>
{% else %}
<span class="empty-placeholder">{{ reporter.no_snippet }}</span>
{% endif %}
</div>
</div>
{% if limit == 0 or loop.index0 < limit %}
<div class="message">
<p class="message-name">
[Line {{ indiv.line }}] {{ indiv.msg }}
<span class="slider">
<button style="float: right;">
<svg class="collapse-trigger" viewBox="0 1 10 10" width="60" height="20">
<g transform="scale(0.13 0.13)">
<path d="M 10 30 l 40 40 l 40 -40" stroke="#777777" stroke-width="15px" fill="transparent"/>
</g>
</svg>
</button>
</span>
</p>
<div class="collapsible expanded message-snippet-container">
{% if indiv.snippet != '' %}
<pre class="message-snippet">{{ indiv.snippet }}</pre>
{% else %}
<span class="empty-placeholder">{{ reporter.no_snippet }}</span>
{% endif %}
</div>
</div>
{% endif %}
{% endfor %}
</div>

Expand Down
2 changes: 1 addition & 1 deletion tests/test.pylintrc
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
# Make sure to register custom options tuple first in `python_ta/__init__.py`
# ===========================================================
# Default max amount of messages for reporter to display.
pyta-number-of-messages = 5
pyta-number-of-messages = 0

# (DEPRECATED: Use output-format option below.) Set the [REPORTS] output-format option instead.
# pyta-reporter = HTMLReporter
Expand Down
32 changes: 32 additions & 0 deletions tests/test_config/file_fixtures/funcs_with_errors.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
"""Python script used for testing that the correct number of error occurrences are being displayed."""
from typing import List

# The following imports are used solely to trigger errors.
import packaging
import pip
import pygments
import pylint


def sum_items(lst: List[int]) -> int:
"""..."""
s = 0
for i in range(len(lst)):
s += lst[i]
return s


def sum_items2(lst: List[int]) -> int:
"""..."""
s = 0
for i in range(0, len(lst)):
s += lst[i]
return s


def sum_items3(lst: List[int]) -> int:
"""..."""
s = 0
for i in range(0, len(lst), 1):
s += lst[i]
return s
55 changes: 55 additions & 0 deletions tests/test_config/test_num_error_occurrences.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
"""
Test suite for checking that the correct number of error occurrences are being displayed.
"""

import contextlib
import io
import os

from python_ta import check_all


def pyta_output(num_msgs: int) -> str:
"""Returns the PythonTA report as a string."""
output = io.StringIO()

curr_dir = os.path.dirname(__file__)
test_file = os.path.join(curr_dir, "file_fixtures", "funcs_with_errors.py")

with contextlib.redirect_stdout(output):
check_all(
module_name=test_file,
config={
"pyta-number-of-messages": num_msgs,
"output-format": "python_ta.reporters.JSONReporter",
},
)

return output.getvalue()


def test_default() -> None:
"""Tests that all messages are displayed when pyta-number-of-messages = 0."""
pyta_report = pyta_output(0)
expected = 12
actual = pyta_report.count("msg_id")

assert expected == actual


def test_num_msgs2() -> None:
"""Tests that only two messages per error are displayed when pyta-number-of-messages = 2."""
pyta_report = pyta_output(2)
expected = 7
actual = pyta_report.count("msg_id")

assert expected == actual


def test_num_msgs_greater() -> None:
"""Tests that all messages are displayed when pyta-number-of-messages is greater than the number of errors."""
pyta_report = pyta_output(5)
expected = 12
actual = pyta_report.count("msg_id")

assert expected == actual
2 changes: 1 addition & 1 deletion tests/test_messages_config/test.pylintrc
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
# Make sure to register custom options tuple first in `python_ta/__init__.py`
# ===========================================================
# Default max amount of messages for reporter to display.
pyta-number-of-messages = 5
pyta-number-of-messages = 0

# (DEPRECATED: Use output-format option below.) Set the [REPORTS] output-format option instead.
# pyta-reporter = HTMLReporter
Expand Down