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.

Sign up for GitHub

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

Jump to bottom

Roll dwds to 24.0.0 and unified_analytics 6.1.0 #147250

Closed
wants to merge 14 commits into from
Closed

Roll dwds to 24.0.0 and unified_analytics 6.1.0 #147250

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
65635f1
Update packages + fixes for breaking changes
eliasyishak Apr 23, 2024
03bc61e
Fix analyze error + remove type cast
eliasyishak Apr 23, 2024
138a2dc
Use local filesystem instead of global
eliasyishak Apr 23, 2024
a8f52d5
Remove globals dependency from `isolated/resident_runner_test.dart`
eliasyishak Apr 23, 2024
931601a
Swap back to using standalone instance for analytics
eliasyishak Apr 23, 2024
97e7438
Logic added to extract the filesystem from globals
eliasyishak Apr 23, 2024
c350891
Analyze error fix
eliasyishak Apr 23, 2024
a5128e8
Refactor to use type cast for isolated runner test
eliasyishak Apr 23, 2024
e588d98
Merge remote-tracking branch 'upstream/master' into roll-dwds-and-ana…
eliasyishak Apr 24, 2024
b58f013
Ignore empty output events in tests
DanTup Apr 24, 2024
515d695
Update comment
DanTup Apr 24, 2024
45f6e14
Merge pull request #11 from DanTup/filter-empty-output-events
eliasyishak Apr 24, 2024
bc59013
just use our own implementation of FakeAnalytics
christopherfujino Apr 24, 2024
28ebeed
Pin DDS to 4.1.0 and re-run flutter update-packages --force-upgrade
elliette Apr 29, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/flutter_tools/lib/src/isolated/devfs_web.dart
View file
Open in desktop
Expand Up @@ -292,7 +292,7 @@ class WebAssetServer implements AssetReader {
},
toolConfiguration: ToolConfiguration(
loadStrategy: ddcModuleSystem
? FrontendServerLegacyStrategyProvider(
? FrontendServerDdcStrategyProvider(
ReloadConfiguration.none,
server,
PackageUriMapper(packageConfig),
Expand Down
16 changes: 8 additions & 8 deletions packages/flutter_tools/pubspec.yaml
View file
Open in desktop
Expand Up @@ -13,8 +13,8 @@ dependencies:
archive: 3.3.2
args: 2.5.0
browser_launcher: 1.1.1
dds: 3.4.0
dwds: 23.3.0
dds: 4.2.0
dwds: 24.0.0
completion: 1.0.1
coverage: 1.7.2
crypto: 3.0.3
Expand Down Expand Up @@ -52,7 +52,7 @@ dependencies:
http_multi_server: 3.2.1
convert: 3.1.1
async: 2.11.0
unified_analytics: 5.8.8+1
unified_analytics: 6.1.0

cli_config: 0.2.0
graphs: 2.3.1
Expand All @@ -77,9 +77,9 @@ dependencies:
clock: 1.1.1 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"
csslib: 1.0.0 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"
dap: 1.2.0 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"
dds_service_extensions: 1.7.0 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"
devtools_shared: 8.1.1 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"
dtd: 2.1.0 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"
dds_service_extensions: 2.0.0 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"
devtools_shared: 9.0.1 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"
dtd: 2.2.0 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"
extension_discovery: 2.0.0 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"
fixnum: 1.1.0 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"
frontend_server_client: 4.0.0 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"
Expand All @@ -100,7 +100,7 @@ dependencies:
sync_http: 0.3.1 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"
term_glyph: 1.2.1 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"
typed_data: 1.3.2 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"
vm_service_interface: 1.0.1 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"
vm_service_interface: 1.1.0 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"
watcher: 1.1.0 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"
web: 0.5.1 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"
yaml_edit: 2.2.0 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"
Expand All @@ -120,4 +120,4 @@ dartdoc:
# Exclude this package from the hosted API docs.
nodoc: true

# PUBSPEC CHECKSUM: 1789
# PUBSPEC CHECKSUM: 3317
2 changes: 1 addition & 1 deletion packages/flutter_tools/test/commands.shard/hermetic/build_ios_test.dart
View file
Open in desktop
Expand Up @@ -70,7 +70,7 @@ final Platform notMacosPlatform = FakePlatform(
);

void main() {
late FileSystem fileSystem;
late MemoryFileSystem fileSystem;
late TestUsage usage;
late FakeAnalytics fakeAnalytics;
late BufferLogger logger;
Expand Down
2 changes: 1 addition & 1 deletion packages/flutter_tools/test/commands.shard/hermetic/build_ipa_test.dart
View file
Open in desktop
Expand Up @@ -72,7 +72,7 @@ class FakePlistUtils extends Fake implements PlistParser {
}

void main() {
late FileSystem fileSystem;
late MemoryFileSystem fileSystem;
late TestUsage usage;
late FakeProcessManager fakeProcessManager;
late ProcessUtils processUtils;
Expand Down
2 changes: 1 addition & 1 deletion packages/flutter_tools/test/commands.shard/hermetic/build_macos_test.dart
View file
Open in desktop
Expand Up @@ -64,7 +64,7 @@ final Platform notMacosPlatform = FakePlatform(
);

void main() {
late FileSystem fileSystem;
late MemoryFileSystem fileSystem;
late TestUsage usage;
late FakeProcessManager fakeProcessManager;
late ProcessUtils processUtils;
Expand Down
2 changes: 1 addition & 1 deletion packages/flutter_tools/test/commands.shard/hermetic/build_windows_test.dart
View file
Open in desktop
Expand Up @@ -45,7 +45,7 @@ final Platform notWindowsPlatform = FakePlatform(
);

void main() {
late FileSystem fileSystem;
late MemoryFileSystem fileSystem;
late ProcessManager processManager;
late TestUsage usage;
late FakeAnalytics fakeAnalytics;
Expand Down
2 changes: 1 addition & 1 deletion packages/flutter_tools/test/general.shard/android/android_gradle_builder_test.dart
View file
Open in desktop
Expand Up @@ -46,7 +46,7 @@ void main() {
late BufferLogger logger;
late TestUsage testUsage;
late FakeAnalytics fakeAnalytics;
late FileSystem fileSystem;
late MemoryFileSystem fileSystem;
late FakeProcessManager processManager;

setUp(() {
Expand Down
2 changes: 1 addition & 1 deletion packages/flutter_tools/test/general.shard/android/gradle_find_bundle_test.dart
View file
Open in desktop
Expand Up @@ -15,7 +15,7 @@ import '../../src/common.dart';
import '../../src/fakes.dart';

void main() {
late FileSystem fileSystem;
late MemoryFileSystem fileSystem;
late FakeAnalytics fakeAnalytics;

setUp(() {
Expand Down
2 changes: 1 addition & 1 deletion packages/flutter_tools/test/general.shard/build_system/targets/ios_test.dart
View file
Open in desktop
Expand Up @@ -42,7 +42,7 @@ const List<String> _kSharedConfig = <String>[

void main() {
late Environment environment;
late FileSystem fileSystem;
late MemoryFileSystem fileSystem;
late FakeProcessManager processManager;
late Artifacts artifacts;
late BufferLogger logger;
Expand Down
2 changes: 1 addition & 1 deletion packages/flutter_tools/test/general.shard/build_system/targets/macos_test.dart
View file
Open in desktop
Expand Up @@ -21,7 +21,7 @@ import '../../../src/fakes.dart';

void main() {
late Environment environment;
late FileSystem fileSystem;
late MemoryFileSystem fileSystem;
late Artifacts artifacts;
late FakeProcessManager processManager;
late File binary;
Expand Down
6 changes: 3 additions & 3 deletions packages/flutter_tools/test/general.shard/hot_test.dart
View file
Open in desktop
Expand Up @@ -109,7 +109,7 @@ void main() {

group('hotRestart', () {
final FakeResidentCompiler residentCompiler = FakeResidentCompiler();
late FileSystem fileSystem;
late MemoryFileSystem fileSystem;
late TestUsage testUsage;
late FakeAnalytics fakeAnalytics;

Expand Down Expand Up @@ -534,7 +534,7 @@ void main() {
});

group('hot attach', () {
late FileSystem fileSystem;
late MemoryFileSystem fileSystem;
late FakeAnalytics fakeAnalytics;

setUp(() {
Expand Down Expand Up @@ -578,7 +578,7 @@ void main() {
});

group('hot cleanupAtFinish()', () {
late FileSystem fileSystem;
late MemoryFileSystem fileSystem;
late FakeAnalytics fakeAnalytics;

setUp(() {
Expand Down
2 changes: 1 addition & 1 deletion packages/flutter_tools/test/general.shard/isolated/hot_test.dart
View file
Open in desktop
Expand Up @@ -27,7 +27,7 @@ import 'fake_native_assets_build_runner.dart';
void main() {
group('native assets', () {
late TestHotRunnerConfig testingConfig;
late FileSystem fileSystem;
late MemoryFileSystem fileSystem;
late FakeAnalytics fakeAnalytics;

setUp(() {
Expand Down
17 changes: 10 additions & 7 deletions packages/flutter_tools/test/general.shard/isolated/resident_runner_test.dart
View file
Open in desktop
Expand Up @@ -3,10 +3,10 @@
// found in the LICENSE file.


import 'package:file/memory.dart';
import 'package:flutter_tools/src/build_info.dart';
import 'package:flutter_tools/src/device.dart';
import 'package:flutter_tools/src/features.dart';
import 'package:flutter_tools/src/globals.dart' as globals;
import 'package:flutter_tools/src/resident_devtools_handler.dart';
import 'package:flutter_tools/src/resident_runner.dart';
import 'package:flutter_tools/src/run_hot.dart';
Expand All @@ -27,18 +27,21 @@ void main() {
late ResidentRunner residentRunner;
late FakeDevice device;
late FakeAnalytics fakeAnalytics;
late MemoryFileSystem fileSystem;
FakeVmServiceHost? fakeVmServiceHost;

setUp(() {
fileSystem = MemoryFileSystem.test();

testbed = Testbed(setup: () {
fakeAnalytics = getInitializedFakeAnalyticsInstance(
fs: globals.fs,
fs: fileSystem,
fakeFlutterVersion: FakeFlutterVersion(),
);

globals.fs.file('.packages')
fileSystem.file('.packages')
.writeAsStringSync('\n');
globals.fs.file(globals.fs.path.join('build', 'app.dill'))
fileSystem.file(fileSystem.path.join('build', 'app.dill'))
..createSync(recursive: true)
..writeAsStringSync('ABC');
residentRunner = HotRunner(
Expand Down Expand Up @@ -81,8 +84,8 @@ void main() {
listViews,
listViews,
]);
globals.fs
.file(globals.fs.path.join('lib', 'main.dart'))
fileSystem
.file(fileSystem.path.join('lib', 'main.dart'))
.createSync(recursive: true);
final FakeNativeAssetsBuildRunner buildRunner = FakeNativeAssetsBuildRunner();
residentRunner = HotRunner(
Expand Down Expand Up @@ -112,7 +115,7 @@ void main() {
expect(buildRunner.packagesWithNativeAssetsInvocations, 0);

expect(residentCompiler.recompileCalled, true);
expect(residentCompiler.receivedNativeAssetsYaml, globals.fs.path.toUri('foo.yaml'));
expect(residentCompiler.receivedNativeAssetsYaml, fileSystem.path.toUri('foo.yaml'));
}),
overrides: <Type, Generator>{
ProcessManager: () => FakeProcessManager.any(),
Expand Down
2 changes: 1 addition & 1 deletion packages/flutter_tools/test/general.shard/macos/cocoapods_test.dart
View file
Open in desktop
Expand Up @@ -29,7 +29,7 @@ enum _StdioStream {
}

void main() {
late FileSystem fileSystem;
late MemoryFileSystem fileSystem;
late FakeProcessManager fakeProcessManager;
late CocoaPods cocoaPodsUnderTest;
late BufferLogger logger;
Expand Down
9 changes: 9 additions & 0 deletions packages/flutter_tools/test/general.shard/resident_runner_helpers.dart
View file
Open in desktop
Expand Up @@ -5,6 +5,7 @@
import 'dart:async';

import 'package:dds/dds.dart' as dds;
import 'package:file/memory.dart';
import 'package:flutter_tools/src/application_package.dart';
import 'package:flutter_tools/src/asset.dart';
import 'package:flutter_tools/src/base/dds.dart';
Expand Down Expand Up @@ -172,6 +173,12 @@ class FakeDartDevelopmentServiceException implements dds.DartDevelopmentServiceE
@override
final String message;
static const String defaultMessage = 'A DDS instance is already connected at http://localhost:8181';

@override
Map<String, Object?> toJson() => <String, Object?>{
'error_code': errorCode,
'message': message,
};
}

class TestFlutterDevice extends FlutterDevice {
Expand All @@ -187,6 +194,8 @@ class TestFlutterDevice extends FlutterDevice {
class ThrowingForwardingFileSystem extends ForwardingFileSystem {
ThrowingForwardingFileSystem(super.delegate);

FileSystem get fileSystem => delegate;

@override
File file(dynamic path) {
if (path == 'foo') {
Expand Down
13 changes: 12 additions & 1 deletion packages/flutter_tools/test/general.shard/resident_runner_test.dart
View file
Open in desktop
Expand Up @@ -11,6 +11,7 @@ import 'package:flutter_tools/src/artifacts.dart';
import 'package:flutter_tools/src/base/command_help.dart';
import 'package:flutter_tools/src/base/common.dart';
import 'package:flutter_tools/src/base/dds.dart';
import 'package:flutter_tools/src/base/error_handling_io.dart';
import 'package:flutter_tools/src/base/file_system.dart';
import 'package:flutter_tools/src/base/io.dart' as io;
import 'package:flutter_tools/src/base/logger.dart';
Expand Down Expand Up @@ -52,8 +53,18 @@ void main() {

setUp(() {
testbed = Testbed(setup: () {
// Logic to get the filesystem for analytics from globals
final ErrorHandlingFileSystem tempFs = globals.fs as ErrorHandlingFileSystem;
final MemoryFileSystem analyticsFileSystem;
if (tempFs.fileSystem is ThrowingForwardingFileSystem) {
analyticsFileSystem =
(tempFs.fileSystem as ThrowingForwardingFileSystem).fileSystem
as MemoryFileSystem;
} else {
analyticsFileSystem = tempFs.fileSystem as MemoryFileSystem;
}
fakeAnalytics = getInitializedFakeAnalyticsInstance(
fs: globals.fs,
fs: analyticsFileSystem,
fakeFlutterVersion: FakeFlutterVersion(),
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@christopherfujino I'm not the biggest fan of this refactor but we had to consider two cases for this file

  1. For most of the tests, we just rely on the ErrorHandlingFileSystem in which we pass the MemoryFileSystem.test constructor as the delegate, so getting the reference to the filesystem for the analytics constructor was fairly easy
  2. There is one test in this file, HotRunner handles failure to write vmservice file, which uses ThrowingForwardingFileSystem for the delegate in ErrorHandlingFileSystem, as a result, I needed to conditionally check for both types before extracting the delegate or I would get the error below
type 'ThrowingForwardingFileSystem' is not a subtype of type 'MemoryFileSystem' in type cast

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When there is no override in the test for FileSystem

ErrorHandlingFileSystem
  |
  --> delegate = MemoryFileSystem

When we override the FileSystem in the testUsingContext overrides we have an additional level of nesting

ErrorHandlingFileSystem
  |
  --> delegate = ThrowingFileSystem
                              |
                              --> delegate = MemoryFileSystem

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for digging into this. Per my comment here, I think the best solution would be to just use an implementation of Analytics that does not rely on a file system at all. IMO, the file system operations of the Analytics class should be tested in package:unified_analytics--in tests for package:flutter_tools, all we should be testing is the events that we're sending it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about for those tests that rely on opt out status, for example, the new crash reporter refactor to use GA4 opt in status?

);

Expand Down
2 changes: 1 addition & 1 deletion packages/flutter_tools/test/general.shard/resident_web_runner_cold_test.dart
View file
Open in desktop
Expand Up @@ -29,7 +29,7 @@ import '../src/test_build_system.dart';
void main() {
late FakeFlutterDevice mockFlutterDevice;
late FakeWebDevFS mockWebDevFS;
late FileSystem fileSystem;
late MemoryFileSystem fileSystem;

setUp(() {
fileSystem = MemoryFileSystem.test();
Expand Down
2 changes: 1 addition & 1 deletion packages/flutter_tools/test/general.shard/resident_web_runner_test.dart
View file
Open in desktop
Expand Up @@ -102,7 +102,7 @@ void main() {
late FakeWebServerDevice webServerDevice;
late FakeDevice mockDevice;
late FakeVmServiceHost fakeVmServiceHost;
late FileSystem fileSystem;
late MemoryFileSystem fileSystem;
late ProcessManager processManager;
late TestUsage testUsage;
late FakeAnalytics fakeAnalytics;
Expand Down
3 changes: 1 addition & 2 deletions packages/flutter_tools/test/general.shard/unified_analytics_test.dart
View file
Open in desktop
Expand Up @@ -4,7 +4,6 @@

import 'package:file/memory.dart';
import 'package:flutter_tools/src/base/config.dart';
import 'package:flutter_tools/src/base/file_system.dart';
import 'package:flutter_tools/src/reporting/unified_analytics.dart';
import 'package:unified_analytics/unified_analytics.dart';

Expand All @@ -15,7 +14,7 @@ void main() {
const String userBranch = 'abc123';
const String clientIde = 'VSCode';

late FileSystem fs;
late MemoryFileSystem fs;
late Config config;
late FakeAnalytics analyticsOverride;

Expand Down
11 changes: 3 additions & 8 deletions packages/flutter_tools/test/src/common.dart
View file
Open in desktop
Expand Up @@ -17,7 +17,6 @@ import 'package:meta/meta.dart';
import 'package:path/path.dart' as path; // flutter_ignore: package_path_import
import 'package:test/test.dart' as test_package show test;
import 'package:test/test.dart' hide test;
import 'package:unified_analytics/testing.dart';
import 'package:unified_analytics/unified_analytics.dart';

import 'fakes.dart';
Expand Down Expand Up @@ -319,31 +318,27 @@ class FileExceptionHandler {
/// instance, then a second instance will be generated and returned. This second
/// instance will be cleared to send events.
FakeAnalytics getInitializedFakeAnalyticsInstance({
required FileSystem fs,
required MemoryFileSystem fs,
required FakeFlutterVersion fakeFlutterVersion,
String? clientIde,
String? enabledFeatures,
}) {
final Directory homeDirectory = fs.directory('/');
final FakeAnalytics initialAnalytics = FakeAnalytics(
final FakeAnalytics initialAnalytics = Analytics.fake(
tool: DashTool.flutterTool,
homeDirectory: homeDirectory,
dartVersion: fakeFlutterVersion.dartSdkVersion,
platform: DevicePlatform.linux,
fs: fs,
surveyHandler: SurveyHandler(homeDirectory: homeDirectory, fs: fs),
flutterChannel: fakeFlutterVersion.channel,
flutterVersion: fakeFlutterVersion.getVersionString(),
);
initialAnalytics.clientShowedMessage();

return FakeAnalytics(
return Analytics.fake(
tool: DashTool.flutterTool,
homeDirectory: homeDirectory,
dartVersion: fakeFlutterVersion.dartSdkVersion,
platform: DevicePlatform.linux,
fs: fs,
surveyHandler: SurveyHandler(homeDirectory: homeDirectory, fs: fs),
flutterChannel: fakeFlutterVersion.channel,
flutterVersion: fakeFlutterVersion.getVersionString(),
clientIde: clientIde,
Expand Down