Merge pull request Ericsson#2666 from csordasmarton/fix_remove_report…

…_from_plist

[client] Do not store empty plist file to the server

analyzer/tests/unit/remove_report_test_files/files/empty.h

@@ -0,0 +1,4 @@
+#ifndef EMPTY_H
+#define EMPTY_H
+
+#endif

analyzer/tests/unit/remove_report_test_files/files/x.cpp

@@ -1,5 +1,6 @@
 #include "x.h"
 #include "y.h"
+#include "empty.h"
 
 long bar2(long a, long b, long c, long d) {
   c = a - b;

analyzer/tests/unit/remove_report_test_files/keep_only_empty.expected.plist

@@ -0,0 +1,12 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
+<plist version="1.0">
+<dict>
+	<key>clang_version</key>
+	<string>clang version 9.0.0 (tags/RELEASE_900/final)</string>
+	<key>diagnostics</key>
+	<array/>
+	<key>files</key>
+	<array/>
+</dict>
+</plist>

analyzer/tests/unit/remove_report_test_files/keep_only_empty.txt

@@ -0,0 +1,2 @@
++files/empty.h
+-*

analyzer/tests/unit/remove_report_test_files/skip_x_header.expected.plist

@@ -555,6 +555,7 @@
 	<array>
 		<string>files/x.cpp</string>
 		<string>files/y.h</string>
+		<string>files/empty.h</string>
 	</array>
 </dict>
 </plist>

analyzer/tests/unit/remove_report_test_files/x.plist

@@ -667,6 +667,7 @@
   <string>files/x.cpp</string>
   <string>files/x.h</string>
   <string>files/y.h</string>
+  <string>files/empty.h</string>
  </array>
 </dict>
 </plist>

analyzer/tests/unit/test_remove_report_from_plist.py

@@ -59,3 +59,17 @@ def test_skip_all_header(self):
             expected = plist_file.read()
 
         self.assertEqual(data, expected)
+
+    def test_keep_only_empty(self):
+        """ Test skipping all files except empty. """
+        with open('keep_only_empty.txt',
+                  encoding="utf-8", errors="ignore") as skip_file:
+            skip_handler = skiplist_handler.SkipListHandler(skip_file.read())
+
+        with open('x.plist', 'r') as plist_data:
+            data = remove_report_from_plist(plist_data, skip_handler)
+
+        with open('keep_only_empty.expected.plist', 'rb') as plist_file:
+            expected = plist_file.read()
+
+        self.assertEqual(data, expected)

codechecker_common/plist_parser.py

@@ -317,7 +317,7 @@ def remove_report_from_plist(plist_file_obj, skip_handler):
         kept_diagnostics, kept_files = get_kept_report_data(report_data,
                                                             file_ids_to_remove)
         report_data['diagnostics'] = kept_diagnostics
-        report_data['files'] = kept_files
+        report_data['files'] = kept_files if kept_diagnostics else []
 
         return plistlib.dumps(report_data)
 

web/client/codechecker_client/cmd/store.py

@@ -309,7 +309,16 @@ def collect_file_hashes_from_plist(plist_file):
             if f.endswith(".plist"):
                 missing_files, source_file_mod_times = \
                     collect_file_hashes_from_plist(plist_file)
-                if not missing_files:
+
+                if missing_files:
+                    LOG.warning("Skipping '%s' because it refers "
+                                "the following missing source files: %s",
+                                plist_file, missing_files)
+                elif not source_file_mod_times:
+                    # If there is no source in the plist we will not upload
+                    # it to the server.
+                    LOG.debug("Skip empty plist file: %s", plist_file)
+                else:
                     LOG.debug("Copying file '%s' to ZIP assembly dir...",
                               plist_file)
                     files_to_compress.add(os.path.join(input_path, f))
@@ -321,10 +330,7 @@ def collect_file_hashes_from_plist(plist_file):
                     for k, v in source_file_mod_times.items():
                         if v > plist_mtime:
                             changed_files.add(k)
-                else:
-                    LOG.warning("Skipping '%s' because it refers "
-                                "the following missing source files: %s",
-                                plist_file, missing_files)
+
             elif f == 'metadata.json':
                 metadata_json_to_compress.add(os.path.join(input_path, f))
             elif f == 'skip_file':

web/server/codechecker_server/api/report_server.py

@@ -2524,8 +2524,21 @@ def checker_is_unavailable(checker_name):
                 continue
 
             file_ids = {}
-            for file_name in files:
-                file_ids[file_name] = file_path_to_id[file_name]
+            if reports:
+                missing_ids_for_files = []
+
+                for file_name in files:
+                    file_id = file_path_to_id.get(file_name, -1)
+                    if file_id == -1:
+                        missing_ids_for_files.append(file_name)
+                        continue
+
+                    file_ids[file_name] = file_id
+
+                if missing_ids_for_files:
+                    LOG.error("Failed to get file path id for '%s'!",
+                              file_name)
+                    continue
 
             # Store report.
             for report in reports:

web/tests/functional/skip/test_skip.py

@@ -7,15 +7,15 @@
 
 """ skip function test.  """
 
-
-import os
 import logging
+import os
+import plistlib
 import unittest
 
 from libtest.debug_printer import print_run_results
 from libtest.thrift_client_to_db import get_all_run_results
 from libtest.result_compare import find_all
-from libtest import env
+from libtest import codechecker, env
 
 
 class TestSkip(unittest.TestCase):
@@ -25,24 +25,26 @@ class TestSkip(unittest.TestCase):
     def setUp(self):
 
         # TEST_WORKSPACE is automatically set by test package __init__.py
-        test_workspace = os.environ['TEST_WORKSPACE']
+        self.test_workspace = os.environ['TEST_WORKSPACE']
 
         test_class = self.__class__.__name__
-        print('Running ' + test_class + ' tests in ' + test_workspace)
+        print('Running ' + test_class + ' tests in ' + self.test_workspace)
+
+        self._codechecker_cfg = env.import_codechecker_cfg(self.test_workspace)
 
         # Get the clang version which is tested.
         self._clang_to_test = env.clang_to_test()
 
         # Get the test configuration from the prepared test workspace.
-        self._testproject_data = env.setup_test_proj_cfg(test_workspace)
+        self._testproject_data = env.setup_test_proj_cfg(self.test_workspace)
         self.assertIsNotNone(self._testproject_data)
 
         # Setup a viewer client to test viewer API calls.
-        self._cc_client = env.setup_viewer_client(test_workspace)
+        self._cc_client = env.setup_viewer_client(self.test_workspace)
         self.assertIsNotNone(self._cc_client)
 
         # Get the run names which belong to this test.
-        run_names = env.get_run_names(test_workspace)
+        run_names = env.get_run_names(self.test_workspace)
 
         runs = self._cc_client.getRunData(None, None, 0, None)
 
@@ -96,3 +98,31 @@ def test_skip(self):
                             "using skip")
 
         self.assertEqual(len(run_results), len(test_proj_res) - len(skipped))
+
+    def test_skip_plist_without_diag(self):
+        """ Store plist file without diagnostics.
+
+        Store plist file which does not contain any diagnostic but it refers
+        some existing source file.
+        """
+        test_dir = os.path.join(self.test_workspace, "no_diag")
+        if not os.path.isdir(test_dir):
+            os.mkdir(test_dir)
+
+        src_file = os.path.join(test_dir, "src.cpp")
+        with open(src_file, 'w', encoding='utf-8', errors='ignore') as src_f:
+            src_f.write("int main() { return 0; }")
+
+        plist_file = os.path.join(test_dir, "no_diag.plist")
+        with open(plist_file, 'wb') as plist_f:
+            data = {
+                'diagnostics': [],
+                'files': [src_file]
+            }
+            plistlib.dump(data, plist_f)
+
+        cfg = dict(self._codechecker_cfg)
+        cfg['reportdir'] = test_dir
+
+        ret = codechecker.store(cfg, 'no_diag')
+        self.assertEqual(ret, 0)