Skip to content

Commit

Permalink
[fix] Refine when a report is regarded as outstanding for tags
Browse files Browse the repository at this point in the history
This patch is many things all in once:
* We fix a bug where diffing tags returned unexpected (and percieved to
  be incorrect) results,
* We redefine what we expect from diffing tags,
* We document more explicitly the difference in between tag and run
  diffs,
* We redefine the expected heaviour around review status rules.

WIP: explain more, add docs commit
  • Loading branch information
Szelethus committed Aug 28, 2023
1 parent deab57c commit 64139c7
Show file tree
Hide file tree
Showing 3 changed files with 63 additions and 18 deletions.
9 changes: 8 additions & 1 deletion web/server/codechecker_server/api/report_server.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import os
import re
import shlex
import sqlparse
import stat
import time
import zlib
Expand Down Expand Up @@ -70,6 +71,10 @@
SQLITE_MAX_COMPOUND_SELECT = 500


def dumpSql(q):
print(sqlparse.format(str(q), reindent=True))


class CommentKindValue:
USER = 0
SYSTEM = 1
Expand Down Expand Up @@ -588,9 +593,11 @@ def get_open_reports_date_filter_query_old(tbl=Report, date=RunHistory.time):
def get_diff_bug_id_query(session, run_ids, tag_ids, open_reports_date):
""" Get bug id query for diff. """
q = session.query(Report.bug_id.distinct())
q = q.filter(Report.fixed_at.is_(None))

if run_ids:
q = q.filter(Report.run_id.in_(run_ids))
if not tag_ids and not open_reports_date:
q = q.filter(Report.fixed_at.is_(None))

if tag_ids:
q = q.outerjoin(RunHistory,
Expand Down
12 changes: 4 additions & 8 deletions web/tests/functional/diff_cmdline/test_diff_cmdline.py
Original file line number Diff line number Diff line change
Expand Up @@ -755,10 +755,8 @@ def get_run_diff_count(diff_type: DiffType):
["run1:tag1"], ["run1:tag2"])
return len(reports)

# FIXME: The FP suppression appeared from one tag to the next, so it
# should be in the RESOLVED set.
self.assertEqual(get_run_diff_count(DiffType.NEW), 0)
self.assertEqual(get_run_diff_count(DiffType.RESOLVED), 0)
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):
Expand All @@ -767,9 +765,7 @@ def get_run_diff_count_reverse(diff_type: DiffType):
["run1:tag2"], ["run1:tag1"])
return len(reports)

# FIXME: The FP suppression appeared from one tag to the next, so it
# should be in the NEW set when the diff sets are reversed.
self.assertEqual(get_run_diff_count_reverse(DiffType.NEW), 0)
self.assertEqual(get_run_diff_count_reverse(DiffType.NEW), 1)
self.assertEqual(get_run_diff_count_reverse(DiffType.RESOLVED), 0)
self.assertEqual(get_run_diff_count_reverse(DiffType.UNRESOLVED), 0)

Expand Down Expand Up @@ -809,7 +805,7 @@ def get_run_diff_count(diff_type: DiffType):
return len(reports)

self.assertEqual(get_run_diff_count(DiffType.NEW), 0)
self.assertEqual(get_run_diff_count(DiffType.RESOLVED), 0)
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):
Expand All @@ -818,7 +814,7 @@ def get_run_diff_count_reverse(diff_type: DiffType):
["run1:tag2"], ["run1:tag1"])
return len(reports)

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

Expand Down
60 changes: 51 additions & 9 deletions web/tests/functional/diff_remote/test_diff_remote.py
Original file line number Diff line number Diff line change
Expand Up @@ -119,8 +119,11 @@ def setup_class(self):
# Extend the checker configuration with the server access.
codechecker_cfg.update(server_access)

# Base analysis
# ===-------------------------------------------------------------=== #
# Baseline analysis.
# ===-------------------------------------------------------------=== #

# ===-------------------------- Analysis -------------------------=== #
altered_file = os.path.join(test_proj_path_base,
"call_and_message.cpp")
project.insert_suppression(altered_file)
Expand All @@ -138,18 +141,24 @@ def setup_class(self):
codechecker_cfg['reportdir'] = os.path.join(test_proj_path_base,
'reports')

# ===------------------------- Store 1. --------------------------=== #
test_project_name_base = project_info['name'] + '_' + uuid.uuid4().hex
ret = codechecker.store(codechecker_cfg, test_project_name_base)
if ret:
sys.exit(1)

# ===------------------------- Store 2. --------------------------=== #
# Store with a literal ':' in the name.
ret = codechecker.store(codechecker_cfg,
test_project_name_base + ":base")
if ret:
sys.exit(1)

# New analysis
# ===-------------------------------------------------------------=== #
# New analysis (as opposed to baseline, this is the newer version).
# ===-------------------------------------------------------------=== #

# ===-------------------------- Analysis -------------------------=== #
altered_file = os.path.join(test_proj_path_new, "call_and_message.cpp")
project.insert_suppression(altered_file)
codechecker_cfg['reportdir'] = os.path.join(test_proj_path_new,
Expand All @@ -166,24 +175,30 @@ def setup_class(self):
codechecker_cfg['reportdir'] = os.path.join(test_proj_path_new,
'reports')

# ===------------------------- Store 1. --------------------------=== #
test_project_name_new = project_info['name'] + '_' + uuid.uuid4().hex
ret = codechecker.store(codechecker_cfg, test_project_name_new)
if ret:
sys.exit(1)

# ===------------------------- Store 2. --------------------------=== #
# Store with a literal ':' in the name.
ret = codechecker.store(codechecker_cfg,
test_project_name_new + ":new")
if ret:
sys.exit(1)

# ===-------------------------------------------------------------=== #
# Another round of analyses for tags -- tag1
# ===-------------------------------------------------------------=== #

# ===-------------------------- Analysis -------------------------=== #
# Analyze multiple times to store results with multiple tags.
codechecker_cfg['reportdir'] = os.path.join(test_proj_path_update,
'reports')

test_project_name_update = \
project_info['name'] + '_' + uuid.uuid4().hex
codechecker_cfg['tag'] = 't1'
codechecker_cfg['checkers'] = ['-d', 'core.CallAndMessage',
'-e', 'core.StackAddressEscape'
]
Expand All @@ -196,35 +211,54 @@ def setup_class(self):
if ret:
sys.exit(1)

# ===--------------------------- Store ---------------------------=== #
# Store update with t1 tag.
codechecker_cfg['tag'] = 't1'
ret = codechecker.store(codechecker_cfg, test_project_name_update)
if ret:
sys.exit(1)

codechecker_cfg['tag'] = 't2'
# ===-------------------------------------------------------------=== #
# Another round of analyses for tags -- tag2
# ===-------------------------------------------------------------=== #

# ===-------------------------- Analysis -------------------------=== #
codechecker_cfg['checkers'] = ['-e', 'core.CallAndMessage',
'-d', 'core.StackAddressEscape'
]
ret = codechecker.analyze(codechecker_cfg, test_proj_path_update)
if ret:
sys.exit(1)

# ===--------------------------- Store ---------------------------=== #
# Store update with t2 tag.
codechecker_cfg['tag'] = 't2'
ret = codechecker.store(codechecker_cfg, test_project_name_update)
if ret:
sys.exit(1)

codechecker_cfg['tag'] = 't3'
# ===-------------------------------------------------------------=== #
# Another round of analyses for tags -- tag3
# Mind that the analysis config from tag2 to tag3 is unchanged.
# ===-------------------------------------------------------------=== #

# ===-------------------------- Analysis -------------------------=== #
ret = codechecker.log_and_analyze(codechecker_cfg,
test_proj_path_update)
if ret:
sys.exit(1)

# ===--------------------------- Store ---------------------------=== #
# Store update with t3 tag.
codechecker_cfg['tag'] = 't3'
ret = codechecker.store(codechecker_cfg, test_project_name_update)
if ret:
sys.exit(1)

# ===-------------------------------------------------------------=== #
# Done with the analyses and stores.
# ===-------------------------------------------------------------=== #

# Order of the test run names matter at comparison!
codechecker_cfg['run_names'] = [test_project_name_base,
test_project_name_new,
Expand Down Expand Up @@ -811,7 +845,8 @@ def get_run_tags(tag_name):
cmp_data,
False)

# 5 new core.CallAndMessage issues.
# 5 new core.CallAndMessage issues (as the checker was enabled from
# tag1 to tag2).
self.assertEqual(len(diff_res), 5)

cmp_data.diffType = DiffType.RESOLVED
Expand All @@ -822,7 +857,10 @@ def get_run_tags(tag_name):
tag_filter,
cmp_data,
False)
self.assertEqual(len(diff_res), 0)

# 3 core.StackAddressEscape reports are resolved (as the checker was
# disabled from tag1 to tag2).
self.assertEqual(len(diff_res), 3)

cmp_data.diffType = DiffType.UNRESOLVED
diff_res = self._cc_client.getRunResults([run_id],
Expand Down Expand Up @@ -880,7 +918,8 @@ def get_run_tags(tag_name):
cmp_data,
False)

# 5 new core.CallAndMessage issues.
# 5 new core.CallAndMessage issues (as the checker was enabled from
# tag1 to tag2).
self.assertEqual(len(diff_res), 5)

cmp_data.diffType = DiffType.RESOLVED
Expand All @@ -891,7 +930,10 @@ def get_run_tags(tag_name):
tag_filter,
cmp_data,
False)
self.assertEqual(len(diff_res), 0)

# 3 core.StackAddressEscape reports are resolved (as the checker was
# disabled from tag1 to tag2).
self.assertEqual(len(diff_res), 3)

cmp_data.diffType = DiffType.UNRESOLVED
diff_res = self._cc_client.getRunResults([run_id],
Expand Down

0 comments on commit 64139c7

Please sign in to comment.