Skip to content

Commit

Permalink
[cmd][diff] Ignore detection status for tags
Browse files Browse the repository at this point in the history
We already do this on the GUI, but on the cmdline, we default the
detection status to NEW, REOPENED and UNRESOLVED. This makes sense for
runs (where we check whether a report is outstanding at the time of the
query), but not for tags (as a report can have a RESOLVED detection
status but still be outstanding at the time of the tag).
  • Loading branch information
Szelethus committed Oct 5, 2023
1 parent 491a3fc commit 5f5c8c0
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 4 deletions.
2 changes: 1 addition & 1 deletion docs/web/diff.md
Original file line number Diff line number Diff line change
Expand Up @@ -760,7 +760,7 @@ You can also name timestamps which we call tags. In the below image, the cogwhee
Comparing runs and comparing tags/timestamps has a subtle difference:

* For runs, we check whether a report is outstanding _at the time of the query_. This means, as seen [in a previous example](diff.md#example-setup-1), if you store a report, _and then_ set a false positive review status rule for it, _and then_ diff this _run_ against another, the report **will not be** present in the run's outstanding reports set.
* For tags/timestamps, we check whether a report is outstanding _before that specific tag/timestamp_. This means that if you store a report under `tag1`, _and then_ set a false positive review status rule for it, _and then_ diff this _tag_ against another, the report **will be** present in the tag's outstanding report set.
* For tags/timestamps, we check whether a report is outstanding _before that specific tag/timestamp_. This means that if you store a report under `tag1`, _and then_ set a false positive review status rule for it, _and then_ diff this _tag_ against another, the report **will be** present in the tag's outstanding report set. This also implies that **we ignore the detection status of the report**.

:warning: Note: Tags are created when the report is stored. As a result, you can diff a local analysis directory against a remote tag, but there are no "local tags".

Expand Down
21 changes: 21 additions & 0 deletions web/client/codechecker_client/cmd_line_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -995,6 +995,27 @@ def get_diff_remote_runs(
ttypes.SortType.FILENAME,
ttypes.Order.ASC))]

# Generally speaking, we only compare outstanding reports in both the
# baseline and newline analyses, so it wouldn't make sense to show reports
# whose detection status indicates otherwise. However, for tags, we check
# whether a report is outstanding at the time of the tag, so in the context
# of that tag, a resolved report may actually be outstanding, so we clear
# the detection status filter.
# TODO: We don't support diffs with timestamps on the cmdline, but we
# should do this for them as well.
if base_run_tags or new_run_tags:
# If the value is not the default, lets warn the user that we overwrote
# it.
default_detection_status = [ttypes.DetectionStatus.NEW,
ttypes.DetectionStatus.REOPENED,
ttypes.DetectionStatus.UNRESOLVED]
if report_filter.detectionStatus != default_detection_status and \
report_filter.detectionStatus != []:
LOG.warning("--detection-status is ignored when comparing tags, "
"showing reports regardless of detection status.")

report_filter.detectionStatus = []

all_results = get_run_results(
client, base_ids, constants.MAX_QUERY_SIZE, 0, sort_mode,
report_filter, cmp_data, True)
Expand Down
18 changes: 15 additions & 3 deletions web/tests/functional/diff_cmdline/test_diff_cmdline.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,11 @@
import unittest

from codechecker_api.codeCheckerDBAccess_v6.ttypes import \
ReviewStatus, DiffType, ReportFilter
ReviewStatus, DiffType, ReportFilter, DetectionStatus

from codechecker_client.cmd_line_client import \
get_diff_local_dirs, get_diff_remote_run_local_dir, \
get_diff_local_dir_remote_run, get_diff_remote_runs
get_diff_local_dir_remote_run, get_diff_remote_runs, init_logger

from libtest import codechecker, env
from libtest.thrift_client_to_db import get_all_run_results
Expand Down Expand Up @@ -64,6 +64,8 @@ def setup_class(self):

env.export_test_cfg(TEST_WORKSPACE, test_config)

init_logger(None, None)

def teardown_class(self):
"""Clean up after the test."""

Expand Down Expand Up @@ -636,7 +638,7 @@ def test_remoteTag_remoteTag_different(self):

report_filter = ReportFilter()
report_filter.reviewStatus = []
report_filter.detection_status = []
report_filter.detectionStatus = []

def get_run_diff_count(diff_type: DiffType):
reports, _, _ = get_diff_remote_runs(
Expand All @@ -648,6 +650,16 @@ def get_run_diff_count(diff_type: DiffType):
self.assertEqual(get_run_diff_count(DiffType.RESOLVED), 1)
self.assertEqual(get_run_diff_count(DiffType.UNRESOLVED), 0)

# This is the default value for --review-status, but for tags, we
# should ignore it.
report_filter.detectionStatus = [DetectionStatus.NEW,
DetectionStatus.REOPENED,
DetectionStatus.UNRESOLVED]

self.assertEqual(get_run_diff_count(DiffType.NEW), 1)
self.assertEqual(get_run_diff_count(DiffType.RESOLVED), 1)
self.assertEqual(get_run_diff_count(DiffType.UNRESOLVED), 0)

def get_run_diff_count_reverse(diff_type: DiffType):
reports, _, _ = get_diff_remote_runs(
self._cc_client, report_filter, diff_type, [],
Expand Down

0 comments on commit 5f5c8c0

Please sign in to comment.