[pgo] deprecate --no-sparse from merge scripts

* recipe has already migrated to --sparse usage for code coverage.
  pgo defaults to it off
* unit tests have been updated to run with default behaviour
  --sparse off. unit test ensures that --sparse set will indeed
  set sparse variable to true.

Bug: 1077304,1105545
Change-Id: Id95b7a1730f80add0df27a5328fe48b1ac3f98f5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2296891Reviewed-by: Yuke Liao <liaoyuke@chromium.org>
Commit-Queue: Jeff Yoon <jeffyoon@chromium.org>
Cr-Commit-Position: refs/heads/master@{#788327}

testing/merge_scripts/code_coverage/merge_lib.py

@@ -23,7 +23,7 @@ logging.basicConfig(
 def _call_profdata_tool(profile_input_file_paths,
                         profile_output_file_path,
                         profdata_tool_path,
-                        sparse=True):
+                        sparse=False):
   """Calls the llvm-profdata tool.
 
   Args:
@@ -82,7 +82,7 @@ def _get_profile_paths(input_dir,
 
 def _validate_and_convert_profraws(profraw_files,
                                    profdata_tool_path,
-                                   sparse=True):
+                                   sparse=False):
   """Validates and converts profraws to profdatas.
 
   For each given .profraw file in the input, this method first validates it by
@@ -135,7 +135,7 @@ def _validate_and_convert_profraws(profraw_files,
 
 def _validate_and_convert_profraw(profraw_file, output_profdata_files,
                                   invalid_profraw_files, counter_overflows,
-                                  profdata_tool_path, sparse=True):
+                                  profdata_tool_path, sparse=False):
   output_profdata_file = profraw_file.replace('.profraw', '.profdata')
   subprocess_cmd = [
       profdata_tool_path,
@@ -223,7 +223,7 @@ def merge_profiles(input_dir,
                    input_extension,
                    profdata_tool_path,
                    input_filename_pattern='.*',
-                   sparse=True,
+                   sparse=False,
                    skip_validation=False):
   """Merges the profiles produced by the shards using llvm-profdata.
 

testing/merge_scripts/code_coverage/merge_results.py

@@ -55,18 +55,6 @@ def _MergeAPIArgumentParser(*args, **kwargs):
       '--per-cl-coverage',
       action='store_true',
       help='set to indicate that this is a per-CL coverage build')
-  # TODO(crbug.com/1077304) - migrate this to sparse=False as default, and have
-  # --sparse to set sparse
-  parser.add_argument(
-      '--no-sparse',
-      action='store_false',
-      dest='sparse',
-      help='run llvm-profdata without the sparse flag.')
-  # TODO(crbug.com/1077304) - The intended behaviour is to default sparse to
-  # false. --no-sparse above was added as a workaround, and will be removed.
-  # This is being introduced now in support of the migration to intended
-  # behavior. Ordering of args matters here, as the default is set by the former
-  # (sparse defaults to False because of ordering. See unit tests for details)
   parser.add_argument(
       '--sparse',
       action='store_true',

testing/merge_scripts/code_coverage/merge_results_test.py

@@ -41,7 +41,7 @@ class MergeProfilesTest(unittest.TestCase):
         build_properties, '--summary-json', 'summary.json', '--task-output-dir',
         task_output_dir, '--profdata-dir', profdata_dir, '--llvm-profdata',
         'llvm-profdata', 'a.json', 'b.json', 'c.json', '--test-target-name',
-        'base_unittests'
+        'base_unittests', '--sparse'
     ]
     with mock.patch.object(merger, 'merge_profiles') as mock_merge:
       mock_merge.return_value = None, None
@@ -75,7 +75,7 @@ class MergeProfilesTest(unittest.TestCase):
         self.assertEqual(
             mock_merge.call_args,
             mock.call(input_dir, output_file, '.profdata', 'llvm-profdata',
-                '.*', sparse=True))
+                '.*', sparse=False))
 
   @mock.patch.object(merger, '_validate_and_convert_profraws')
   def test_merge_profraw(self, mock_validate_and_convert_profraws):
@@ -110,7 +110,6 @@ class MergeProfilesTest(unittest.TestCase):
                       'merge',
                       '-o',
                       'output/dir/default.profdata',
-                      '-sparse=true',
                       '/b/some/path/0/default-1.profdata',
                       '/b/some/path/1/default-2.profdata',
                   ],
@@ -145,7 +144,6 @@ class MergeProfilesTest(unittest.TestCase):
                       'merge',
                       '-o',
                       'output/dir/default.profdata',
-                      '-sparse=true',
                       '/b/some/path/0/default-1.profraw',
                       '/b/some/path/0/default-2.profraw',
                       '/b/some/path/1/default-1.profraw',
@@ -193,7 +191,6 @@ class MergeProfilesTest(unittest.TestCase):
                       'merge',
                       '-o',
                       'output/dir/default.profdata',
-                      '-sparse=true',
                       '/b/some/path/base_unittests/default.profdata',
                       '/b/some/path/url_unittests/default.profdata',
                   ],
@@ -231,7 +228,6 @@ class MergeProfilesTest(unittest.TestCase):
                       'merge',
                       '-o',
                       'output/dir/default.profdata',
-                      '-sparse=true',
                       '/b/some/path/base_unittests/base_unittests.profdata',
                       '/b/some/path/url_unittests/url_unittests.profdata',
                   ],
@@ -330,30 +326,14 @@ class MergeProfilesTest(unittest.TestCase):
 
     test_scenarios = [
       {
-        # Base set of args should set --sparse to true by default
+        # Base set of args should set --sparse to false by default
         'args': None,
-        'expected_outcome': True,
-      },
-      {
-        # Sparse should parse to False when --no-sparse is specified
-        'args': ['--no-sparse'],
         'expected_outcome': False,
       },
       {
         # Sparse should parse True when only --sparse is specified
         'args': ['--sparse'],
         'expected_outcome': True,
-      },
-      {
-        # Sparse should take the last arg specified, so with --no-sparse at the
-        # end this should resolve false.
-        'args': ['--sparse', '--no-sparse'],
-        'expected_outcome': False,
-      },
-      {
-        # --sparse specified at end should resolve true.
-        'args': ['--no-sparse', '--sparse'],
-        'expected_outcome': True,
       }
     ]
 

testing/merge_scripts/code_coverage/merge_steps.py

@@ -23,19 +23,6 @@ def _merge_steps_argument_parser(*args, **kwargs):
       default='.*',
       help='regex pattern of profdata filename to merge for current test type. '
           'If not present, all profdata files will be merged.')
-  # TODO(crbug.com/1077304) - migrate this to sparse=False as default, and have
-  # --sparse to set sparse
-  parser.add_argument(
-      '--no-sparse',
-      action='store_false',
-      dest='sparse',
-      help='run llvm-profdata without the sparse flag.')
-  # TODO(crbug.com/1077304) - The intended behaviour is to default sparse to
-  # false. --no-sparse above was added as a workaround, and will be removed.
-  # This is being introduced now in support of the migration to intended
-  # behavior. Ordering of args matters here, as the default is set by the former
-  # (sparse defaults to False because of ordering. See merge_results unit tests
-  # for details)
   parser.add_argument(
       '--sparse',
       action='store_true',