Skip to content

Commit 7dbb51d

Browse files
[native_toolchain_c] Disallow test skips on CI (#3438)
Co-authored-by: Liam Appelbe <liama@google.com>
1 parent 3aa280f commit 7dbb51d

8 files changed

Lines changed: 72 additions & 30 deletions

File tree

pkgs/native_test_helpers/lib/native_test_helpers.dart

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,4 +3,5 @@
33
// BSD-style license that can be found in the LICENSE file.
44

55
export 'src/find_package_root.dart';
6+
export 'src/skip_local.dart';
67
export 'src/yaml_to_json.dart';
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
// Copyright (c) 2026, the Dart project authors. Please see the AUTHORS file
2+
// for details. All rights reserved. Use of this source code is governed by a
3+
// BSD-style license that can be found in the LICENSE file.
4+
5+
import 'dart:io';
6+
7+
/// Returns a skip reason if the [condition] is met and we are running locally.
8+
///
9+
/// Returns `null` (forcing the test or expectation to run) if running on CI
10+
/// (detected by the `GITHUB_ACTIONS` environment variable).
11+
// ignore: avoid_positional_boolean_parameters
12+
Object? skipLocal(bool condition, String reason) {
13+
// Disallow skips when running in CI environments, where all toolchains and
14+
// dependencies must be present.
15+
final isGithubActions = Platform.environment.containsKey('GITHUB_ACTIONS');
16+
17+
// The Dart SDK's internal CI infrastructure (Buildbot / Swarming).
18+
final isDartSdkCI =
19+
Platform.environment.containsKey('BUILDBOT_BUILDERNAME') ||
20+
Platform.environment.containsKey('SWARMING_TASK_ID');
21+
22+
final isCI = isGithubActions || isDartSdkCI;
23+
if (isCI) return null;
24+
return condition ? reason : null;
25+
}

pkgs/native_toolchain_c/test/cbuilder/cbuilder_cross_macos_host_test.dart

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import 'dart:io';
1313
import 'package:code_assets/code_assets.dart';
1414
import 'package:hooks/hooks.dart';
1515
import 'package:logging/logging.dart';
16+
import 'package:native_test_helpers/native_test_helpers.dart';
1617
import 'package:native_toolchain_c/native_toolchain_c.dart';
1718
import 'package:native_toolchain_c/src/native_toolchain/apple_clang.dart';
1819
import 'package:native_toolchain_c/src/native_toolchain/clang.dart';
@@ -202,7 +203,10 @@ void main() async {
202203
);
203204
await expectMachineArchitecture(libUri, arch, os);
204205
},
205-
skip: os == OS.linux && !lldAvailable ? 'ld.lld not available' : null,
206+
skip: skipLocal(
207+
os == OS.linux && !lldAvailable,
208+
'ld.lld not available',
209+
),
206210
);
207211
}
208212

pkgs/native_toolchain_c/test/clinker/objects_helper.dart

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import 'dart:io';
66

77
import 'package:code_assets/code_assets.dart';
88
import 'package:hooks/hooks.dart';
9+
import 'package:native_test_helpers/native_test_helpers.dart';
910
import 'package:native_toolchain_c/native_toolchain_c.dart';
1011
import 'package:test/test.dart';
1112

@@ -99,7 +100,10 @@ void runObjectsTests(
99100
expect(
100101
symbols,
101102
stringContainsInOrder(['my_func', 'my_other_func']),
102-
skip: symbols == null ? 'tool to extract symbols unavailable' : false,
103+
skip: skipLocal(
104+
symbols == null,
105+
'tool to extract symbols unavailable',
106+
),
103107
);
104108
});
105109
}

pkgs/native_toolchain_c/test/clinker/rust_test.dart

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import 'dart:io';
66

77
import 'package:code_assets/code_assets.dart';
88
import 'package:hooks/hooks.dart';
9+
import 'package:native_test_helpers/native_test_helpers.dart';
910
import 'package:native_toolchain_c/native_toolchain_c.dart';
1011
import 'package:test/test.dart';
1112

@@ -67,11 +68,12 @@ Future<void> main() async {
6768
targetArchitecture,
6869
targetOS,
6970
);
70-
final skipReason = symbols == null
71-
? 'tool to extract symbols unavailable'
72-
: false;
73-
expect(symbols, contains('my_other_func'), skip: skipReason);
74-
expect(symbols, isNot(contains('my_func')), skip: skipReason);
71+
final skip = skipLocal(
72+
symbols == null,
73+
'tool to extract symbols unavailable',
74+
);
75+
expect(symbols, contains('my_other_func'), skip: skip);
76+
expect(symbols, isNot(contains('my_func')), skip: skip);
7577
});
7678

7779
test('link rust binary without script keeps symbols', () async {
@@ -91,11 +93,12 @@ Future<void> main() async {
9193
targetArchitecture,
9294
targetOS,
9395
);
94-
final skipReason = symbols == null
95-
? 'tool to extract symbols unavailable'
96-
: false;
97-
expect(symbols, contains('my_other_func'), skip: skipReason);
98-
expect(symbols, contains('my_func'), skip: skipReason);
96+
final skip = skipLocal(
97+
symbols == null,
98+
'tool to extract symbols unavailable',
99+
);
100+
expect(symbols, contains('my_other_func'), skip: skip);
101+
expect(symbols, contains('my_func'), skip: skip);
99102
});
100103
}
101104

pkgs/native_toolchain_c/test/clinker/treeshake_helper.dart

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import 'dart:io';
66

77
import 'package:code_assets/code_assets.dart';
88
import 'package:hooks/hooks.dart';
9+
import 'package:native_test_helpers/native_test_helpers.dart';
910
import 'package:native_toolchain_c/native_toolchain_c.dart';
1011
import 'package:test/test.dart';
1112

@@ -151,15 +152,16 @@ void runTreeshakeTests(
151152
);
152153

153154
final symbols = await readSymbols(asset, targetOS);
154-
final skipReason = symbols == null
155-
? 'tool to extract symbols unavailable'
156-
: false;
155+
final skip = skipLocal(
156+
symbols == null,
157+
'tool to extract symbols unavailable',
158+
);
157159
if (clinker.linker != linkerAutoKeepAll) {
158-
expect(symbols, contains('my_other_func'), skip: skipReason);
159-
expect(symbols, isNot(contains('my_func')), skip: skipReason);
160+
expect(symbols, contains('my_other_func'), skip: skip);
161+
expect(symbols, isNot(contains('my_func')), skip: skip);
160162
} else {
161-
expect(symbols, contains('my_other_func'), skip: skipReason);
162-
expect(symbols, contains('my_func'), skip: skipReason);
163+
expect(symbols, contains('my_other_func'), skip: skip);
164+
expect(symbols, contains('my_func'), skip: skip);
163165
}
164166

165167
final sizeInBytes = await File.fromUri(asset.file!).length();

pkgs/native_toolchain_c/test/clinker/windows_module_definition_helper.dart

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import 'dart:io';
66

77
import 'package:code_assets/code_assets.dart';
88
import 'package:hooks/hooks.dart';
9+
import 'package:native_test_helpers/native_test_helpers.dart';
910
import 'package:native_toolchain_c/native_toolchain_c.dart';
1011
import 'package:test/test.dart';
1112

@@ -69,14 +70,15 @@ void runWindowsModuleDefinitionTests(List<Architecture> architectures) {
6970
final asset = codeAssets.first;
7071
expect(asset, isA<CodeAsset>());
7172
final symbols = await readSymbols(asset, targetOS);
72-
final skipReason = symbols == null
73-
? 'tool to extract symbols unavailable'
74-
: false;
75-
expect(symbols, contains('my_func'), skip: skipReason);
73+
final skip = skipLocal(
74+
symbols == null,
75+
'tool to extract symbols unavailable',
76+
);
77+
expect(symbols, contains('my_func'), skip: skip);
7678
// Module Definition file causes my_unexported_func to be exported even
7779
// though it wasn't marked for export.
78-
expect(symbols, contains('my_unexported_func'), skip: skipReason);
79-
expect(symbols, isNot(contains('my_other_func')), skip: skipReason);
80+
expect(symbols, contains('my_unexported_func'), skip: skip);
81+
expect(symbols, isNot(contains('my_other_func')), skip: skip);
8082
},
8183
);
8284
}

pkgs/native_toolchain_c/test/helpers.dart

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -446,14 +446,15 @@ Future<void> expectMachineArchitecture(
446446
);
447447
} else if (Platform.isWindows && targetOS == OS.windows) {
448448
final result = await _runDumpbin(['/HEADERS'], libUri);
449-
final skipReason = result == null
450-
? 'tool to determine binary architecture unavailable'
451-
: false;
452-
expect(result?.exitCode, 0, skip: skipReason);
449+
final skip = skipLocal(
450+
result == null,
451+
'tool to determine binary architecture unavailable',
452+
);
453+
expect(result?.exitCode, 0, skip: skip);
453454
final machine = result?.stdout
454455
.split('\n')
455456
.firstWhere((e) => e.contains('machine'));
456-
expect(machine, contains(dumpbinFileFormat[targetArch]), skip: skipReason);
457+
expect(machine, contains(dumpbinFileFormat[targetArch]), skip: skip);
457458
}
458459
}
459460

0 commit comments

Comments
 (0)