From d522f860b0b070d0af900fec4c8aeb50a63602cd Mon Sep 17 00:00:00 2001 From: Philip Pfaffe Date: Thu, 18 Apr 2024 09:09:06 +0000 Subject: [PATCH] [cxx] Use new test driver infrastructure for the cxx debugging extension This lets us remove a test runner. Drive-by: Remove goma-related flags. Bug: b:333423685 Change-Id: I104113dc3b49ab34e13c8abe83d9c2994345c04e Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/5465640 Commit-Queue: Philip Pfaffe Reviewed-by: Kim-Anh Tran Reviewed-by: Jack Franklin Auto-Submit: Philip Pfaffe --- .../cxx_debugging/e2e/MochaRootHooks.ts | 10 +- extensions/cxx_debugging/e2e/TestDriver.ts | 6 +- .../e2e/cxx-debugging-extension-helpers.ts | 7 +- extensions/cxx_debugging/e2e/runner.py | 35 ++- extensions/cxx_debugging/tools/bootstrap.py | 38 --- scripts/test/run_test_suite.py | 218 ------------------ 6 files changed, 24 insertions(+), 290 deletions(-) delete mode 100755 scripts/test/run_test_suite.py diff --git a/extensions/cxx_debugging/e2e/MochaRootHooks.ts b/extensions/cxx_debugging/e2e/MochaRootHooks.ts index 87d53ba92c..1880a7dbf8 100644 --- a/extensions/cxx_debugging/e2e/MochaRootHooks.ts +++ b/extensions/cxx_debugging/e2e/MochaRootHooks.ts @@ -13,8 +13,8 @@ import { setBrowserAndPages, setTestServerPort, } from 'test/conductor/puppeteer-state.js'; -import {click} from 'test/shared/helper'; -import {TestConfig} from 'test/TestConfig'; +import {TestConfig} from 'test/conductor/test_config.js'; +import {click} from 'test/shared/helper.js'; const EXTENSION_DIR = path.join(__dirname, '..', '..', '..', 'DevTools_CXX_Debugging.stage2', 'gen'); const DEVTOOLS_DIR = path.join(__dirname, '..', '..', '..', 'devtools-frontend', 'gen'); @@ -23,7 +23,7 @@ async function beforeAll() { setTestServerPort(Number(process.env.testServerPort)); registerHandlers(); - const executablePath = process.env['CHROME_BIN']; + const executablePath = TestConfig.chromeBinary; const defaultViewport = { width: 1280, @@ -31,9 +31,9 @@ async function beforeAll() { }; const browser = await puppeteer.launch({ - headless: !Boolean(process.env['DEBUG_TEST']) && !TestConfig.debug, + headless: !TestConfig.debug, devtools: true, - dumpio: !process.env['DEBUG_TEST'] && !TestConfig.debug, + dumpio: !TestConfig.debug, executablePath, defaultViewport, args: [ diff --git a/extensions/cxx_debugging/e2e/TestDriver.ts b/extensions/cxx_debugging/e2e/TestDriver.ts index b568f159aa..132ba7513b 100644 --- a/extensions/cxx_debugging/e2e/TestDriver.ts +++ b/extensions/cxx_debugging/e2e/TestDriver.ts @@ -4,7 +4,7 @@ import {assert} from 'chai'; import {type ElementHandle, type JSHandle} from 'puppeteer-core'; - +import {TestConfig} from 'test/conductor/test_config.js'; import { CONSOLE_TAB_SELECTOR, focusConsolePrompt, @@ -37,9 +37,9 @@ import { import {describe, it} from 'test/shared/mocha-extensions.js'; import { + type Action, loadTests, openTestSuiteResourceInSourcesPanel, - type Action, } from './cxx-debugging-extension-helpers.js'; const STEP_OVER_BUTTON = '[aria-label="Step over next function call"]'; @@ -161,7 +161,7 @@ describe('CXX Debugging Extension Test Suite', function() { } } catch (e) { console.error(e.toString()); - if (process.env['DEBUG_TEST']) { + if (TestConfig.debug) { await timeout(100000); } throw e; diff --git a/extensions/cxx_debugging/e2e/cxx-debugging-extension-helpers.ts b/extensions/cxx_debugging/e2e/cxx-debugging-extension-helpers.ts index 5e7720a01a..ee31bb4889 100644 --- a/extensions/cxx_debugging/e2e/cxx-debugging-extension-helpers.ts +++ b/extensions/cxx_debugging/e2e/cxx-debugging-extension-helpers.ts @@ -56,11 +56,6 @@ export function getTestsuiteResourcesPath() { } export function loadTests() { - const TEST_SUITE = process.env['TEST_SUITE']; - if (!TEST_SUITE) { - return []; - } - - const tests = JSON.parse(fs.readFileSync(path.join(TEST_SUITE, 'tests.json')).toString()); + const tests = JSON.parse(fs.readFileSync(path.join(__dirname, 'tests.json')).toString()); return tests as TestSpec[]; } diff --git a/extensions/cxx_debugging/e2e/runner.py b/extensions/cxx_debugging/e2e/runner.py index e14534f73b..64358cac2a 100755 --- a/extensions/cxx_debugging/e2e/runner.py +++ b/extensions/cxx_debugging/e2e/runner.py @@ -14,7 +14,6 @@ import threading import yaml - def repo_path(*paths): RootDirectory = os.path.dirname( os.path.dirname( @@ -108,6 +107,9 @@ def list_tests(path): if f.endswith('.yaml')) +NODE = repo_path('//third_party/node/node.py') + + class Test(object): def __init__(self, build_root, path): output_directory = repo_path(build_root, @@ -238,9 +240,6 @@ class Compile(RunnerCommand): Command = 'compile' Help = 'Compile the tests and the dependencies' - def _register_options(self, parser): - parser.add_argument('--goma') - def __call__(self, options): self.build_extension(options) self.build_devtools(options.build_root, options.verbose) @@ -275,10 +274,9 @@ class Compile(RunnerCommand): ninja(build_root, 'test_suite', verbose) def build_driver(self, build_root, verbose): - node = repo_path('//third_party/node/node.py') tsc = repo_path('//node_modules/typescript/bin/tsc') run_process(sys.executable, - node, + NODE, '--output', tsc, '-p', @@ -288,7 +286,7 @@ class Compile(RunnerCommand): verbose=verbose) def build_extension(self, options): - args = ['-goma', options.goma] if options.goma else ['-no-goma'] + args = [] if options.release or options.release_version: args.append('-release-version') args.append(options.release_version or 0) @@ -386,9 +384,8 @@ class Init(RunnerCommand): 0 if options.debug else 120000 } with open(repo_path(test_suite_dir, '.mocharc.js'), 'w') as mocharc: - mocharc.write( - 'process.env.TEST_SERVER_TYPE = "hosted-mode";\nmodule.exports = {};' - .format(json.dumps(mocha_spec, indent=2))) + mocharc.write('module.exports = {};'.format( + json.dumps(mocha_spec, indent=2))) with open(repo_path(test_suite_dir, 'tests.json'), 'w') as tests_file: tests = [Init.generate_tests(t, test_suite_dir) for t in tests] @@ -492,7 +489,6 @@ class Run(Init): return init if options.compile: - options.goma = None Compile()(options) else: ninja(options.build_root, 'test_suite', options.verbose) @@ -506,20 +502,19 @@ class Run(Init): repo_path(options.build_root, get_artifact_dir('devtools-frontend'), 'gen'))), - 'TEST_SUITE': - repo_path(options.build_root, get_artifact_dir('test_suite')), } + args = ['--'] if options.debug: - env['DEBUG_TEST'] = '1' + args.append('--debug') run_process(sys.executable, - repo_path('//scripts/test/run_test_suite.py'), - '--chrome-features=WebAssemblySimd', - '--chrome-features=SharedArrayBuffer', - '--test-suite', + NODE, + '--output', + repo_path('//node_modules/mocha/bin/mocha'), + '--config', repo_path(options.build_root, - get_artifact_dir('test_suite')), + get_artifact_dir('test_suite'), '.mocharc.js'), + *args, env=env, - cwd=repo_path(options.build_root), verbose=options.verbose or options.debug) diff --git a/extensions/cxx_debugging/tools/bootstrap.py b/extensions/cxx_debugging/tools/bootstrap.py index feedff8149..0456ce62a2 100755 --- a/extensions/cxx_debugging/tools/bootstrap.py +++ b/extensions/cxx_debugging/tools/bootstrap.py @@ -42,19 +42,6 @@ def exec_extension(): return ".exe" if is_windows() else "" -def get_gomacc(OPTIONS): - if OPTIONS.no_goma: - return None - if OPTIONS.goma: - return OPTIONS.goma - else: - goma_ctl = shutil.which('goma_ctl') - if goma_ctl: - depot_tools_dir = os.path.dirname(goma_ctl) - gomacc = os.path.join(depot_tools_dir, '.cipd_bin', 'gomacc') - return gomacc - return None - def call(cmd, verbose=False, **kwargs): if verbose: @@ -89,10 +76,6 @@ def stage1(sysroot_dir, source_dir, OPTIONS): cmake_args.append('-DCMAKE_C_COMPILER={}'.format(OPTIONS.cc)) if OPTIONS.cxx: cmake_args.append('-DCMAKE_CXX_COMPILER={}'.format(OPTIONS.cxx)) - gomacc = get_gomacc(OPTIONS) - if gomacc: - cmake_args.extend(('-DCMAKE_CXX_COMPILER_LAUNCHER={}'.format(gomacc), - '-DCMAKE_C_COMPILER_LAUNCHER={}'.format(gomacc))) maybe_cmake(binary_dir, cmake_args, OPTIONS.verbose) @@ -200,16 +183,8 @@ def stage2(source_dir, stage1_dir, OPTIONS): maybe_cmake(binary_dir, cmake_args, OPTIONS.verbose) - gomacc = get_gomacc(OPTIONS) num_cores = os.cpu_count() env = os.environ.copy() - if gomacc: - env['EM_COMPILER_WRAPPER'] = gomacc - # autoninja does not recognize the environment variable, so set the - # jobs manually - num_cores *= int(os.environ.get('NINJA_CORE_MULTIPLIER', '40')) - else: - num_cores += 2 if not OPTIONS.no_check: call(['ninja', '-j%d' % num_cores, 'all', 'check-extension'], @@ -267,9 +242,6 @@ def script_main(args): parser.add_argument('-cmake', default=shutil.which('cmake', path=cmake_dir), help='Path to the cmake configure tool.') - parser.add_argument('-goma', - default=shutil.which('gomacc'), - help='Path to the goma compiler launcher (gomacc).') parser.add_argument('-cc', default=shutil.which('clang', path=clang_dir), help='The C compiler.') @@ -279,7 +251,6 @@ def script_main(args): parser.add_argument('-extension-source', default=source_dir, help='Path to alternate repo for source.') - parser.add_argument('-check', action='store_true') # TODO(pfaffe) remove parser.add_argument('-verbose', action='store_true') parser.add_argument('-stage1', help='Path to a pre-built stage 1') parser.add_argument('-static', @@ -291,10 +262,6 @@ def script_main(args): default=True, dest='static', help='Link the first stage dynamically.') - parser.add_argument('-sysroot', default='') # TODO(pfaffe) remove - parser.add_argument('-no-goma', - action='store_true', - help='Build without goma.') parser.add_argument('-no-sysroot', action='store_true', help='Disable sysroot.') @@ -352,11 +319,6 @@ def script_main(args): sys.stderr.write('-infra overrides -no-sysroot') OPTIONS.no_sysroot = False - if OPTIONS.sysroot: - sys.stderr.write('The -sysroot option is deprecated and has no effect') - if OPTIONS.check: - sys.stderr.write('The -check option is deprecated and has no effect') - if OPTIONS.stage1: stage1_dir = OPTIONS.stage1 else: diff --git a/scripts/test/run_test_suite.py b/scripts/test/run_test_suite.py deleted file mode 100755 index cc7ec806a6..0000000000 --- a/scripts/test/run_test_suite.py +++ /dev/null @@ -1,218 +0,0 @@ -#!/usr/bin/env vpython3 -# -# Copyright 2020 The Chromium Authors. All rights reserved. -# Use of this source code is governed by a BSD-style license that can be -# found in the LICENSE file. -""" -Run tests on a pinned version of chrome. - -DEPRECATED: please use run_test_suite.js instead. -""" - -import argparse -import os -import sys - -ROOT_DIRECTORY = os.path.join(os.path.dirname(os.path.abspath(__file__)), '..', - '..') -scripts_path = os.path.join(ROOT_DIRECTORY, 'scripts') -sys.path.append(scripts_path) - -import devtools_paths -import test_helpers - - -def parse_options(cli_args): - parser = argparse.ArgumentParser(description='Run tests') - parser.add_argument('--chrome-binary', - dest='chrome_binary', - help='path to Chromium binary') - parser.add_argument( - '--test-suite', - dest='test_suite', - help= - 'test suite name. DEPRECATED: please use --test-suite-path instead.') - parser.add_argument( - '--test-suite-path', - dest='test_suite_path', - help= - 'path to test suite, starting from the out/TARGET directory. Should use Linux path separators.' - ) - parser.add_argument('--test-file', - dest='test_file', - help='an absolute path for the file to test') - parser.add_argument( - '--target', - '-t', - default='Default', - dest='target', - help='The name of the Ninja output directory. Defaults to "Default"') - parser.add_argument( - '--chrome-features', - dest='chrome_features', - help= - 'comma separated list of strings passed to --enable-features on the chromium commandline' - ) - parser.add_argument( - '--jobs', - default='1', - dest='jobs', - help= - 'The number of parallel runners to use (if supported). Defaults to 1') - parser.add_argument('--cwd', - dest='cwd', - help='Path to the directory containing the out dir', - default=devtools_paths.devtools_root_path()) - parser.add_argument( - '--node_modules-path', - dest='node_modules_path', - help= - 'Path to the node_modules directory for Node to use. Will use Node defaults if not set.', - default=None) - parser.add_argument('test_patterns', nargs='*') - return parser.parse_args(cli_args) - - -def run_tests(chrome_binary, - chrome_features, - test_suite_path, - test_suite, - jobs, - target, - cwd=None, - node_modules_path=None, - test_patterns=None): - env = os.environ.copy() - env['CHROME_BIN'] = chrome_binary - if chrome_features: - env['CHROME_FEATURES'] = chrome_features - - if test_patterns: - env['TEST_PATTERNS'] = ';'.join(test_patterns) - - if jobs: - env['JOBS'] = jobs - - if target: - env['TARGET'] = target - - if node_modules_path is not None: - # Node requires the path to be absolute - env['NODE_PATH'] = os.path.abspath(node_modules_path) - - if not cwd: - cwd = devtools_paths.devtools_root_path() - - exec_command = [devtools_paths.node_path()] - - if 'DEBUG_TEST' in env: - exec_command.append('--inspect') - - exec_command = exec_command + [ - devtools_paths.mocha_path(), - '--config', - os.path.join(test_suite_path, '.mocharc.js'), - ] - - exit_code = test_helpers.popen(exec_command, cwd=cwd, env=env) - if exit_code != 0: - return True - - return False - - -def run_test(): - print( - "DEPRECATED: run_test_suite.py is deprecated and will be removed in the future.\nPlease use run_test_suite.js which is newer and more robust with handling paths." - ) - OPTIONS = parse_options(sys.argv[1:]) - is_cygwin = sys.platform == 'cygwin' - chrome_binary = None - test_suite = None - chrome_features = None - - # Default to the downloaded / pinned Chromium binary - downloaded_chrome_binary = devtools_paths.downloaded_chrome_binary_path() - if test_helpers.check_chrome_binary(downloaded_chrome_binary): - chrome_binary = downloaded_chrome_binary - - # Override with the arg value if provided. - if OPTIONS.chrome_binary: - chrome_binary = OPTIONS.chrome_binary - if not test_helpers.check_chrome_binary(chrome_binary): - print('Unable to find a Chrome binary at \'%s\'' % chrome_binary) - sys.exit(1) - - if OPTIONS.chrome_features: - chrome_features = OPTIONS.chrome_features - - if (chrome_binary is None): - print('Unable to run, no Chrome binary provided') - sys.exit(1) - - if OPTIONS.jobs: - jobs = OPTIONS.jobs - - test_file = OPTIONS.test_file - test_patterns = OPTIONS.test_patterns - if test_file: - test_patterns.append(test_file) - - print('Using Chromium binary ({}{})\n'.format( - chrome_binary, ' ' + chrome_features if chrome_features else '')) - print('Using target (%s)\n' % OPTIONS.target) - - if test_file is not None: - print( - 'The test_file argument is obsolete, just pass the filename as positional argument' - ) - if test_patterns: - print('Testing file(s) (%s)' % ', '.join(test_patterns)) - - cwd = OPTIONS.cwd - target = OPTIONS.target - node_modules_path = OPTIONS.node_modules_path - - print('Running tests from %s\n' % cwd) - - test_suite_path_input = OPTIONS.test_suite_path - test_suite = OPTIONS.test_suite - test_suite_parts = None - if test_suite: - # test-suite is deprecated and will be removed, but we support it for now to not break the bots until their recipes are updated. - test_suite_parts = ['gen', 'test', test_suite] - elif test_suite_path_input: - # We take the input with Linux path separators, but need to split and join to make sure this works on Windows. - test_suite_parts = test_suite_path_input.split('/') - else: - print( - 'Unable to run, require one of --test-suite or --test-suite-path to be provided.' - ) - sys.exit(1) - - print('Using Test Suite (%s)\n' % os.path.join(*test_suite_parts)) - - test_suite_path = os.path.join(os.path.abspath(cwd), 'out', OPTIONS.target, - *test_suite_parts) - - errors_found = False - try: - errors_found = run_tests(chrome_binary, - chrome_features, - test_suite_path, - test_suite, - jobs, - target, - cwd, - node_modules_path, - test_patterns=test_patterns) - except Exception as err: - print(err) - - if errors_found: - print('ERRORS DETECTED') - sys.exit(1) - - -if __name__ == '__main__': - run_test()