Reland "[Chrome Stdlib] Use DEPS to download diff test data"

This is a reland of commit 6c8f992 This reland adds the deps entry for chrome_5672_histograms.pftrace.gz which was missing in the original CL, causing b/342182426 which caused the original CL to be reverted. Original change's description: > [Chrome Stdlib] Use DEPS to download diff test data > > We want to download Perfetto test data via GCS dependencies in DEPS > rather than Perfetto's test_data script. To make this change we: > > 1. Modify the test_data.py script to wrap > `upload_to_google_storage_first_class.py` instead of Perfetto's > `test_data` script. > 2. Add a Presubmit check to ensure the .sha256 files are in sync with > the deps entries. > 3. Update docs to give instructions for the new workflow > > Change-Id: I20616c95553f93603e338dd9fa47e84facfb60d8 > Bug: 312895063 > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5545295 > Reviewed-by: Stephen Nusko <[email protected]> > Commit-Queue: Rasika Navarange <[email protected]> > Reviewed-by: Dominic Battre <[email protected]> > Cr-Commit-Position: refs/heads/main@{#1304407} Bug: 312895063 Change-Id: I6bcd74324612e5c49ae59bee3f0b20f673d70b81 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5562906 Reviewed-by: Dominic Battre <[email protected]> Reviewed-by: Stephen Nusko <[email protected]> Commit-Queue: Rasika Navarange <[email protected]> Cr-Commit-Position: refs/heads/main@{#1305095}
sunlin-link · May 23, 2024 · c2d33d2 · c2d33d2
1 parent e877e15
commit c2d33d2
Show file tree

Hide file tree

Showing 5 changed files with 421 additions and 35 deletions.
diff --git a/DEPS b/DEPS
@@ -1958,6 +1958,83 @@ deps = {
   'src/third_party/perfetto':
     Var('android_git') + '/platform/external/perfetto.git' + '@' + '03fe17e0be05dd6c60cf6351a696c1864468b982',
 
+  'src/base/tracing/test/data': {
+    'bucket': 'perfetto',
+    'objects': [
+      {
+        'object_name': 'test_data/chrome_fcp_lcp_navigations.pftrace-ae01d849fbd75a98be1b7ddd5a8873217c377b393a1d5bbd788ed3364f7fefc3',
+        'sha256sum': 'ae01d849fbd75a98be1b7ddd5a8873217c377b393a1d5bbd788ed3364f7fefc3',
+        'size_bytes': 2398645,
+        'generation': 1697714434866488,
+        'output_file': 'chrome_fcp_lcp_navigations.pftrace'
+      },
+      {
+        'object_name': 'test_data/chrome_input_with_frame_view.pftrace-a93548822e481508c728ccc5da3ad34afcd0aec02ca7a7a4dad84ff340ee5975',
+        'sha256sum': 'a93548822e481508c728ccc5da3ad34afcd0aec02ca7a7a4dad84ff340ee5975',
+        'size_bytes': 6392331,
+        'generation': 1711402389089075,
+        'output_file': 'chrome_input_with_frame_view.pftrace'
+      },
+      {
+        'object_name': 'test_data/scroll_offsets_trace_2.pftrace-2ddd9f78d91d51e39c72c520bb54fdc9dbf1333ae722e87633fc345159296289',
+        'sha256sum': '2ddd9f78d91d51e39c72c520bb54fdc9dbf1333ae722e87633fc345159296289',
+        'size_bytes': 1496388,
+        'generation': 1712592637141461,
+        'output_file': 'scroll_offsets_trace_2.pftrace'
+      },
+      {
+        'object_name': 'test_data/top_level_java_choreographer_slices-8001e73b2458e94f65a606bb558a645ba5bca553b57fe416001f6c2175675a8a',
+        'sha256sum': '8001e73b2458e94f65a606bb558a645ba5bca553b57fe416001f6c2175675a8a',
+        'size_bytes': 5323017,
+        'generation': 1671708979893186,
+        'output_file': 'top_level_java_choreographer_slices'
+      },
+      {
+        'object_name': 'test_data/chrome_page_load_all_categories_not_extended.pftrace.gz-6586e9e2bbc0c996caddb321a0374328654983733e6ffd7f4635ac07db32a493',
+        'sha256sum': '6586e9e2bbc0c996caddb321a0374328654983733e6ffd7f4635ac07db32a493',
+        'size_bytes': 1277750,
+        'generation': 1652442088902445,
+        'output_file': 'chrome_page_load_all_categories_not_extended.pftrace.gz'
+      },
+      {
+        'object_name': 'test_data/speedometer.perfetto_trace.gz-8a159b354d74a3ca0d38ce9cd071ef47de322db4261ee266bfafe04d70310529',
+        'sha256sum': '8a159b354d74a3ca0d38ce9cd071ef47de322db4261ee266bfafe04d70310529',
+        'size_bytes': 891088,
+        'generation': 1684336047660953,
+        'output_file': 'speedometer.perfetto_trace.gz'
+      },
+      {
+        'object_name': 'test_data/cpu_powerups_1.pb-70f5511ba0cd6ce1359e3cadb4d1d9301fb6e26be85158e3384b06f41418d386',
+        'sha256sum': '70f5511ba0cd6ce1359e3cadb4d1d9301fb6e26be85158e3384b06f41418d386',
+        'size_bytes': 2033064,
+        'generation': 1669652389509708,
+        'output_file': 'cpu_powerups_1.pb'
+      },
+      {
+        'object_name': 'test_data/chrome_5672_histograms.pftrace.gz-a09bd44078ac71bcfbc901b0544750e8344d0d0f6f96e220f700a5a53fa932ee',
+        'sha256sum': 'a09bd44078ac71bcfbc901b0544750e8344d0d0f6f96e220f700a5a53fa932ee',
+        'size_bytes': 1127472,
+        'generation': 1684946598804577,
+        'output_file': 'chrome_5672_histograms.pftrace.gz'
+      },
+      {
+        'object_name': 'test_data/chrome_custom_navigation_trace.gz-ff68279e3cec94076b69259d756eed181a63eaf834d8b956a7f4ba665fabf939',
+        'sha256sum': 'ff68279e3cec94076b69259d756eed181a63eaf834d8b956a7f4ba665fabf939',
+        'size_bytes': 7572484,
+        'generation': 1666713705258900,
+        'output_file': 'chrome_custom_navigation_trace.gz'
+      },
+      {
+        'object_name': 'test_data/scroll_offsets.pftrace-62101edb5204fec8bea30124f65d4e49bda0808d7b036e95f89445aaad6d8d98',
+        'sha256sum': '62101edb5204fec8bea30124f65d4e49bda0808d7b036e95f89445aaad6d8d98',
+        'size_bytes': 769741,
+        'generation': 1693402148909129,
+        'output_file': 'scroll_offsets.pftrace'
+      }
+    ],
+    'dep_type': 'gcs'
+  },
+
   'src/third_party/perl': {
       'url': Var('chromium_git') + '/chromium/deps/perl.git' + '@' + '8ef97ff3b7332e38e61b347a2fbed425a4617151',
       'condition': 'checkout_win',
@@ -5199,16 +5276,6 @@ hooks = [
     ],
   },
 
-  # Download test data for Perfetto diff tests
-  {
-    'name': 'perfetto_testdata',
-    'condition': 'host_os == "linux"',
-    'pattern': '\\.sha256',
-    'action': [ 'vpython3',
-                'src/base/tracing/test/test_data.py',
-                'download',
-    ],
-  },
   # Pull down WPR Archive files
   {
     'name': 'Fetch WPR archive files',

diff --git a/PRESUBMIT.py b/PRESUBMIT.py
@@ -3372,6 +3372,61 @@ def CheckForNewDEPSDownloadFromGoogleStorageHooks(input_api, output_api):
     return []
 
 
+def CheckEachPerfettoTestDataFileHasDepsEntry(input_api, output_api):
+    test_data_filter = lambda f: input_api.FilterSourceFile(
+        f, files_to_check=[r'^base/tracing/test/data/.*\.sha256'])
+    if not any(input_api.AffectedFiles(file_filter=test_data_filter)):
+        return []
+
+    # Find DEPS entry
+    deps_entry = []
+    for f in input_api.AffectedFiles(include_deletes=False):
+        if f.LocalPath() == 'DEPS':
+            new_deps = _ParseDeps('\n'.join(f.NewContents()))['deps']
+            deps_entry = new_deps['src/base/tracing/test/data']
+    if not deps_entry:
+        return [output_api.PresubmitError(
+            'You must update the DEPS file when you update a '
+            '.sha256 file in base/tracing/test/data'
+        )]
+
+    output = []
+    for f in input_api.AffectedFiles(file_filter=test_data_filter):
+        objects = deps_entry['objects']
+        if not f.NewContents():
+            # Deleted file so check that DEPS entry removed
+            sha256_from_file = f.OldContents()[0]
+            object_entry = next(
+                (item for item in objects if item["sha256sum"] == sha256_from_file),
+                None)
+            if object_entry:
+                output.append(output_api.PresubmitError(
+                    'You deleted %s so you must also remove the corresponding DEPS entry.'
+                    % f.LocalPath()
+                ))
+            continue
+
+        sha256_from_file = f.NewContents()[0]
+        object_entry = next(
+            (item for item in objects if item["sha256sum"] == sha256_from_file),
+            None)
+        if not object_entry:
+            output.append(output_api.PresubmitError(
+                'No corresponding DEPS entry found for %s. '
+                'Run `base/tracing/test/test_data.py get_deps --filepath %s` '
+                'to generate the DEPS entry.'
+                % (f.LocalPath(), f.LocalPath())
+            ))
+
+    if output:
+        output.append(output_api.PresubmitError(
+            'The DEPS entry for `src/base/tracing/test/data` in the DEPS file has not been '
+            'updated properly. Run `base/tracing/test/test_data.py get_all_deps` to see what '
+            'the DEPS entry should look like.'
+        ))
+    return output
+
+
 def CheckAddedDepsHaveTargetApprovals(input_api, output_api):
     """When a dependency prefixed with + is added to a DEPS file, we
     want to make sure that the change is reviewed by an OWNER of the

diff --git a/PRESUBMIT_test.py b/PRESUBMIT_test.py
@@ -225,6 +225,132 @@ def testNotUnitTestMacros(self):
         MockInputApi(), MockFile('some/path/source.cc', lines))
     self.assertEqual(0, len(errors))
 
+
+class CheckEachPerfettoTestDataFileHasDepsEntry(unittest.TestCase):
+
+  def testNewSha256FileNoDEPS(self):
+    input_api = MockInputApi()
+    input_api.files = [
+      MockFile('base/tracing/test/data/new.pftrace.sha256', []),
+    ]
+    results = PRESUBMIT.CheckEachPerfettoTestDataFileHasDepsEntry(input_api, MockOutputApi())
+    self.assertEqual(
+      ('You must update the DEPS file when you update a .sha256 file '
+       'in base/tracing/test/data'), results[0].message)
+
+  def testNewSha256FileSuccess(self):
+    input_api = MockInputApi()
+    new_deps = """deps = {
+                    'src/base/tracing/test/data': {
+                      'bucket': 'perfetto',
+                      'objects': [
+                        {
+                          'object_name': 'test_data/new.pftrace-a1b2c3f4',
+                          'sha256sum': 'a1b2c3f4',
+                          'size_bytes': 1,
+                          'generation': 1,
+                          'output_file': 'new.pftrace'
+                        },
+                      ],
+                      'dep_type': 'gcs'
+                    },
+                  }""".splitlines()
+    input_api.files = [
+      MockFile('base/tracing/test/data/new.pftrace.sha256', ['a1b2c3f4']),
+      MockFile('DEPS', new_deps),
+    ]
+    results = PRESUBMIT.CheckEachPerfettoTestDataFileHasDepsEntry(input_api, MockOutputApi())
+    self.assertEqual(0, len(results))
+
+  def testNewSha256FileWrongSha256(self):
+    input_api = MockInputApi()
+    new_deps = """deps = {
+                    'src/base/tracing/test/data': {
+                      'bucket': 'perfetto',
+                      'objects': [
+                        {
+                          'object_name': 'test_data/new.pftrace-a1b2c3f4',
+                          'sha256sum': 'wrong_hash',
+                          'size_bytes': 1,
+                          'generation': 1,
+                          'output_file': 'new.pftrace'
+                        },
+                      ],
+                      'dep_type': 'gcs'
+                    },
+                  }""".splitlines()
+    f = MockFile('base/tracing/test/data/new.pftrace.sha256', ['a1b2c3f4'])
+    input_api.files = [
+      f,
+      MockFile('DEPS', new_deps),
+    ]
+    results = PRESUBMIT.CheckEachPerfettoTestDataFileHasDepsEntry(input_api, MockOutputApi())
+    self.assertEqual((
+      'No corresponding DEPS entry found for %s. '
+      'Run `base/tracing/test/test_data.py get_deps --filepath %s` '
+      'to generate the DEPS entry.' % (f.LocalPath(), f.LocalPath())),
+      results[0].message)
+
+  def testDeleteSha256File(self):
+    input_api = MockInputApi()
+    old_deps = """deps = {
+                    'src/base/tracing/test/data': {
+                      'bucket': 'perfetto',
+                      'objects': [
+                        {
+                          'object_name': 'test_data/new.pftrace-a1b2c3f4',
+                          'sha256sum': 'a1b2c3f4',
+                          'size_bytes': 1,
+                          'generation': 1,
+                          'output_file': 'new.pftrace'
+                        },
+                      ],
+                      'dep_type': 'gcs'
+                    },
+                  }""".splitlines()
+    f = MockFile('base/tracing/test/data/new.pftrace.sha256', [], ['a1b2c3f4'], action='D')
+    input_api.files = [
+      f,
+      MockFile('DEPS', old_deps),
+    ]
+    results = PRESUBMIT.CheckEachPerfettoTestDataFileHasDepsEntry(input_api, MockOutputApi())
+    self.assertEqual((
+      'You deleted %s so you must also remove the corresponding DEPS entry.'
+      % f.LocalPath()), results[0].message)
+
+  def testDeleteSha256Success(self):
+    input_api = MockInputApi()
+    new_deps = """deps = {
+                    'src/base/tracing/test/data': {
+                      'bucket': 'perfetto',
+                      'objects': [],
+                      'dep_type': 'gcs'
+                    },
+                  }""".splitlines()
+    old_deps = """deps = {
+                    'src/base/tracing/test/data': {
+                      'bucket': 'perfetto',
+                      'objects': [
+                        {
+                          'object_name': 'test_data/new.pftrace-a1b2c3f4',
+                          'sha256sum': 'a1b2c3f4',
+                          'size_bytes': 1,
+                          'generation': 1,
+                          'output_file': 'new.pftrace'
+                        },
+                      ],
+                      'dep_type': 'gcs'
+                    },
+                  }""".splitlines()
+    f = MockFile('base/tracing/test/data/new.pftrace.sha256', [], ['a1b2c3f4'], action='D')
+    input_api.files = [
+      f,
+      MockFile('DEPS', new_deps, old_deps),
+    ]
+    results = PRESUBMIT.CheckEachPerfettoTestDataFileHasDepsEntry(input_api, MockOutputApi())
+    self.assertEqual(0, len(results))
+
+
 class CheckAddedDepsHaveTestApprovalsTest(unittest.TestCase):
 
   def calculate(self, old_include_rules, old_specific_include_rules,

diff --git a/base/tracing/test/README b/base/tracing/test/README
@@ -10,6 +10,7 @@ Currently, the diff tests only run on Linux. You can build and run the diff test
 
 ```
 $ gn gen --args='' out/Linux
+$ gclient sync
 $ autoninja -C out/Linux perfetto_diff_tests
 $ out/Linux/bin/run_perfetto_diff_tests
 ```
@@ -22,13 +23,34 @@ Your new diff test should go in `base/tracing/test/trace_processor/diff_tests/ch
 
 If you are adding a **new TestSuite**, be sure to add it to `include_index.py` so the runner knows to run this new TestSuite.
 
-If your test requires modifying or adding new test data i.e. a new trace in `base/tracing/test/data`, you will need to upload this to the GCS bucket. These trace files are too large to be checked-in to the codebase so we check-in only `.sha256` files. You can upload any new traces with this script:
+### Adding New Test Data
 
+If your test requires modifying or adding new test data i.e. a new trace in `base/tracing/test/data`, you will need to upload this to the GCS bucket.
+
+```
+$ base/tracing/test/test_data.py upload <path_to_file>
+```
+
+This script will output a deps entry which you will need to add to the [DEPS file](../../../DEPS) (see examples in the `src/base/tracing/test/data` entry).
 ```
-$ base/tracing/test/test_data.py upload --verbose
+{
+  "path": {
+    "dep_type": "gcs",
+    "bucket": "perfetto",
+    "objects": [
+      {
+        "object_name": "test_data/file_name-a1b2c3f4",
+        "sha256sum": "a1b2c3f4",
+        "size_bytes": 12345,
+        "generation": 1234567
+      }
+    ]
+  }
+}
 ```
+You will need to **manually** add this to the deps entry. After adding this entry, running `gclient sync` will download the test files in your local repo. See these [docs](https://chromium.googlesource.com/chromium/src/+/HEAD/docs/gcs_dependencies.md) for the GCS dependency workflow.
 
-This script will upload your file and generate the `.sha256` file in the `base/tracing/test/data/` directory. You can then upload this file with your CL.
+Perfetto has it's own way to download GCS objects with the [test_data](../../../third_party/perfetto/tools/test_data) script. This script uses .sha256 files to download. The `test_data.py upload` command will also generate the `file_name-a1b2c3f4.sha256` in `base/tracing/test/data`. You will need to check-in these files in the codebase so they can by rolled to Perfetto, so the tests can work there too.
 
 ## Writing TestTraceProcessor Tests