-
Notifications
You must be signed in to change notification settings - Fork 86
Send debug info from injected client to the debug extension #1772
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
Changes from all commits
2f1305e
a468829
fa15cb1
aa02b29
597ceef
35cd0c3
506abba
96134d7
bd1af85
18fd811
760e1d7
390577c
a325d12
c7f56dc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -86,6 +86,7 @@ class Dwds { | |
bool launchDevToolsInNewWindow = true, | ||
SdkConfigurationProvider? sdkConfigurationProvider, | ||
bool emitDebugEvents = true, | ||
bool isInternalBuild = false, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is something passing this value, or is is it going to be added in future CLs? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not yet, this will get passed by flutter_tools, ddr etc when they start dwds |
||
}) async { | ||
globalLoadStrategy = loadStrategy; | ||
sdkConfigurationProvider ??= DefaultSdkConfigurationProvider(); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,16 @@ | ||
# How to generate data files: | ||
|
||
## Creating a new data file: | ||
|
||
1. Create a new file for your data type in the `/data` directory with the | ||
`.dart` extension | ||
1. Create an abstract class for your data type (see existing files for examples) | ||
1. Add the new data type to `/data/serializers.dart` 4 Run: | ||
`dart run build_runner build` from DWDS root (this will generate the | ||
`.g.dart` file) | ||
|
||
## To update an existing data file: | ||
|
||
1. Make your changes | ||
1. Run: `dart run build_runner clean` from DWDS root | ||
1. Run: `dart run build_runner build` from DWDS root |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,25 @@ | ||
// 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. | ||
|
||
import 'package:built_value/built_value.dart'; | ||
import 'package:built_value/serializer.dart'; | ||
|
||
part 'debug_info.g.dart'; | ||
|
||
abstract class DebugInfo implements Built<DebugInfo, DebugInfoBuilder> { | ||
static Serializer<DebugInfo> get serializer => _$debugInfoSerializer; | ||
|
||
factory DebugInfo([Function(DebugInfoBuilder) updates]) = _$DebugInfo; | ||
|
||
DebugInfo._(); | ||
|
||
String? get appEntrypointPath; | ||
String? get appId; | ||
String? get appInstanceId; | ||
String? get appOrigin; | ||
String? get appUrl; | ||
String? get dwdsVersion; | ||
String? get extensionUrl; | ||
bool? get isInternalBuild; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it possible to add tests to make sure all the data in the debug info is received here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can't think of a good way to test this until we are actually using the debug info to trigger a debug session. We don't have access to the extension icon to see if it has been changed (that is why we have this
onFakeClick
method in the MV2 extension, and we can't intercept runtime messages in tests because they are for extension communication only.Once more of the functionality is added (and we are triggering a debug session) the existence of these values can be tested.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh but I can add tests for the injected client until then, will update those!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was able to add a test for the internal build property on the window object. Not sure how to test the
CustomEvent
unfortunately. Ideally webdriver would have a way of registering an event listener on the page document, so that we could load up the dart app and listen for thedart-app-ready
event and confirm that the properties are what we expect, but unfortunately webdriver doesn't support event listeners like that.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks Elliott! We chatter offline about adding test in subsequent PRs.