Skip to content

Commit

Permalink
[fix] Source code comment over review status rule
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
bruntib committed Oct 12, 2023
1 parent 6f57fa2 commit 2ad6e2e
Show file tree
Hide file tree
Showing 2 changed files with 52 additions and 12 deletions.
7 changes: 4 additions & 3 deletions web/server/codechecker_server/api/mass_store_run.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
57 changes: 48 additions & 9 deletions web/tests/functional/review_status/test_review_status.py
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand All @@ -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])
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 2ad6e2e

Please sign in to comment.