Skip to content

Commit

Permalink
Revert "[Chrome Stdlib] Use DEPS to download diff test data"
Browse files Browse the repository at this point in the history 
This reverts commit 6c8f992.

Reason for revert: b/342182426

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: I07872d3e5bdc550a6795197d396c11dadbbd3a3d
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5558666
Commit-Queue: Zachary Tan <[email protected]>
Auto-Submit: Rasika Navarange <[email protected]>
Bot-Commit: Rubber Stamper <[email protected]>
Reviewed-by: Zachary Tan <[email protected]>
Owners-Override: Zachary Tan <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1304506}
  • Loading branch information
Rasika Navarange authored and Chromium LUCI CQ committed May 22, 2024
1 parent 7ef3715 commit 303cdf2
Show file tree
Hide file tree
Showing 5 changed files with 35 additions and 414 deletions.
80 changes: 10 additions & 70 deletions DEPS
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1958,76 +1958,6 @@ deps = {
'src/third_party/perfetto':
Var('android_git') + '/platform/external/perfetto.git' + '@' + 'fe95ade50a871086b1cfeab9e8374a916bc12b71',

'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_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',
Expand Down Expand Up @@ -5269,6 +5199,16 @@ 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',
Expand Down
55 changes: 0 additions & 55 deletions PRESUBMIT.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3372,61 +3372,6 @@ 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
Expand Down
126 changes: 0 additions & 126 deletions PRESUBMIT_test.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -225,132 +225,6 @@ 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,
Expand Down
28 changes: 3 additions & 25 deletions base/tracing/test/README
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ 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
```
Expand All @@ -23,34 +22,13 @@ 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.

### 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. 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:

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).
```
{
"path": {
"dep_type": "gcs",
"bucket": "perfetto",
"objects": [
{
"object_name": "test_data/file_name-a1b2c3f4",
"sha256sum": "a1b2c3f4",
"size_bytes": 12345,
"generation": 1234567
}
]
}
}
$ base/tracing/test/test_data.py upload --verbose
```
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.

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.
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.

## Writing TestTraceProcessor Tests

Expand Down
Loading

0 comments on commit 303cdf2

Please sign in to comment.