Skip to content

Commit

Permalink
Migrateheader_guard_check to package:test. (#54811)
Browse files Browse the repository at this point in the history
Work towards flutter/flutter#133569.

I also augmented the `run_tests.py` script to support an incremental migration to `package:test`, PTAL.
  • Loading branch information
matanlurey authored Aug 28, 2024
1 parent f51b33b commit 135864f
Show file tree
Hide file tree
Showing 4 changed files with 102 additions and 51 deletions.
57 changes: 49 additions & 8 deletions testing/run_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,16 @@
# Explicitly import the parts of sys that are needed. This is to avoid using
# sys.stdout and sys.stderr directly. Instead, only the logger defined below
# should be used for output.
from sys import exit as sys_exit, platform as sys_platform
from sys import exit as sys_exit, platform as sys_platform, path as sys_path
import tempfile
import time
import typing
import xvfb

THIS_DIR = os.path.abspath(os.path.dirname(__file__))
sys_path.insert(0, os.path.join(THIS_DIR, '..', 'third_party', 'pyyaml', 'lib'))
import yaml # pylint: disable=import-error, wrong-import-position

SCRIPT_DIR = os.path.dirname(os.path.realpath(__file__))
BUILDROOT_DIR = os.path.abspath(os.path.join(os.path.realpath(__file__), '..', '..', '..'))
OUT_DIR = os.path.join(BUILDROOT_DIR, 'out')
Expand Down Expand Up @@ -65,8 +69,9 @@ def is_asan(build_dir):
return False


def run_cmd(
def run_cmd( # pylint: disable=too-many-arguments
cmd: typing.List[str],
cwd: str = None,
forbidden_output: typing.List[str] = None,
expect_failure: bool = False,
env: typing.Dict[str, str] = None,
Expand All @@ -81,12 +86,13 @@ def run_cmd(
command_string = ' '.join(cmd)

print_divider('>')
logger.info('Running command "%s"', command_string)
logger.info('Running command "%s" in "%s"', command_string, cwd)

start_time = time.time()

process = subprocess.Popen(
cmd,
cwd=cwd,
stdout=subprocess.PIPE,
stderr=subprocess.STDOUT,
env=env,
Expand Down Expand Up @@ -905,14 +911,49 @@ def gather_dart_smoke_test(build_dir, test_filter):


def gather_dart_package_tests(build_dir, package_path, extra_opts):
dart_tests = glob.glob('%s/test/*_test.dart' % package_path)
if not dart_tests:
raise Exception('No tests found for Dart package at %s' % package_path)
for dart_test_file in dart_tests:
opts = ['--disable-dart-dev', dart_test_file] + extra_opts
if uses_package_test_runner(package_path):
# Package that use package:test (dart test) do not support command-line arguments.
#
# The usual workaround is to use Platform.environment, but that would require changing
# the test execution a tiny bit. TODO(https://github.com/flutter/flutter/issues/133569).
#
# Until then, assert that no extra_opts are passed and explain the limitation.
assert not extra_opts, '%s uses package:test and cannot use CLI args' % package_path
# TODO(https://github.com/flutter/flutter/issues/154263): Restore `--disable-dart-dev`.
opts = ['test']
yield EngineExecutableTask(
build_dir, os.path.join('dart-sdk', 'bin', 'dart'), None, flags=opts, cwd=package_path
)
else:
dart_tests = glob.glob('%s/test/*_test.dart' % package_path)
if not dart_tests:
raise Exception('No tests found for Dart package at %s' % package_path)
for dart_test_file in dart_tests:
opts = ['--disable-dart-dev', dart_test_file] + extra_opts
yield EngineExecutableTask(
build_dir, os.path.join('dart-sdk', 'bin', 'dart'), None, flags=opts, cwd=package_path
)


# Returns whether the given package path should be tested with `dart test`.
#
# Inferred by a dependency on the `package:test` package in the pubspec.yaml.
def uses_package_test_runner(package):
pubspec = os.path.join(package, 'pubspec.yaml')
if not os.path.exists(pubspec):
return False
with open(pubspec, 'r') as file:
# Check if either "dependencies" or "dev_dependencies" contains "test".
data = yaml.safe_load(file)
if data is None:
return False
deps = data.get('dependencies', {})
if 'test' in deps:
return True
dev_deps = data.get('dev_dependencies', {})
if 'test' in dev_deps:
return True
return False


# Returns a list of Dart packages to test.
Expand Down
5 changes: 1 addition & 4 deletions tools/header_guard_check/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,5 @@ dependencies:
source_span: any

dev_dependencies:
async_helper: any
expect: any
litetest: any
process_fakes: any
smith: any
test: any
85 changes: 48 additions & 37 deletions tools/header_guard_check/test/header_file_test.dart
Original file line number Diff line number Diff line change
@@ -1,18 +1,21 @@
// Copyright 2013 The Flutter Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
@TestOn('vm')
library;

import 'dart:io' as io;

import 'package:header_guard_check/src/header_file.dart';
import 'package:litetest/litetest.dart';
import 'package:path/path.dart' as p;
import 'package:source_span/source_span.dart';
import 'package:test/test.dart';

Future<int> main(List<String> args) async {
Future<int> main() async {
void withTestFile(String path, String contents, void Function(io.File) fn) {
// Create a temporary file and delete it when we're done.
final io.Directory tempDir = io.Directory.systemTemp.createTempSync('header_guard_check_test');
final io.Directory tempDir =
io.Directory.systemTemp.createTempSync('header_guard_check_test');
final io.File file = io.File(p.join(tempDir.path, path));
file.writeAsStringSync(contents);
try {
Expand Down Expand Up @@ -157,7 +160,8 @@ Future<int> main(List<String> args) async {
guard: null,
pragmaOnce: null,
);
expect(headerFile.computeExpectedName(engineRoot: ''), endsWith('FOO_BAR_BAZ_H_'));
expect(headerFile.computeExpectedName(engineRoot: ''),
endsWith('FOO_BAR_BAZ_H_'));
}
});

Expand Down Expand Up @@ -240,14 +244,16 @@ Future<int> main(List<String> args) async {
withTestFile('foo.h', input, (io.File file) {
final HeaderFile headerFile = HeaderFile.parse(file.path);
expect(headerFile.fix(engineRoot: p.dirname(file.path)), isTrue);
expect(file.readAsStringSync(), <String>[
'#ifndef FLUTTER_FOO_H_',
'#define FLUTTER_FOO_H_',
'',
'// ...',
'#endif // FLUTTER_FOO_H_',
'',
].join('\n'));
expect(
file.readAsStringSync(),
<String>[
'#ifndef FLUTTER_FOO_H_',
'#define FLUTTER_FOO_H_',
'',
'// ...',
'#endif // FLUTTER_FOO_H_',
'',
].join('\n'));
});
});

Expand All @@ -261,13 +267,15 @@ Future<int> main(List<String> args) async {
withTestFile('foo.h', input, (io.File file) {
final HeaderFile headerFile = HeaderFile.parse(file.path);
expect(headerFile.fix(engineRoot: p.dirname(file.path)), isTrue);
expect(file.readAsStringSync(), <String>[
'#ifndef FLUTTER_FOO_H_',
'#define FLUTTER_FOO_H_',
'',
'#endif // FLUTTER_FOO_H_',
'',
].join('\n'));
expect(
file.readAsStringSync(),
<String>[
'#ifndef FLUTTER_FOO_H_',
'#define FLUTTER_FOO_H_',
'',
'#endif // FLUTTER_FOO_H_',
'',
].join('\n'));
});
});

Expand All @@ -287,27 +295,30 @@ Future<int> main(List<String> args) async {
withTestFile('foo.h', input, (io.File file) {
final HeaderFile headerFile = HeaderFile.parse(file.path);
expect(headerFile.fix(engineRoot: p.dirname(file.path)), isTrue);
expect(file.readAsStringSync(), <String>[
'// 1.',
'// 2.',
'// 3.',
'',
'#ifndef FLUTTER_FOO_H_',
'#define FLUTTER_FOO_H_',
'',
"#import 'flutter/shell/platform/darwin/Flutter.h'",
'',
'@protocl Flutter',
'',
'@end',
'',
'#endif // FLUTTER_FOO_H_',
'',
].join('\n'));
expect(
file.readAsStringSync(),
<String>[
'// 1.',
'// 2.',
'// 3.',
'',
'#ifndef FLUTTER_FOO_H_',
'#define FLUTTER_FOO_H_',
'',
"#import 'flutter/shell/platform/darwin/Flutter.h'",
'',
'@protocl Flutter',
'',
'@end',
'',
'#endif // FLUTTER_FOO_H_',
'',
].join('\n'));
});
});

test('does not touch a file with an existing guard and another #define', () {
test('does not touch a file with an existing guard and another #define',
() {
final String input = <String>[
'// 1.',
'// 2.',
Expand Down
6 changes: 4 additions & 2 deletions tools/header_guard_check/test/header_guard_check_test.dart
Original file line number Diff line number Diff line change
@@ -1,15 +1,17 @@
// Copyright 2013 The Flutter Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
@TestOn('vm')
library;

import 'dart:io' as io;

import 'package:engine_repo_tools/engine_repo_tools.dart';
import 'package:header_guard_check/header_guard_check.dart';
import 'package:litetest/litetest.dart';
import 'package:path/path.dart' as p;
import 'package:test/test.dart';

Future<int> main(List<String> args) async {
Future<int> main() async {
void withTestRepository(String path, void Function(io.Directory) fn) {
// Create a temporary directory and delete it when we're done.
final io.Directory tempDir = io.Directory.systemTemp.createTempSync('header_guard_check_test');
Expand Down

0 comments on commit 135864f

Please sign in to comment.