[analyzer_testing] Allow testing code with dependencies

[PiotrRogulski]

The AnalysisRuleTest class only allows testing isolated, standalone code snippets. However, many of our custom lint rules (provided in this package) depend on Flutter-based packages (for example, we check if a class is a HookWidget, or if an expression is equivalent to Alignment.center). Thus, to test the behavior of our lints, we need to write code that imports Flutter and possibly other packages — an example of such a test, written with custom_lint, is here: avoid_single_child_in_multi_child_widget_test.dart

[FMorschel]

I've never used this package myself in another project that is not the SDK. Could you please take a look at whether you can do something similar to this code for you:

sdk/pkg/analysis_server/test/src/services/correction/fix/import_library_project_test.dart

Lines 1269 to 1295 in 3f158dc

	Future<void> test_withClass_pub_other_inLib_dependencies() async {
	var aaaRoot = getFolder('$packagesRootPath/aaa');
	newFile('${aaaRoot.path}/lib/a.dart', '''
	class Test {}
	''');
	updateTestPubspecFile(r'''
	name: test
	dependencies:
	aaa: any
	''');
	writeTestPackageConfig(
	config: PackageConfigFileBuilder()
	..add(name: 'aaa', rootPath: aaaRoot.path),
	);
	await resolveTestCode('''
	void f(Test t) {}
	''');
	await assertHasFix('''
	import 'package:aaa/a.dart';
	void f(Test t) {}
	''');
	}

If you can, how can we make this clearer? Where should we document it, etc?

The actual implementation comes from:

sdk/pkg/analysis_server/test/support/configuration_files.dart

Lines 37 to 103 in 3f158dc

	/// Writes a package_config.json for the package at [projectFolderPath]. If
	/// [packageName] is not supplied, the last segment of [projectFolderPath] is
	/// used.
	void writePackageConfig(
	String projectFolderPath, {
	// The name of this package. If not provided, the last segment of the path
	// will be used.
	String? packageName,
	PackageConfigFileBuilder? config,
	String? languageVersion,
	bool flutter = false,
	bool meta = false,
	bool pedantic = false,
	}) {
	projectFolderPath = resourceProvider.convertPath(projectFolderPath);
	if (config == null) {
	config = PackageConfigFileBuilder();
	} else {
	config = config.copy();
	}
	// Add this package to its own config.
	config.add(
	name: packageName ?? pathContext.basename(projectFolderPath),
	rootPath: projectFolderPath,
	languageVersion: languageVersion ?? testPackageLanguageVersion,
	);
	if (meta || flutter) {
	var libFolder = addMeta();
	config.add(name: 'meta', rootPath: libFolder.parent.path);
	}
	if (flutter) {
	var uiLibFolder = addUI();
	config.add(name: 'ui', rootPath: uiLibFolder.parent.path);
	var flutterLibFolder = addFlutter();
	config.add(name: 'flutter', rootPath: flutterLibFolder.parent.path);
	}
	if (addFlutterTestPackageDep) {
	var libFolder = addFlutterTest();
	config.add(name: 'flutter_test', rootPath: libFolder.parent.path);
	}
	if (pedantic) {
	var libFolder = addPedantic();
	config.add(name: 'pedantic', rootPath: libFolder.parent.path);
	}
	if (addVectorMathPackageDep) {
	var libFolder = addVectorMath();
	config.add(name: 'vector_math', rootPath: libFolder.parent.path);
	}
	var content = config.toContent(pathContext: pathContext);
	var projectFolder = resourceProvider.getFolder(projectFolderPath);
	var dartToolFolder = projectFolder.getChildAssumingFolder(
	file_paths.dotDartTool,
	)..create();
	dartToolFolder
	.getChildAssumingFile(file_paths.packageConfigJson)
	.writeAsStringSync(content);
	}

We actually have this code for Flutter widgets:

https://github.com/dart-lang/sdk/blob/3f158dcd71df66ec2f2c103f5c7dc42a40af5f16/pkg/analysis_server/lib/src/services/correction/dart/flutter_wrap.dart

https://github.com/dart-lang/sdk/blob/3f158dcd71df66ec2f2c103f5c7dc42a40af5f16/pkg/analysis_server/test/src/services/correction/assist/flutter_wrap_center_test.dart

[PiotrRogulski]

After some digging in the API of analyzer_testing, I noticed that AnalysisRuleTest has several flags to enable importing some common libraries – including addFlutterPackageDep, which seems to do exactly that (adds flutter and ui packages to the config).

But after setting the flag to true, I get the following error:

Output of dart test

$ dart test test/use_padding_test.dart
  Bad state: Unable to find sdk/pkg/ in /var/folders/y2/7kx_pwvs60x0dznq55zqy11m0000gn/T/dart_test.kernel.Muk8sq/test.dart_1.dill
  package:analyzer_testing/package_root.dart 44:3                                packageRoot
  package:analyzer_testing/mock_packages/mock_packages.dart 14:56                _cacheFiles
  package:analyzer_testing/mock_packages/mock_packages.dart 155:7                MockPackagesMixin._addFiles
  package:analyzer_testing/mock_packages/mock_packages.dart 141:25               MockPackagesMixin.addUI
  package:analyzer_testing/src/analysis_rule/pub_package_resolution.dart 434:20  PubPackageResolutionTest.writeTestPackageConfig
  package:analyzer_testing/src/analysis_rule/pub_package_resolution.dart 384:5   PubPackageResolutionTest.setUp
  package:analyzer_testing/analysis_rule/analysis_rule.dart 99:11                AnalysisRuleTest.setUp
  test/use_padding_test.dart 19:11                                               UsePaddingTest.setUp
  dart:mirrors                                                                   _ClosureMirror.apply
  package:test_reflective_loader/test_reflective_loader.dart 238:32              _invokeSymbolIfExists
  package:test_reflective_loader/test_reflective_loader.dart 277:11              _runTest
  package:test_reflective_loader/test_reflective_loader.dart 115:20              defineReflectiveTests.<fn>.<fn>
  package:test_reflective_loader/test_reflective_loader.dart 164:38              _addTestsIfTopLevelSuite.runTests.<fn>

Output of flutter test

$ flutter test test/use_padding_test.dart
Shell: [ERROR:flutter/shell/common/shell.cc(119)] Dart Error: error: import of dart:mirrors is not supported in the current Dart runtime
Shell: [ERROR:flutter/runtime/dart_isolate.cc(146)] Could not prepare isolate.
Shell: [ERROR:flutter/runtime/runtime_controller.cc(562)] Could not create root isolate.
Shell: [ERROR:flutter/shell/common/shell.cc(794)] Could not launch engine with configuration.

(and then the process breaks, even after ^C-ing it the terminal is unusable)

[FMorschel]

the process breaks, even after ^C-ing it the terminal is unusable

That seems like a problem with test and/or test_reflective_loader. I'm not sure, let me ping some people from the team to see what they have to say here.

CC @natebosch (test) @scheglov (analyzer_testing & test_reflective_loader)

Can any of you help us here?

[bwilkerson]

Also @srawlins

[srawlins]

Yes importing those mock packages disk only works for code running in the Dart SDK (like the built-in lint rule tests). I think we need to refactor the mock package libraries to be like the mock SDK, @bwilkerson . Meaning as gigantic String literals.

Otherwise I don't know of a bullet-proof way to access resources on disk. We could use Platform.script maybe, and go searching up for a package_config.json file, and look for analyzer_testing, and then look for the resources from there. This feels fragile to me... I think I'd rather convert the mock package libraries to strings.

[scheglov]

It looks that dart test compiles into kernel snapshot, and put it somewhere in temporary directory. So, I'm not 100% sure that location tricks will work at runtime, once you have kernel, you are independent, and don't have to carry package_config.json to run. But I don't know, maybe it is still copied into /tmp/xyz for this purpose.

We could improve the solution with strings by generating these strings inside analyzer_testing from files on disk, and have a test that ensures that this always happens.

[bwilkerson]

I think that going to strings would be somewhat unfortunate, but I think the bigger issue is that it won't solve the problem that plugin developers are facing. I think that the real problem here is the inability to control what is and isn't available for the test to use, and there are at least three use cases I can think of that are impacted by this problem.

1. The 'flutter' package we provide is a very small subset of the content of the flutter package in the SDK from which the tests are being run. If the plugin developer wants to use a member in a test that isn't included in our subset, then even if the package were populated from strings they still wouldn't be able to do so.

2. If the plugin developer wants to test against a different version of the flutter package, for example to see whether their plugin works across multiple sdk versions, there's no way to do so.

3. If the plugin developer wants to use a member in a test that is part of their own package (published or otherwise), then they can't do that either.

[srawlins]

If the plugin developer wants to use a member in a test that is part of their own package (published or otherwise), then they can't do that either.

I don't understand this one. Developers can write the files as part of the test, like I do here:

var matcherPath = '/packages/matcher';
newFile('$matcherPath/lib/matcher.dart', '''
void expect(dynamic actual, dynamic matcher) {}
const isNotNull = 0;
const isNull = 0;
''');
writeTestPackageConfig(
  PackageConfigFileBuilder()
    ..add(name: 'matcher', rootPath: convertPath(matcherPath)),
);

[srawlins]

I think it would work similarly to bake in your own support for flutter.

If we don't want to support addFlutterPackageDep (and the other add*PackageDep) conveniences, we should remove them.

[bwilkerson]

Is that documented? If not then we should consider doing so. If so, then I need to do a better job of looking at the docs before replying. :-)

[srawlins]

The docs for the new analyzer_testing package are pretty light. I tried to add doc comments while migrating code into the package, but indeed, writeTestPackageConfig has no doc comment 😅 . And the README could be called a stub: https://pub.dev/packages/analyzer_testing.