From 2ad6e2e8171d27abc77c9d7e7c143e71aaa1be18 Mon Sep 17 00:00:00 2001 From: bruntib Date: Thu, 12 Oct 2023 14:35:15 +0200 Subject: [PATCH] [fix] Source code comment over review status rule When a report is suppressed by a source code comment, then it should be stronger than a review status rule with the same bug hash. This is working fine when a new review status rule is set in the GUI for existing reports. Setting a review status rule has effect on the reports which don't have a review status provided in a source code comment. However, if a new report is stored, then its review status is set based on the review status rule, even if it has a source code comment. This is fixed by this patch: the source code comment should be stronger even in a run store process. --- .../codechecker_server/api/mass_store_run.py | 7 ++- .../review_status/test_review_status.py | 57 ++++++++++++++++--- 2 files changed, 52 insertions(+), 12 deletions(-) diff --git a/web/server/codechecker_server/api/mass_store_run.py b/web/server/codechecker_server/api/mass_store_run.py index e15362eeca..efebef5d21 100644 --- a/web/server/codechecker_server/api/mass_store_run.py +++ b/web/server/codechecker_server/api/mass_store_run.py @@ -1150,9 +1150,10 @@ def get_skip_handler( # but before first check the performance reports_to_rs_rules = session.query(ReviewStatusRule, DBReport) \ .join(DBReport, DBReport.bug_id == ReviewStatusRule.bug_hash) \ - .filter(sqlalchemy.and_(DBReport.run_id == run_id, - ReviewStatusRule.bug_hash. - in_(self.__new_report_hashes))) + .filter(sqlalchemy.and_( + DBReport.run_id == run_id, + DBReport.review_status_is_in_source == False, + ReviewStatusRule.bug_hash.in_(self.__new_report_hashes))) # Set the newly stored reports for review_status, db_report in reports_to_rs_rules: diff --git a/web/tests/functional/review_status/test_review_status.py b/web/tests/functional/review_status/test_review_status.py index 6d51132b8f..d4b218cbb2 100644 --- a/web/tests/functional/review_status/test_review_status.py +++ b/web/tests/functional/review_status/test_review_status.py @@ -580,19 +580,19 @@ def test_review_status_changes(self): lambda r: r.bugHash == UNCOMMENTED_BUG_HASH, reports1)) - multi_confirmed_report = next(filter( + multi_confirmed_report1 = next(filter( lambda r: r.bugHash == MULTI_REPORT_HASH and r.reviewData.status == ReviewStatus.CONFIRMED, reports1)) - multi_intentional_report = next(filter( + multi_intentional_report1 = next(filter( lambda r: r.bugHash == MULTI_REPORT_HASH and r.reviewData.status == ReviewStatus.INTENTIONAL, reports1)) - multi_unreviewed_report = next(filter( + multi_unreviewed_report1 = next(filter( lambda r: r.bugHash == MULTI_REPORT_HASH and r.reviewData.status == ReviewStatus.UNREVIEWED, reports1), None) - multi_false_positive_report = next(filter( + multi_false_positive_report1 = next(filter( lambda r: r.bugHash == MULTI_REPORT_HASH and r.reviewData.status == ReviewStatus.FALSE_POSITIVE, reports1)) @@ -611,17 +611,17 @@ def test_review_status_changes(self): "This is intentional") self.assertIsNotNone(uncommented_report1.fixedAt) self.assertEqual( - multi_confirmed_report.reviewData.comment, + multi_confirmed_report1.reviewData.comment, "Has a source code comment.") - self.assertIsNone(multi_confirmed_report.fixedAt) + self.assertIsNone(multi_confirmed_report1.fixedAt) self.assertEqual( - multi_intentional_report.reviewData.comment, + multi_intentional_report1.reviewData.comment, "Has a different source code comment.") # Only the one with no source code comment changes its review status. self.assertEqual( - multi_false_positive_report.reviewData.comment, + multi_false_positive_report1.reviewData.comment, "This is false positive.") - self.assertIsNone(multi_unreviewed_report) + self.assertIsNone(multi_unreviewed_report1) rule_filter = ReviewStatusRuleFilter( reportHashes=[UNCOMMENTED_BUG_HASH]) @@ -652,16 +652,55 @@ def test_review_status_changes(self): reports2 = get_all_run_results(self._cc_client, runid2) + null_deref_report2 = next(filter( + lambda r: r.bugHash == NULL_DEREF_BUG_HASH, + reports2)) uncommented_report2 = next(filter( lambda r: r.bugHash == UNCOMMENTED_BUG_HASH, reports2)) + multi_confirmed_report2 = next(filter( + lambda r: r.bugHash == MULTI_REPORT_HASH and + r.reviewData.status == ReviewStatus.CONFIRMED, + reports2)) + multi_intentional_report2 = next(filter( + lambda r: r.bugHash == MULTI_REPORT_HASH and + r.reviewData.status == ReviewStatus.INTENTIONAL, + reports2)) + multi_unreviewed_report2 = next(filter( + lambda r: r.bugHash == MULTI_REPORT_HASH and + r.reviewData.status == ReviewStatus.UNREVIEWED, + reports2), None) + multi_false_positive_report2 = next(filter( + lambda r: r.bugHash == MULTI_REPORT_HASH and + r.reviewData.status == ReviewStatus.FALSE_POSITIVE, + reports2)) + + self.assertIn( + (null_deref_report2.reviewData.status, + null_deref_report2.reviewData.comment), + map(lambda x: + (x['review_status'], x['review_status_comment']), + suppress_project_bugs[null_deref_report2.bugHash])) self.assertEqual( uncommented_report2.reviewData.status, ReviewStatus.INTENTIONAL) self.assertEqual( uncommented_report2.reviewData.comment, "This is intentional") + self.assertIsNotNone(uncommented_report2.fixedAt) + self.assertEqual( + multi_confirmed_report2.reviewData.comment, + "Has a source code comment.") + self.assertIsNone(multi_confirmed_report2.fixedAt) + self.assertEqual( + multi_intentional_report2.reviewData.comment, + "Has a different source code comment.") + # Only the one with no source code comment changes its review status. + self.assertEqual( + multi_false_positive_report2.reviewData.comment, + "This is false positive.") + self.assertIsNone(multi_unreviewed_report2) # A report which gets its review status from "ReviewStatus" table at # storage should be fixed at its storage immediately if its review