[native_test_helpers] Allow skipping dumpbin on Dart SDK CI (dart-lang#3442)

Author: dcharkes

pkgs/native_test_helpers/lib/src/skip_local.dart

@@ -8,18 +8,25 @@ import 'dart:io';
 ///
 /// Returns `null` (forcing the test or expectation to run) if running on CI
 /// (detected by the `GITHUB_ACTIONS` environment variable).
-// ignore: avoid_positional_boolean_parameters
-Object? skipLocal(bool condition, String reason) {
+Object? skipLocal(
+  // ignore: avoid_positional_boolean_parameters
+  bool condition,
+  String reason, {
+  bool skipDartSdkCI = false,
+}) {
   // Disallow skips when running in CI environments, where all toolchains and
   // dependencies must be present.
   final isGithubActions = Platform.environment.containsKey('GITHUB_ACTIONS');
 
-  // The Dart SDK's internal CI infrastructure (Buildbot / Swarming).
+  // Note: The Dart SDK's internal CI infrastructure (Buildbot / Swarming)
+  // does not always have all tools (e.g. `dumpbin` is missing on Windows).
+  // We allow skipping there only when `allowSkipDartSdk` is true.
   final isDartSdkCI =
       Platform.environment.containsKey('BUILDBOT_BUILDERNAME') ||
       Platform.environment.containsKey('SWARMING_TASK_ID');
 
-  final isCI = isGithubActions || isDartSdkCI;
-  if (isCI) return null;
+  if (isGithubActions) return null;
+  if (isDartSdkCI && !skipDartSdkCI) return null;
+
   return condition ? reason : null;
 }

pkgs/native_toolchain_c/test/clinker/objects_helper.dart

@@ -6,7 +6,6 @@ import 'dart:io';
 
 import 'package:code_assets/code_assets.dart';
 import 'package:hooks/hooks.dart';
-import 'package:native_test_helpers/native_test_helpers.dart';
 import 'package:native_toolchain_c/native_toolchain_c.dart';
 import 'package:test/test.dart';
 
@@ -96,14 +95,10 @@ void runObjectsTests(
       expect(codeAssets, hasLength(1));
       final asset = codeAssets.first;
       expect(asset, isA<CodeAsset>());
-      final symbols = await readSymbols(asset, targetOS);
-      expect(
-        symbols,
-        stringContainsInOrder(['my_func', 'my_other_func']),
-        skip: skipLocal(
-          symbols == null,
-          'tool to extract symbols unavailable',
-        ),
+      await expectSymbols(
+        asset: asset,
+        targetOS: targetOS,
+        symbols: ['my_func', 'my_other_func'],
       );
     });
   }

pkgs/native_toolchain_c/test/clinker/rust_test.dart

@@ -6,7 +6,6 @@ import 'dart:io';
 
 import 'package:code_assets/code_assets.dart';
 import 'package:hooks/hooks.dart';
-import 'package:native_test_helpers/native_test_helpers.dart';
 import 'package:native_toolchain_c/native_toolchain_c.dart';
 import 'package:test/test.dart';
 
@@ -60,20 +59,20 @@ Future<void> main() async {
     final treeshakeOption = LinkerOptions.treeshake(
       symbolsToKeep: ['my_other_func'],
     );
-    final symbols = await _link(
+    final asset = await _link(
       staticLib,
       treeshakeOption,
       linkInput,
       linkOutputBuilder,
       targetArchitecture,
       targetOS,
     );
-    final skip = skipLocal(
-      symbols == null,
-      'tool to extract symbols unavailable',
+    await expectSymbols(
+      asset: asset,
+      targetOS: targetOS,
+      symbols: ['my_other_func'],
+      symbolsNotToContain: ['my_func'],
     );
-    expect(symbols, contains('my_other_func'), skip: skip);
-    expect(symbols, isNot(contains('my_func')), skip: skip);
   });
 
   test('link rust binary without script keeps symbols', () async {
@@ -85,24 +84,23 @@ Future<void> main() async {
       stripDebug: true,
       gcSections: true,
     );
-    final symbols = await _link(
+    final asset = await _link(
       staticLib,
       manualOption,
       linkInput,
       linkOutputBuilder,
       targetArchitecture,
       targetOS,
     );
-    final skip = skipLocal(
-      symbols == null,
-      'tool to extract symbols unavailable',
+    await expectSymbols(
+      asset: asset,
+      targetOS: targetOS,
+      symbols: ['my_other_func', 'my_func'],
     );
-    expect(symbols, contains('my_other_func'), skip: skip);
-    expect(symbols, contains('my_func'), skip: skip);
   });
 }
 
-Future<String?> _link(
+Future<CodeAsset> _link(
   Uri staticLib,
   LinkerOptions manualOption,
   LinkInput linkInput,
@@ -122,5 +120,5 @@ Future<String?> _link(
 
   await expectMachineArchitecture(asset.file!, targetArchitecture, targetOS);
 
-  return await readSymbols(asset, targetOS);
+  return asset;
 }

pkgs/native_toolchain_c/test/clinker/treeshake_helper.dart

@@ -6,7 +6,6 @@ import 'dart:io';
 
 import 'package:code_assets/code_assets.dart';
 import 'package:hooks/hooks.dart';
-import 'package:native_test_helpers/native_test_helpers.dart';
 import 'package:native_toolchain_c/native_toolchain_c.dart';
 import 'package:test/test.dart';
 
@@ -151,17 +150,19 @@ void runTreeshakeTests(
         targetOS,
       );
 
-      final symbols = await readSymbols(asset, targetOS);
-      final skip = skipLocal(
-        symbols == null,
-        'tool to extract symbols unavailable',
-      );
       if (clinker.linker != linkerAutoKeepAll) {
-        expect(symbols, contains('my_other_func'), skip: skip);
-        expect(symbols, isNot(contains('my_func')), skip: skip);
+        await expectSymbols(
+          asset: asset,
+          targetOS: targetOS,
+          symbols: ['my_other_func'],
+          symbolsNotToContain: ['my_func'],
+        );
       } else {
-        expect(symbols, contains('my_other_func'), skip: skip);
-        expect(symbols, contains('my_func'), skip: skip);
+        await expectSymbols(
+          asset: asset,
+          targetOS: targetOS,
+          symbols: ['my_other_func', 'my_func'],
+        );
       }
 
       final sizeInBytes = await File.fromUri(asset.file!).length();

pkgs/native_toolchain_c/test/clinker/windows_module_definition_helper.dart

@@ -6,7 +6,6 @@ import 'dart:io';
 
 import 'package:code_assets/code_assets.dart';
 import 'package:hooks/hooks.dart';
-import 'package:native_test_helpers/native_test_helpers.dart';
 import 'package:native_toolchain_c/native_toolchain_c.dart';
 import 'package:test/test.dart';
 
@@ -69,16 +68,14 @@ void runWindowsModuleDefinitionTests(List<Architecture> architectures) {
         expect(codeAssets, hasLength(1));
         final asset = codeAssets.first;
         expect(asset, isA<CodeAsset>());
-        final symbols = await readSymbols(asset, targetOS);
-        final skip = skipLocal(
-          symbols == null,
-          'tool to extract symbols unavailable',
-        );
-        expect(symbols, contains('my_func'), skip: skip);
         // Module Definition file causes my_unexported_func to be exported even
         // though it wasn't marked for export.
-        expect(symbols, contains('my_unexported_func'), skip: skip);
-        expect(symbols, isNot(contains('my_other_func')), skip: skip);
+        await expectSymbols(
+          asset: asset,
+          targetOS: targetOS,
+          symbols: ['my_func', 'my_unexported_func'],
+          symbolsNotToContain: ['my_other_func'],
+        );
       },
     );
   }

pkgs/native_toolchain_c/test/helpers.dart

@@ -258,6 +258,30 @@ Future<void> expectSymbolNotUndefined(
   );
 }
 
+/// Asserts that the library described by [asset] contains [symbols] and does
+/// not contain [symbolsNotToContain].
+Future<void> expectSymbols({
+  required CodeAsset asset,
+  required OS targetOS,
+  List<String> symbols = const [],
+  List<String> symbolsNotToContain = const [],
+}) async {
+  final symbolsString = await readSymbols(asset, targetOS);
+  final skip = skipLocal(
+    symbolsString == null,
+    'tool to extract symbols unavailable',
+    // The Dart SDK CI does not have dumpbin.
+    skipDartSdkCI: true,
+  );
+  expect(symbolsString, isNotNull, skip: skip);
+  for (final symbol in symbols) {
+    expect(symbolsString, contains(symbol), skip: skip);
+  }
+  for (final symbol in symbolsNotToContain) {
+    expect(symbolsString, isNot(contains(symbol)), skip: skip);
+  }
+}
+
 /// Returns null if the dumpbin tool is not available.
 Future<RunProcessResult?> _runDumpbin(
   List<String> arguments,
@@ -449,6 +473,8 @@ Future<void> expectMachineArchitecture(
     final skip = skipLocal(
       result == null,
       'tool to determine binary architecture unavailable',
+      // The Dart SDK CI does not have dumpbin.
+      skipDartSdkCI: true,
     );
     expect(result?.exitCode, 0, skip: skip);
     final machine = result?.stdout