Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Commit 135864f

Browse files
authored
Migrateheader_guard_check to package:test. (#54811)
Work towards flutter/flutter#133569. I also augmented the `run_tests.py` script to support an incremental migration to `package:test`, PTAL.
1 parent f51b33b commit 135864f

File tree

4 files changed

+102
-51
lines changed

4 files changed

+102
-51
lines changed

testing/run_tests.py

Lines changed: 49 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -23,12 +23,16 @@
2323
# Explicitly import the parts of sys that are needed. This is to avoid using
2424
# sys.stdout and sys.stderr directly. Instead, only the logger defined below
2525
# should be used for output.
26-
from sys import exit as sys_exit, platform as sys_platform
26+
from sys import exit as sys_exit, platform as sys_platform, path as sys_path
2727
import tempfile
2828
import time
2929
import typing
3030
import xvfb
3131

32+
THIS_DIR = os.path.abspath(os.path.dirname(__file__))
33+
sys_path.insert(0, os.path.join(THIS_DIR, '..', 'third_party', 'pyyaml', 'lib'))
34+
import yaml # pylint: disable=import-error, wrong-import-position
35+
3236
SCRIPT_DIR = os.path.dirname(os.path.realpath(__file__))
3337
BUILDROOT_DIR = os.path.abspath(os.path.join(os.path.realpath(__file__), '..', '..', '..'))
3438
OUT_DIR = os.path.join(BUILDROOT_DIR, 'out')
@@ -65,8 +69,9 @@ def is_asan(build_dir):
6569
return False
6670

6771

68-
def run_cmd(
72+
def run_cmd( # pylint: disable=too-many-arguments
6973
cmd: typing.List[str],
74+
cwd: str = None,
7075
forbidden_output: typing.List[str] = None,
7176
expect_failure: bool = False,
7277
env: typing.Dict[str, str] = None,
@@ -81,12 +86,13 @@ def run_cmd(
8186
command_string = ' '.join(cmd)
8287

8388
print_divider('>')
84-
logger.info('Running command "%s"', command_string)
89+
logger.info('Running command "%s" in "%s"', command_string, cwd)
8590

8691
start_time = time.time()
8792

8893
process = subprocess.Popen(
8994
cmd,
95+
cwd=cwd,
9096
stdout=subprocess.PIPE,
9197
stderr=subprocess.STDOUT,
9298
env=env,
@@ -905,14 +911,49 @@ def gather_dart_smoke_test(build_dir, test_filter):
905911

906912

907913
def gather_dart_package_tests(build_dir, package_path, extra_opts):
908-
dart_tests = glob.glob('%s/test/*_test.dart' % package_path)
909-
if not dart_tests:
910-
raise Exception('No tests found for Dart package at %s' % package_path)
911-
for dart_test_file in dart_tests:
912-
opts = ['--disable-dart-dev', dart_test_file] + extra_opts
914+
if uses_package_test_runner(package_path):
915+
# Package that use package:test (dart test) do not support command-line arguments.
916+
#
917+
# The usual workaround is to use Platform.environment, but that would require changing
918+
# the test execution a tiny bit. TODO(https://github.com/flutter/flutter/issues/133569).
919+
#
920+
# Until then, assert that no extra_opts are passed and explain the limitation.
921+
assert not extra_opts, '%s uses package:test and cannot use CLI args' % package_path
922+
# TODO(https://github.com/flutter/flutter/issues/154263): Restore `--disable-dart-dev`.
923+
opts = ['test']
913924
yield EngineExecutableTask(
914925
build_dir, os.path.join('dart-sdk', 'bin', 'dart'), None, flags=opts, cwd=package_path
915926
)
927+
else:
928+
dart_tests = glob.glob('%s/test/*_test.dart' % package_path)
929+
if not dart_tests:
930+
raise Exception('No tests found for Dart package at %s' % package_path)
931+
for dart_test_file in dart_tests:
932+
opts = ['--disable-dart-dev', dart_test_file] + extra_opts
933+
yield EngineExecutableTask(
934+
build_dir, os.path.join('dart-sdk', 'bin', 'dart'), None, flags=opts, cwd=package_path
935+
)
936+
937+
938+
# Returns whether the given package path should be tested with `dart test`.
939+
#
940+
# Inferred by a dependency on the `package:test` package in the pubspec.yaml.
941+
def uses_package_test_runner(package):
942+
pubspec = os.path.join(package, 'pubspec.yaml')
943+
if not os.path.exists(pubspec):
944+
return False
945+
with open(pubspec, 'r') as file:
946+
# Check if either "dependencies" or "dev_dependencies" contains "test".
947+
data = yaml.safe_load(file)
948+
if data is None:
949+
return False
950+
deps = data.get('dependencies', {})
951+
if 'test' in deps:
952+
return True
953+
dev_deps = data.get('dev_dependencies', {})
954+
if 'test' in dev_deps:
955+
return True
956+
return False
916957

917958

918959
# Returns a list of Dart packages to test.

tools/header_guard_check/pubspec.yaml

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,5 @@ dependencies:
2121
source_span: any
2222

2323
dev_dependencies:
24-
async_helper: any
25-
expect: any
26-
litetest: any
2724
process_fakes: any
28-
smith: any
25+
test: any

tools/header_guard_check/test/header_file_test.dart

Lines changed: 48 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,21 @@
11
// Copyright 2013 The Flutter Authors. All rights reserved.
22
// Use of this source code is governed by a BSD-style license that can be
33
// found in the LICENSE file.
4+
@TestOn('vm')
5+
library;
46

57
import 'dart:io' as io;
68

79
import 'package:header_guard_check/src/header_file.dart';
8-
import 'package:litetest/litetest.dart';
910
import 'package:path/path.dart' as p;
1011
import 'package:source_span/source_span.dart';
12+
import 'package:test/test.dart';
1113

12-
Future<int> main(List<String> args) async {
14+
Future<int> main() async {
1315
void withTestFile(String path, String contents, void Function(io.File) fn) {
1416
// Create a temporary file and delete it when we're done.
15-
final io.Directory tempDir = io.Directory.systemTemp.createTempSync('header_guard_check_test');
17+
final io.Directory tempDir =
18+
io.Directory.systemTemp.createTempSync('header_guard_check_test');
1619
final io.File file = io.File(p.join(tempDir.path, path));
1720
file.writeAsStringSync(contents);
1821
try {
@@ -157,7 +160,8 @@ Future<int> main(List<String> args) async {
157160
guard: null,
158161
pragmaOnce: null,
159162
);
160-
expect(headerFile.computeExpectedName(engineRoot: ''), endsWith('FOO_BAR_BAZ_H_'));
163+
expect(headerFile.computeExpectedName(engineRoot: ''),
164+
endsWith('FOO_BAR_BAZ_H_'));
161165
}
162166
});
163167

@@ -240,14 +244,16 @@ Future<int> main(List<String> args) async {
240244
withTestFile('foo.h', input, (io.File file) {
241245
final HeaderFile headerFile = HeaderFile.parse(file.path);
242246
expect(headerFile.fix(engineRoot: p.dirname(file.path)), isTrue);
243-
expect(file.readAsStringSync(), <String>[
244-
'#ifndef FLUTTER_FOO_H_',
245-
'#define FLUTTER_FOO_H_',
246-
'',
247-
'// ...',
248-
'#endif // FLUTTER_FOO_H_',
249-
'',
250-
].join('\n'));
247+
expect(
248+
file.readAsStringSync(),
249+
<String>[
250+
'#ifndef FLUTTER_FOO_H_',
251+
'#define FLUTTER_FOO_H_',
252+
'',
253+
'// ...',
254+
'#endif // FLUTTER_FOO_H_',
255+
'',
256+
].join('\n'));
251257
});
252258
});
253259

@@ -261,13 +267,15 @@ Future<int> main(List<String> args) async {
261267
withTestFile('foo.h', input, (io.File file) {
262268
final HeaderFile headerFile = HeaderFile.parse(file.path);
263269
expect(headerFile.fix(engineRoot: p.dirname(file.path)), isTrue);
264-
expect(file.readAsStringSync(), <String>[
265-
'#ifndef FLUTTER_FOO_H_',
266-
'#define FLUTTER_FOO_H_',
267-
'',
268-
'#endif // FLUTTER_FOO_H_',
269-
'',
270-
].join('\n'));
270+
expect(
271+
file.readAsStringSync(),
272+
<String>[
273+
'#ifndef FLUTTER_FOO_H_',
274+
'#define FLUTTER_FOO_H_',
275+
'',
276+
'#endif // FLUTTER_FOO_H_',
277+
'',
278+
].join('\n'));
271279
});
272280
});
273281

@@ -287,27 +295,30 @@ Future<int> main(List<String> args) async {
287295
withTestFile('foo.h', input, (io.File file) {
288296
final HeaderFile headerFile = HeaderFile.parse(file.path);
289297
expect(headerFile.fix(engineRoot: p.dirname(file.path)), isTrue);
290-
expect(file.readAsStringSync(), <String>[
291-
'// 1.',
292-
'// 2.',
293-
'// 3.',
294-
'',
295-
'#ifndef FLUTTER_FOO_H_',
296-
'#define FLUTTER_FOO_H_',
297-
'',
298-
"#import 'flutter/shell/platform/darwin/Flutter.h'",
299-
'',
300-
'@protocl Flutter',
301-
'',
302-
'@end',
303-
'',
304-
'#endif // FLUTTER_FOO_H_',
305-
'',
306-
].join('\n'));
298+
expect(
299+
file.readAsStringSync(),
300+
<String>[
301+
'// 1.',
302+
'// 2.',
303+
'// 3.',
304+
'',
305+
'#ifndef FLUTTER_FOO_H_',
306+
'#define FLUTTER_FOO_H_',
307+
'',
308+
"#import 'flutter/shell/platform/darwin/Flutter.h'",
309+
'',
310+
'@protocl Flutter',
311+
'',
312+
'@end',
313+
'',
314+
'#endif // FLUTTER_FOO_H_',
315+
'',
316+
].join('\n'));
307317
});
308318
});
309319

310-
test('does not touch a file with an existing guard and another #define', () {
320+
test('does not touch a file with an existing guard and another #define',
321+
() {
311322
final String input = <String>[
312323
'// 1.',
313324
'// 2.',

tools/header_guard_check/test/header_guard_check_test.dart

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,17 @@
11
// Copyright 2013 The Flutter Authors. All rights reserved.
22
// Use of this source code is governed by a BSD-style license that can be
33
// found in the LICENSE file.
4+
@TestOn('vm')
5+
library;
46

57
import 'dart:io' as io;
68

79
import 'package:engine_repo_tools/engine_repo_tools.dart';
810
import 'package:header_guard_check/header_guard_check.dart';
9-
import 'package:litetest/litetest.dart';
1011
import 'package:path/path.dart' as p;
12+
import 'package:test/test.dart';
1113

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

0 commit comments

Comments
 (0)