Skip to content

Commit

Permalink
Lookup the package config location for Dart and Flutter test targets (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
kenzieschmoll authored Jul 9, 2024
1 parent 330f180 commit 4f0a0f8
Show file tree
Hide file tree
Showing 6 changed files with 102 additions and 27 deletions.
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(
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

0 comments on commit 4f0a0f8

Please sign in to comment.