From 5f5c8c0a4b1c028de69dea568fe9697bbccbcb45 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Krist=C3=B3f=20Umann?= Date: Wed, 20 Sep 2023 15:08:47 +0200 Subject: [PATCH] [cmd][diff] Ignore detection status for tags 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). --- docs/web/diff.md | 2 +- .../codechecker_client/cmd_line_client.py | 21 +++++++++++++++++++ .../diff_cmdline/test_diff_cmdline.py | 18 +++++++++++++--- 3 files changed, 37 insertions(+), 4 deletions(-) diff --git a/docs/web/diff.md b/docs/web/diff.md index 20bc4dc645..de731f16fc 100644 --- a/docs/web/diff.md +++ b/docs/web/diff.md @@ -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". diff --git a/web/client/codechecker_client/cmd_line_client.py b/web/client/codechecker_client/cmd_line_client.py index c23ab63759..fba88dfc0d 100644 --- a/web/client/codechecker_client/cmd_line_client.py +++ b/web/client/codechecker_client/cmd_line_client.py @@ -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) diff --git a/web/tests/functional/diff_cmdline/test_diff_cmdline.py b/web/tests/functional/diff_cmdline/test_diff_cmdline.py index 685074ecc7..2f91e99be7 100644 --- a/web/tests/functional/diff_cmdline/test_diff_cmdline.py +++ b/web/tests/functional/diff_cmdline/test_diff_cmdline.py @@ -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 @@ -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.""" @@ -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( @@ -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, [],