Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Lookup the package config location for Dart and Flutter test targets #7941

Merged
merged 16 commits into from
Jul 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@ part of 'server.dart';
/// serve their assets on the server, and return the list of available
/// extensions here.
Future<List<DevToolsExtensionConfig>> refreshAvailableExtensions(
// TODO(https://github.com/flutter/devtools/issues/7944): pass the URI to the
// package config file instead of passing the app root and rebulding the URI
// to the package config file.
Uri? appRoot,
) async {
_log.fine('refreshAvailableExtensions for app root: ${appRoot.toString()}');
Expand Down
4 changes: 4 additions & 0 deletions packages/devtools_app_shared/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
## 0.2.2-wip
* Lookup the connected app package root from an expression evaluation when
the connected app is a Dart or Flutter test.
* Added a field `logExceptions` to `EvalOnDartLibrary` that defaults to true but
can be disabled to prevent exceptions from being logged to console.
* Add `caseInsensitiveFuzzyMatch` extension method on `String`.
* Add common widgets `DevToolsClearableTextField`, `InputDecorationSuffixButton`,
and `RoundedDropDownButton`.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ class EvalOnDartLibrary extends DisposableController
ValueListenable<IsolateRef?>? isolate,
this.disableBreakpoints = true,
this.oneRequestAtATime = false,
this.logExceptions = true,
}) : _clientId = Random().nextInt(1000000000) {
_libraryRef = Completer<LibraryRef>();

Expand All @@ -60,6 +61,9 @@ class EvalOnDartLibrary extends DisposableController
/// Whether to disable breakpoints triggered while evaluating expressions.
final bool disableBreakpoints;

/// Whether to log exceptions to stdout on failed evaluations.
final bool logExceptions;

/// An ID unique to this instance, so that [asyncEval] keeps working even if
/// the devtool is opened on multiple tabs at the same time.
final int _clientId;
Expand Down Expand Up @@ -261,7 +265,7 @@ class EvalOnDartLibrary extends DisposableController
}

void _handleError(Object e, StackTrace stack) {
if (_disposed) return;
if (_disposed || !logExceptions) return;

if (e is RPCError) {
_log.shout('RPCError: $e', e, stack);
Expand Down
98 changes: 80 additions & 18 deletions packages/devtools_app_shared/lib/src/service/service_manager.dart
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,13 @@ import 'package:devtools_shared/devtools_shared.dart';
import 'package:flutter/foundation.dart';
import 'package:logging/logging.dart';
import 'package:meta/meta.dart';
import 'package:path/path.dart' as path;
import 'package:vm_service/vm_service.dart' hide Error;

import '../utils/utils.dart';
import 'connected_app.dart';
import 'dtd_manager.dart';
import 'eval_on_dart_library.dart' hide SentinelException;
import 'flutter_version.dart';
import 'isolate_manager.dart';
import 'isolate_state.dart';
Expand Down Expand Up @@ -522,30 +524,90 @@ class ServiceManager<T extends VmService> {
// VM service connection spawned from `dart test` or `flutter test`).
if (packageRootUriString?.endsWith('.dart') ?? false) {
final rootLibrary = await _mainIsolateRootLibrary();
final testTargetFileUriString =
(rootLibrary?.dependencies ?? <LibraryDependency>[])
.firstWhereOrNull((dep) => dep.prefix == 'test')
?.target
?.uri;
if (testTargetFileUriString != null) {
if (rootLibrary != null) {
packageRootUriString = (await _lookupPackageRootByEval(rootLibrary)) ??
// TODO(kenz): remove this fallback once all test bootstrap
// generators include the `packageConfigLocation` constant we
// can evaluate.
await _lookupPackageRootByImportPrefix(
rootLibrary,
dtdManager,
);
}
}
_log.fine(
'[connectedAppPackageRoot] package root for test target: '
'$packageRootUriString',
);
return packageRootUriString == null
? null
: Uri.parse(packageRootUriString);
}

Future<String?> _lookupPackageRootByEval(Library rootLibrary) async {
final eval = EvalOnDartLibrary(
rootLibrary.uri!,
this.service! as VmService,
serviceManager: this,
// Swallow exceptions since this evaluation may be called on an older
// version of package:test where we do not expect the evaluation to
// succeed.
logExceptions: false,
);
final evalDisposable = Disposable();
try {
final packageConfig = (await eval.evalInstance(
'packageConfigLocation',
isAlive: evalDisposable,
))
.valueAsString;

// TODO(https://github.com/flutter/devtools/issues/7944): return the
// unmodified package config location. For this case, be sure to handle
// invalid values like the empty String or 'null'.
final packageConfigIdentifier =
path.join('.dart_tool', 'package_config.json');
if (packageConfig?.endsWith(packageConfigIdentifier) ?? false) {
_log.fine(
'[connectedAppPackageRoot] detected test library from root library '
'imports: $testTargetFileUriString',
'[connectedAppPackageRoot] detected test package config from root '
'library eval: $packageConfig.',
);
packageRootUriString = await packageRootFromFileUriString(
testTargetFileUriString,
dtd: dtdManager.connection.value,
);
_log.fine(
'[connectedAppPackageRoot] package root for test target: '
'$packageRootUriString',
return packageConfig!.substring(
jakemac53 marked this conversation as resolved.
Show resolved Hide resolved
0,
// Minus 1 to remove the trailing slash.
packageConfig.length - packageConfigIdentifier.length - 1,
);
}
} catch (_) {
// Fail gracefully if the evaluation fails.
} finally {
evalDisposable.dispose();
}
return null;
}

return packageRootUriString == null
? null
: Uri.parse(packageRootUriString);
Future<String?> _lookupPackageRootByImportPrefix(
Library rootLibrary,
DTDManager dtdManager,
) async {
final testTargetFileUriString =
(rootLibrary.dependencies ?? <LibraryDependency>[])
.firstWhereOrNull((dep) => dep.prefix == 'test')
?.target
?.uri;
if (testTargetFileUriString != null) {
_log.fine(
'[connectedAppPackageRoot] detected test library from root library '
'imports: $testTargetFileUriString',
);
// TODO(https://github.com/flutter/devtools/issues/7944): return the
// unmodified package config location.
return await packageRootFromFileUriString(
testTargetFileUriString,
dtd: dtdManager.connection.value,
);
}
return null;
}

Future<Library?> _mainIsolateRootLibrary() async {
Expand Down
1 change: 1 addition & 0 deletions packages/devtools_app_shared/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ dependencies:
sdk: flutter
logging: ^1.1.1
meta: ^1.9.1
path: ^1.8.0
pointer_interceptor: ^0.9.3+3
url_launcher: ^6.1.0
vm_service: ^14.2.1
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,13 +105,13 @@ class ExtensionsManager {
if (root.toString() == rootFileUriString) continue;

await _addExtensionsForRoot(
// TODO(https://github.com/dart-lang/pub/issues/4218): this logic
// TODO(https://github.com/flutter/devtools/issues/7944): this logic
// assumes that the .dart_tool folder containing the
// package_config.json file is in the same directory as the
// pubspec.yaml file (since `dartToolingDaemon.getProjectRoots`
// returns all directories within the IDE workspace roots that have
// a pubspec.yaml file). This may be an incorrect assumption for
// monorepos.
// pub workspaces.
root.toString(),
logs: logs,
parsingErrors: parsingErrors,
Expand Down Expand Up @@ -143,9 +143,9 @@ class ExtensionsManager {
_assertUriFormat(rootFileUriString);
final List<Extension> extensions;
try {
// TODO(https://github.com/dart-lang/pub/issues/4218): this assumes that
// the .dart_tool/package_config.json file is in the package root, which
// may be an incorrect assumption for monorepos.
// TODO(https://github.com/flutter/devtools/issues/7944): this assumes
// that the .dart_tool/package_config.json file is in the package root,
// which may be an incorrect assumption for pub workspaces.
final packageConfigPath = path.posix.join(
rootFileUriString,
'.dart_tool',
Expand Down Expand Up @@ -188,9 +188,10 @@ class ExtensionsManager {
final extensionConfig = DevToolsExtensionConfig.parse({
...config,
DevToolsExtensionConfig.extensionAssetsPathKey: location,
// TODO(kenz): for monorepos, we may want to store the
// devtools_options.yaml at the same location as the workspace's
// .dart_tool/package_config.json file.
// TODO(https://github.com/flutter/devtools/issues/7944): for pub
// workspaces, we may want to store the devtools_options.yaml at the
// same location as the workspace's .dart_tool/package_config.json
// file.
DevToolsExtensionConfig.devtoolsOptionsUriKey:
path.join(rootFileUriString, devtoolsOptionsFileName),
DevToolsExtensionConfig.isPubliclyHostedKey: isPubliclyHosted,
Expand Down
Loading