Skip to content

Commit

Permalink
fix 'NoneType' object is not subscriptable (#160)
Browse files Browse the repository at this point in the history
Fix for https://l.codecov.dev/hqZ1Ci (Sentry issue)

The error itself is `TypeError: 'NoneType' object is not subscriptable`.
This was happening because if we don't have static analysis for a file
the lines relevant for that file might be `None`, and we we're not checking
this condition prior to starting to access the dict items.

The fix itself is very simple, but I wanted to add type hints and better
tests too, so they're in the change as well.
  • Loading branch information
giovanni-guidini committed Oct 25, 2023
1 parent 9bc093f commit db1ffbf
Show file tree
Hide file tree
Showing 2 changed files with 81 additions and 7 deletions.
26 changes: 19 additions & 7 deletions tasks/label_analysis.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import logging
from typing import List, Optional, Set, Tuple
from typing import Dict, List, Optional, Set, Tuple, TypedDict

import sentry_sdk
from shared.celery_config import label_analysis_task_name
Expand Down Expand Up @@ -30,6 +30,16 @@
)


class LinesRelevantToChangeInFile(TypedDict):
all: bool
lines: Set[int]


class LinesRelevantToChange(TypedDict):
all: bool
files: Dict[str, Optional[LinesRelevantToChangeInFile]]


class LabelAnalysisRequestProcessingTask(
BaseCodecovTask, name=label_analysis_task_name
):
Expand Down Expand Up @@ -75,9 +85,9 @@ async def run_async(self, db_session, request_id, *args, **kwargs):
return self._handle_larq_already_calculated(label_analysis_request)

try:
lines_relevant_to_diff = await self._get_lines_relevant_to_diff(
label_analysis_request
)
lines_relevant_to_diff: Optional[
LinesRelevantToChange
] = await self._get_lines_relevant_to_diff(label_analysis_request)
base_report = self._get_base_report(label_analysis_request)

if lines_relevant_to_diff and base_report:
Expand Down Expand Up @@ -217,7 +227,7 @@ def _get_requested_labels(self, label_analysis_request: LabelAnalysisRequest):

@sentry_sdk.trace
def _get_existing_labels(
self, report: Report, lines_relevant_to_diff
self, report: Report, lines_relevant_to_diff: LinesRelevantToChange
) -> Tuple[Set[str], Set[str], Set[str]]:
all_report_labels = self.get_all_report_labels(report)
executable_lines_labels, global_level_labels = self.get_executable_lines_labels(
Expand Down Expand Up @@ -403,7 +413,9 @@ def get_relevant_executable_lines(
return static_analysis_comparison_service.get_base_lines_relevant_to_change()

@sentry_sdk.trace
def get_executable_lines_labels(self, report: Report, executable_lines) -> set:
def get_executable_lines_labels(
self, report: Report, executable_lines: LinesRelevantToChange
) -> set:
if executable_lines["all"]:
return (self.get_all_report_labels(report), set())
full_sessions = set()
Expand All @@ -412,7 +424,7 @@ def get_executable_lines_labels(self, report: Report, executable_lines) -> set:
# Prime piece of code to be rust-ifyied
for name, file_executable_lines in executable_lines["files"].items():
rf = report.get(name)
if rf:
if rf and file_executable_lines:
if file_executable_lines["all"]:
for line_number, line in rf.lines:
if line and line.datapoints:
Expand Down
62 changes: 62 additions & 0 deletions tasks/tests/unit/test_label_analysis.py
Original file line number Diff line number Diff line change
Expand Up @@ -400,8 +400,21 @@ def sample_report_with_labels():
complexity=None,
),
)
random_rf = ReportFile("path/from/randomfile_no_static_analysis.html")
random_rf.append(
1,
ReportLine.create(
coverage=1,
type=None,
sessions=[(LineSession(id=1, coverage=1))],
datapoints=None,
complexity=None,
),
)
r.append(first_rf)
r.append(second_rf)
r.append(random_rf)

return r


Expand Down Expand Up @@ -665,6 +678,55 @@ def test_get_executable_lines_labels_all_labels_in_one_file(sample_report_with_l
)


def test_get_executable_lines_labels_some_labels_in_one_file(sample_report_with_labels):
executable_lines = {
"all": False,
"files": {"source.py": {"all": False, "lines": set([5, 6])}},
}
task = LabelAnalysisRequestProcessingTask()
assert task.get_executable_lines_labels(
sample_report_with_labels, executable_lines
) == (
{"apple", "label_one", "pineapple", "banana"},
set(),
)


def test_get_executable_lines_labels_some_labels_in_one_file_with_globals(
sample_report_with_labels,
):
executable_lines = {
"all": False,
"files": {"source.py": {"all": False, "lines": set([6, 8])}},
}
task = LabelAnalysisRequestProcessingTask()
assert task.get_executable_lines_labels(
sample_report_with_labels, executable_lines
) == (
{"label_one", "pineapple", "banana", "orangejuice", "applejuice"},
{"applejuice", "justjuice", "orangejuice"},
)


def test_get_executable_lines_labels_some_labels_in_one_file_other_null(
sample_report_with_labels,
):
executable_lines = {
"all": False,
"files": {
"source.py": {"all": False, "lines": set([5, 6])},
"path/from/randomfile_no_static_analysis.html": None,
},
}
task = LabelAnalysisRequestProcessingTask()
assert task.get_executable_lines_labels(
sample_report_with_labels, executable_lines
) == (
{"apple", "label_one", "pineapple", "banana"},
set(),
)


def test_get_all_labels_one_session(sample_report_with_labels):
task = LabelAnalysisRequestProcessingTask()
assert task.get_labels_per_session(sample_report_with_labels, 1) == {
Expand Down

0 comments on commit db1ffbf

Please sign in to comment.