[jnigen] Bunch of small testsuite improvements (https://github.com/dart-lang/jnigen/issues/257)
* Throw error in test if summarizer is stale
* Fail if jni dylib is stale
* Add spawnIfNotExists. (It sounds like SQL, which means no further evolution is possible)
* Add tags mechanism to jnigen test
* Add `summarizer_test` and `large_test` tags. So that in dev env, can
exclude these with -x tag for fast iteration.
* Adds dart_test.yaml which is required for this configuration by test
package.
* Use random seed for jnigen tests in CI
diff --git a/.github/workflows/test-package.yml b/.github/workflows/test-package.yml
index a5bb09e..1809e2c 100644
--- a/.github/workflows/test-package.yml
+++ b/.github/workflows/test-package.yml
@@ -92,8 +92,10 @@
- name: Run summarizer tests
run: mvn surefire:test
working-directory: ./pkgs/jnigen/java
+ - name: Build summarizer
+ run: dart run jnigen:setup
- name: Run VM tests
- run: dart test --platform vm
+ run: dart test --test-randomize-ordering-seed random
- name: Install coverage
run: dart pub global activate coverage
- name: Collect coverage
@@ -243,7 +245,10 @@
- name: build notification_plugin example APK
run: flutter build apk --target-platform=android-arm64
working-directory: ./pkgs/jnigen/example/notification_plugin/example
- - run: dart test
+ - name: Build summarizer
+ run: dart run jnigen:setup
+ - name: Run tests
+ run: dart test --test-randomize-ordering-seed random
test_jni_macos_minimal:
needs: [analyze_jni]
@@ -290,7 +295,7 @@
- run: git config --global core.autocrlf true
- run: dart pub get
- run: dart run jnigen:setup
- - run: dart test
+ - run: dart test --test-randomize-ordering-seed random
build_jni_example_linux:
runs-on: ubuntu-latest
diff --git a/pkgs/jni/bin/setup.dart b/pkgs/jni/bin/setup.dart
index eb4f9c9..3f5bb79 100644
--- a/pkgs/jni/bin/setup.dart
+++ b/pkgs/jni/bin/setup.dart
@@ -7,12 +7,13 @@
import 'package:args/args.dart';
import 'package:package_config/package_config.dart';
-const ansiRed = '\x1b[31m';
-const ansiDefault = '\x1b[39;49m';
+import 'package:jni/src/build_util/build_util.dart';
const jniNativeBuildDirective =
'# jni_native_build (Build with jni:setup. Do not delete this line.)';
+// When changing this constant here, also change corresponding path in
+// test/test_util.
const _defaultRelativeBuildPath = "build/jni_libs";
const _buildPath = "build-path";
@@ -126,19 +127,6 @@
return sources;
}
-/// Returns true if [artifact] does not exist, or any file in [sourceDir] is
-/// newer than [artifact].
-bool needsBuild(File artifact, Directory sourceDir) {
- if (!artifact.existsSync()) return true;
- final fileLastModified = artifact.lastModifiedSync();
- for (final entry in sourceDir.listSync(recursive: true)) {
- if (entry.statSync().modified.isAfter(fileLastModified)) {
- return true;
- }
- }
- return false;
-}
-
/// Returns the name of file built using sources in [cDir]
String getTargetName(Directory cDir) {
for (final file in cDir.listSync(recursive: true)) {
diff --git a/pkgs/jni/lib/src/build_util/build_util.dart b/pkgs/jni/lib/src/build_util/build_util.dart
new file mode 100644
index 0000000..8d267df
--- /dev/null
+++ b/pkgs/jni/lib/src/build_util/build_util.dart
@@ -0,0 +1,24 @@
+// Copyright (c) 2022, the Dart project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+// Any shared build logic should be here. This way it can be reused across bin/,
+// tool/ and test/.
+
+import 'dart:io';
+
+const ansiRed = '\x1b[31m';
+const ansiDefault = '\x1b[39;49m';
+
+/// Returns true if [artifact] does not exist, or any file in [sourceDir] is
+/// newer than [artifact].
+bool needsBuild(File artifact, Directory sourceDir) {
+ if (!artifact.existsSync()) return true;
+ final fileLastModified = artifact.lastModifiedSync();
+ for (final entry in sourceDir.listSync(recursive: true)) {
+ if (entry.statSync().modified.isAfter(fileLastModified)) {
+ return true;
+ }
+ }
+ return false;
+}
diff --git a/pkgs/jni/lib/src/jni.dart b/pkgs/jni/lib/src/jni.dart
index 277c2a5..97ff46e 100644
--- a/pkgs/jni/lib/src/jni.dart
+++ b/pkgs/jni/lib/src/jni.dart
@@ -85,13 +85,33 @@
List<String> classPath = const [],
bool ignoreUnrecognized = false,
int jniVersion = JniVersions.JNI_VERSION_1_6,
+ }) {
+ final status = spawnIfNotExists(
+ dylibDir: dylibDir,
+ jvmOptions: jvmOptions,
+ classPath: classPath,
+ ignoreUnrecognized: ignoreUnrecognized,
+ jniVersion: jniVersion,
+ );
+ if (status == false) {
+ throw JvmExistsException();
+ }
+ }
+
+ /// Same as [spawn] but if a JVM exists, returns silently instead of
+ /// throwing [JvmExistsException].
+ ///
+ /// If the options are different than that of existing VM, the existing VM's
+ /// options will remain in effect.
+ static bool spawnIfNotExists({
+ String? dylibDir,
+ List<String> jvmOptions = const [],
+ List<String> classPath = const [],
+ bool ignoreUnrecognized = false,
+ int jniVersion = JniVersions.JNI_VERSION_1_6,
}) =>
using((arena) {
_dylibDir = dylibDir;
- final existVm = _bindings.GetJavaVM();
- if (existVm != nullptr) {
- throw JvmExistsException();
- }
final jvmArgs = _createVMArgs(
options: jvmOptions,
classPath: classPath,
@@ -102,12 +122,9 @@
);
final status = _bindings.SpawnJvm(jvmArgs);
if (status == JniErrorCode.JNI_OK) {
- return;
+ return true;
} else if (status == DART_JNI_SINGLETON_EXISTS) {
- throw JvmExistsException();
- } else if (status == JniErrorCode.JNI_EEXIST) {
- sleep(const Duration(seconds: 1));
- throw JvmExistsException();
+ return false;
} else {
throw SpawnException.of(status);
}
diff --git a/pkgs/jni/test/exception_test.dart b/pkgs/jni/test/exception_test.dart
index 4fb34ef..01acfdf 100644
--- a/pkgs/jni/test/exception_test.dart
+++ b/pkgs/jni/test/exception_test.dart
@@ -11,6 +11,7 @@
void main() {
if (!Platform.isAndroid) {
+ checkDylibIsUpToDate();
bool caught = false;
try {
// If library does not exist, a helpful exception should be thrown.
diff --git a/pkgs/jni/test/global_env_test.dart b/pkgs/jni/test/global_env_test.dart
index 8f65d34..9b6a8b2 100644
--- a/pkgs/jni/test/global_env_test.dart
+++ b/pkgs/jni/test/global_env_test.dart
@@ -25,11 +25,8 @@
// You have to manually pass the path to the `dartjni` dynamic library.
if (!Platform.isAndroid) {
- try {
- Jni.spawn(dylibDir: "build/jni_libs", jvmOptions: ["-Xmx128m"]);
- } on JvmExistsException catch (_) {
- // TODO(#51): Support destroying and reinstantiating JVM.
- }
+ checkDylibIsUpToDate();
+ Jni.spawnIfNotExists(dylibDir: "build/jni_libs", jvmOptions: ["-Xmx128m"]);
}
run(testRunner: test);
}
diff --git a/pkgs/jni/test/jarray_test.dart b/pkgs/jni/test/jarray_test.dart
index 4eca3d4..5717d53 100644
--- a/pkgs/jni/test/jarray_test.dart
+++ b/pkgs/jni/test/jarray_test.dart
@@ -12,11 +12,8 @@
void main() {
// Don't forget to initialize JNI.
if (!Platform.isAndroid) {
- try {
- Jni.spawn(dylibDir: "build/jni_libs", jvmOptions: ["-Xmx128m"]);
- } on JvmExistsException catch (_) {
- // TODO(#51): Support destroying and reinstantiating JVM.
- }
+ checkDylibIsUpToDate();
+ Jni.spawnIfNotExists(dylibDir: "build/jni_libs", jvmOptions: ["-Xmx128m"]);
}
run(testRunner: test);
}
diff --git a/pkgs/jni/test/jobject_test.dart b/pkgs/jni/test/jobject_test.dart
index e951002..8478d5a 100644
--- a/pkgs/jni/test/jobject_test.dart
+++ b/pkgs/jni/test/jobject_test.dart
@@ -17,11 +17,8 @@
void main() {
// Don't forget to initialize JNI.
if (!Platform.isAndroid) {
- try {
- Jni.spawn(dylibDir: "build/jni_libs", jvmOptions: ["-Xmx128m"]);
- } on JvmExistsException catch (_) {
- // TODO(#51): Support destroying and reinstantiating JVM.
- }
+ checkDylibIsUpToDate();
+ Jni.spawnIfNotExists(dylibDir: "build/jni_libs", jvmOptions: ["-Xmx128m"]);
}
run(testRunner: test);
}
diff --git a/pkgs/jni/test/test_util/test_util.dart b/pkgs/jni/test/test_util/test_util.dart
index 40d86eb..07e711a 100644
--- a/pkgs/jni/test/test_util/test_util.dart
+++ b/pkgs/jni/test/test_util/test_util.dart
@@ -2,8 +2,37 @@
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.
+import 'dart:io';
+
+import 'package:jni/src/build_util/build_util.dart';
+
typedef TestCaseCallback = void Function();
typedef TestRunnerCallback = void Function(
String description,
TestCaseCallback test,
);
+
+final currentDir = Directory.current.uri;
+final dllSuffix =
+ Platform.isWindows ? "dll" : (Platform.isMacOS ? "dylib" : "so");
+final dllPrefix = Platform.isWindows ? '' : 'lib';
+final dllPath =
+ currentDir.resolve("build/jni_libs/${dllPrefix}dartjni.$dllSuffix");
+final srcPath = currentDir.resolve("src/");
+
+/// Fail if dartjni dll is stale.
+void checkDylibIsUpToDate() {
+ final dllFile = File.fromUri(dllPath);
+ if (needsBuild(File.fromUri(dllPath), Directory.fromUri(srcPath))) {
+ final cause = dllFile.existsSync()
+ ? 'not up-to-date with source modifications'
+ : 'not built';
+ var message = '\nFatal: dartjni.$dllSuffix is $cause. Please run '
+ '`dart run jni:setup` and try again.';
+ if (stderr.supportsAnsiEscapes) {
+ message = ansiRed + message + ansiDefault;
+ }
+ stderr.writeln(message);
+ exit(1);
+ }
+}
diff --git a/pkgs/jni/test/type_test.dart b/pkgs/jni/test/type_test.dart
index ecdcf10..68693a5 100644
--- a/pkgs/jni/test/type_test.dart
+++ b/pkgs/jni/test/type_test.dart
@@ -205,11 +205,8 @@
void main() {
if (!Platform.isAndroid) {
- try {
- Jni.spawn(dylibDir: "build/jni_libs", jvmOptions: ["-Xmx128m"]);
- } on JvmExistsException catch (_) {
- // TODO(#51): Support destroying and reinstantiating JVM.
- }
+ checkDylibIsUpToDate();
+ Jni.spawnIfNotExists(dylibDir: "build/jni_libs", jvmOptions: ["-Xmx128m"]);
}
run(testRunner: test);
}
diff --git a/pkgs/jnigen/dart_test.yaml b/pkgs/jnigen/dart_test.yaml
new file mode 100644
index 0000000..0fb13bc
--- /dev/null
+++ b/pkgs/jnigen/dart_test.yaml
@@ -0,0 +1,12 @@
+## Copyright (c) 2023, the Dart project authors. Please see the AUTHORS file
+## for details. All rights reserved. Use of this source code is governed by a
+## BSD-style license that can be found in the LICENSE file.
+
+## This file defines test groups for jnigen. This will be helpful to exclude
+## lengthy tests by using -x flag.
+
+tags:
+ large_test:
+ timeout: 2x
+ summarizer_test:
+ timeout: 1x
diff --git a/pkgs/jnigen/test/bindings_test.dart b/pkgs/jnigen/test/bindings_test.dart
index 107c24d..7d98863 100644
--- a/pkgs/jnigen/test/bindings_test.dart
+++ b/pkgs/jnigen/test/bindings_test.dart
@@ -97,6 +97,7 @@
}
void main() async {
+ await checkLocallyBuiltDependencies();
setUpAll(setupDylibsAndClasses);
test('static final fields', () {
diff --git a/pkgs/jnigen/test/config_test.dart b/pkgs/jnigen/test/config_test.dart
index fd935c3..2faa528 100644
--- a/pkgs/jnigen/test/config_test.dart
+++ b/pkgs/jnigen/test/config_test.dart
@@ -10,6 +10,7 @@
import 'package:path/path.dart' as path show equals;
import 'jackson_core_test/generate.dart';
+import 'test_util/test_util.dart';
const packageTests = 'test';
final jacksonCoreTests = absolute(packageTests, 'jackson_core_test');
@@ -93,7 +94,8 @@
});
}
-void main() {
+void main() async {
+ await checkLocallyBuiltDependencies();
final config = Config.parseArgs([
'--config',
jnigenYaml,
diff --git a/pkgs/jnigen/test/dart_generator_test.dart b/pkgs/jnigen/test/dart_generator_test.dart
index 4ad2eb8..e9bbb76 100644
--- a/pkgs/jnigen/test/dart_generator_test.dart
+++ b/pkgs/jnigen/test/dart_generator_test.dart
@@ -5,7 +5,11 @@
import 'package:jnigen/src/bindings/dart_generator.dart';
import 'package:test/test.dart';
-void main() {
+import 'test_util/test_util.dart';
+
+void main() async {
+ await checkLocallyBuiltDependencies();
+
test('OutsideInBuffer', () {
final buffer = OutsideInBuffer();
buffer.appendLeft('f(');
diff --git a/pkgs/jnigen/test/jackson_core_test/generated_files_test.dart b/pkgs/jnigen/test/jackson_core_test/generated_files_test.dart
index 75dde7d..9129612 100644
--- a/pkgs/jnigen/test/jackson_core_test/generated_files_test.dart
+++ b/pkgs/jnigen/test/jackson_core_test/generated_files_test.dart
@@ -11,6 +11,8 @@
import 'generate.dart';
void main() async {
+ await checkLocallyBuiltDependencies();
+
test("compare generated bindings for jackson_core", () async {
final lib = join(thirdPartyDir, 'lib');
final src = join(thirdPartyDir, 'src');
@@ -22,14 +24,16 @@
'not just required classes', () async {
final config = getConfig(generateFullVersion: true);
await generateAndAnalyzeBindings(config);
- }, timeout: const Timeout(Duration(minutes: 2)));
+ }, timeout: const Timeout(Duration(minutes: 2)), tags: largeTestTag);
+
test('generate and analyze bindings using ASM', () async {
final config = getConfig(generateFullVersion: true, useAsm: true);
await generateAndAnalyzeBindings(config);
- }, timeout: const Timeout(Duration(minutes: 2)));
+ }, timeout: const Timeout(Duration(minutes: 2)), tags: largeTestTag);
+
test('Generate and analyze pure dart bindings', () async {
final config = getConfig(generateFullVersion: true);
config.outputConfig.bindingsType = BindingsType.dartOnly;
await generateAndAnalyzeBindings(config);
- }, timeout: const Timeout.factor(2));
+ }, timeout: const Timeout.factor(2), tags: largeTestTag);
}
diff --git a/pkgs/jnigen/test/kotlin_test/generated_files_test.dart b/pkgs/jnigen/test/kotlin_test/generated_files_test.dart
index 9ae50e3..be9ff90 100644
--- a/pkgs/jnigen/test/kotlin_test/generated_files_test.dart
+++ b/pkgs/jnigen/test/kotlin_test/generated_files_test.dart
@@ -10,6 +10,9 @@
import '../test_util/test_util.dart';
void main() async {
+ // This is not run in setupAll, because we want to exit with one line of
+ // error message, not throw a long exception.
+ await checkLocallyBuiltDependencies();
test(
"Generate and compare bindings for kotlin_test",
() async {
@@ -29,5 +32,6 @@
);
},
timeout: const Timeout.factor(1.5),
- ); // test if generated file == expected file
+ tags: largeTestTag,
+ );
}
diff --git a/pkgs/jnigen/test/package_resolver_test.dart b/pkgs/jnigen/test/package_resolver_test.dart
index 39b8998..98c0d2b 100644
--- a/pkgs/jnigen/test/package_resolver_test.dart
+++ b/pkgs/jnigen/test/package_resolver_test.dart
@@ -5,6 +5,8 @@
import 'package:jnigen/src/bindings/resolver.dart';
import 'package:test/test.dart';
+import 'test_util/test_util.dart';
+
class ResolverTest {
ResolverTest(this.binaryName, this.expectedImport, this.expectedName);
String binaryName;
@@ -12,7 +14,8 @@
String expectedName;
}
-void main() {
+void main() async {
+ await checkLocallyBuiltDependencies();
final resolver = Resolver(
importMap: {
'org.apache.pdfbox': 'package:pdfbox/pdfbox.dart',
diff --git a/pkgs/jnigen/test/regenerate_examples_test.dart b/pkgs/jnigen/test/regenerate_examples_test.dart
index 0e2ac62..50e41b6 100644
--- a/pkgs/jnigen/test/regenerate_examples_test.dart
+++ b/pkgs/jnigen/test/regenerate_examples_test.dart
@@ -23,42 +23,59 @@
/// them to provided reference outputs.
///
/// [dartOutput] and [cOutput] are relative paths from example project dir.
-void testExample(String exampleName, String dartOutput, String? cOutput) {
- test('Generate and compare bindings for $exampleName',
- timeout: const Timeout.factor(2), () async {
- final examplePath = join('example', exampleName);
- final configPath = join(examplePath, 'jnigen.yaml');
+///
+/// Pass [isLargeTest] as true if the test will take considerable time.
+void testExample(String exampleName, String dartOutput, String? cOutput,
+ {bool isLargeTest = false}) {
+ test(
+ 'Generate and compare bindings for $exampleName',
+ timeout: const Timeout.factor(2),
+ () async {
+ final examplePath = join('example', exampleName);
+ final configPath = join(examplePath, 'jnigen.yaml');
- final dartBindingsPath = join(examplePath, dartOutput);
- String? cBindingsPath;
- if (cOutput != null) {
- cBindingsPath = join(examplePath, cOutput);
- }
+ final dartBindingsPath = join(examplePath, dartOutput);
+ String? cBindingsPath;
+ if (cOutput != null) {
+ cBindingsPath = join(examplePath, cOutput);
+ }
- final config = Config.parseArgs(['--config', configPath]);
- try {
- await generateAndCompareBindings(config, dartBindingsPath, cBindingsPath);
- } on GradleException catch (_) {
- stderr.writeln('Skip: $exampleName');
- }
- });
+ final config = Config.parseArgs(['--config', configPath]);
+ try {
+ await generateAndCompareBindings(
+ config, dartBindingsPath, cBindingsPath);
+ } on GradleException catch (_) {
+ stderr.writeln('Skip: $exampleName');
+ }
+ },
+ tags: isLargeTest ? largeTestTag : null,
+ );
}
-void main() {
+void main() async {
+ await checkLocallyBuiltDependencies();
testExample(
'in_app_java',
join('lib', 'android_utils.dart'),
join('src', 'android_utils'),
+ isLargeTest: true,
);
- testExample('pdfbox_plugin', join('lib', 'src', 'third_party'), 'src');
+ testExample(
+ 'pdfbox_plugin',
+ join('lib', 'src', 'third_party'),
+ 'src',
+ isLargeTest: false,
+ );
testExample(
'notification_plugin',
join('lib', 'notifications.dart'),
'src',
+ isLargeTest: true,
);
testExample(
'kotlin_plugin',
join('lib', 'kotlin_bindings.dart'),
'src',
+ isLargeTest: true,
);
}
diff --git a/pkgs/jnigen/test/simple_package_test/generated_files_test.dart b/pkgs/jnigen/test/simple_package_test/generated_files_test.dart
index 98a6d5f..084afe0 100644
--- a/pkgs/jnigen/test/simple_package_test/generated_files_test.dart
+++ b/pkgs/jnigen/test/simple_package_test/generated_files_test.dart
@@ -10,6 +10,8 @@
import '../test_util/test_util.dart';
void main() async {
+ await checkLocallyBuiltDependencies();
+
test("Generate and compare bindings for simple_package", () async {
await generateAndCompareBindings(
getConfig(),
diff --git a/pkgs/jnigen/test/summary_generation_test.dart b/pkgs/jnigen/test/summary_generation_test.dart
index 8386db6..e3582c6 100644
--- a/pkgs/jnigen/test/summary_generation_test.dart
+++ b/pkgs/jnigen/test/summary_generation_test.dart
@@ -5,6 +5,8 @@
// These tests validate summary generation in various scenarios.
// Currently, no validation of the summary content itself is done.
+@Tags(['summarizer_test'])
+
import 'dart:io';
import 'package:jnigen/src/config/config.dart';
@@ -107,9 +109,10 @@
);
}
-void main() {
+void main() async {
+ await checkLocallyBuiltDependencies();
late Directory tempDir;
- setUpAll(() {
+ setUpAll(() async {
tempDir = getTempDir("jnigen_summary_tests_");
});
diff --git a/pkgs/jnigen/test/test_util/test_util.dart b/pkgs/jnigen/test/test_util/test_util.dart
index 6870701..450e28a 100644
--- a/pkgs/jnigen/test/test_util/test_util.dart
+++ b/pkgs/jnigen/test/test_util/test_util.dart
@@ -5,14 +5,20 @@
import 'dart:io';
import 'package:jnigen/jnigen.dart';
+import 'package:jnigen/src/util/find_package.dart';
import 'package:path/path.dart' hide equals;
import 'package:test/test.dart';
import 'package:logging/logging.dart' show Level;
-import 'package:jnigen/src/logging/logging.dart' show printError, log;
+import 'package:jnigen/src/logging/logging.dart';
final _currentDirectory = Directory(".");
+// If changing these constants, grep for these values. In some places, test
+// package expects string literals.
+const largeTestTag = 'large_test';
+const summarizerTestTag = 'summarizer_test';
+
Directory getTempDir(String prefix) {
return _currentDirectory.createTempSync(prefix);
}
@@ -127,3 +133,28 @@
tempDir.deleteSync(recursive: true);
}
}
+
+final summarizerJar = join('.', '.dart_tool', 'jnigen', 'ApiSummarizer.jar');
+
+Future<void> failIfSummarizerNotBuilt() async {
+ final jarExists = await File(summarizerJar).exists();
+ if (!jarExists) {
+ stderr.writeln();
+ log.fatal('Please build summarizer by running '
+ '`dart run jnigen:setup` and try again');
+ }
+ final isJarStale = jarExists &&
+ await isPackageModifiedAfter(
+ 'jnigen', await File(summarizerJar).lastModified(), 'java/');
+ if (isJarStale) {
+ stderr.writeln();
+ log.fatal('Summarizer is not rebuilt after recent changes. '
+ 'Please run `dart run jnigen:setup` and try again.');
+ }
+}
+
+/// Verifies if locally built dependencies (currently `ApiSummarizer`)
+/// are up-to-date.
+Future<void> checkLocallyBuiltDependencies() async {
+ await failIfSummarizerNotBuilt();
+}
