From 37f5276ecab408994e30c839cf04a83b4fb0ffc4 Mon Sep 17 00:00:00 2001 From: MarkZ Date: Wed, 24 Jan 2024 14:47:16 -0800 Subject: [PATCH] Adding tests for cross-module constant equality after hot restart in DDC (#2349) Adding tests for cross-module constant equality after hot restart in DDC. --- dwds/CHANGELOG.md | 2 + dwds/test/fixtures/context.dart | 10 + dwds/test/fixtures/project.dart | 32 +++ dwds/test/reload_correctness_test.dart | 203 ++++++++++++++++++ dwds/test/reload_test.dart | 1 + .../_testHotRestart1Sound/lib/library1.dart | 10 + fixtures/_testHotRestart1Sound/pubspec.yaml | 17 ++ .../_testHotRestart2Sound/lib/library2.dart | 8 + fixtures/_testHotRestart2Sound/pubspec.yaml | 19 ++ .../_testHotRestart2Sound/web/base_index.html | 8 + fixtures/_testHotRestart2Sound/web/index.html | 7 + fixtures/_testHotRestart2Sound/web/main.dart | 34 +++ 12 files changed, 351 insertions(+) create mode 100644 dwds/test/reload_correctness_test.dart create mode 100644 fixtures/_testHotRestart1Sound/lib/library1.dart create mode 100644 fixtures/_testHotRestart1Sound/pubspec.yaml create mode 100644 fixtures/_testHotRestart2Sound/lib/library2.dart create mode 100644 fixtures/_testHotRestart2Sound/pubspec.yaml create mode 100644 fixtures/_testHotRestart2Sound/web/base_index.html create mode 100644 fixtures/_testHotRestart2Sound/web/index.html create mode 100644 fixtures/_testHotRestart2Sound/web/main.dart diff --git a/dwds/CHANGELOG.md b/dwds/CHANGELOG.md index 47e6d363c..42179d692 100644 --- a/dwds/CHANGELOG.md +++ b/dwds/CHANGELOG.md @@ -1,5 +1,7 @@ ## 23.4.0-wip +- Adding tests for constants in DDC after a hot restart - [#2349](https://github.com/dart-lang/webdev/pull/2349) + ## 23.3.0 - Filter out internal type properties from the new DDC type system. - [#2348](https://github.com/dart-lang/webdev/pull/2348) diff --git a/dwds/test/fixtures/context.dart b/dwds/test/fixtures/context.dart index 34a45e76e..4e15a8161 100644 --- a/dwds/test/fixtures/context.dart +++ b/dwds/test/fixtures/context.dart @@ -504,6 +504,16 @@ class TestContext { file.writeAsStringSync(fileContents.replaceAll(toReplace, replaceWith)); } + void makeEditToDartLibFile({ + required String libFileName, + required String toReplace, + required String replaceWith, + }) { + final file = File(project.dartLibFilePath(libFileName)); + final fileContents = file.readAsStringSync(); + file.writeAsStringSync(fileContents.replaceAll(toReplace, replaceWith)); + } + Future waitForSuccessfulBuild({ Duration? timeout, bool propagateToBrowser = false, diff --git a/dwds/test/fixtures/project.dart b/dwds/test/fixtures/project.dart index 0d5458b92..f4f0c54e6 100644 --- a/dwds/test/fixtures/project.dart +++ b/dwds/test/fixtures/project.dart @@ -206,6 +206,26 @@ class TestProject { nullSafety: NullSafety.sound, ); + static const testHotRestart1 = TestProject._( + packageName: '_test_hot_restart1', + packageDirectory: '_testHotRestart1Sound', + webAssetsPath: 'web', + dartEntryFileName: 'main.dart', + htmlEntryFileName: 'index.html', + nullSafety: NullSafety.sound, + ); + + /// This series of hot restart tests is divided across multiple packages in + /// order to test correctness when only a subset of libraries are updated. + static const testHotRestart2 = TestProject._( + packageName: '_test_hot_restart2', + packageDirectory: '_testHotRestart2Sound', + webAssetsPath: 'web', + dartEntryFileName: 'main.dart', + htmlEntryFileName: 'index.html', + nullSafety: NullSafety.sound, + ); + const TestProject._({ required this.packageName, required this.packageDirectory, @@ -234,4 +254,16 @@ class TestProject { workingDirectory: absolutePackageDirectory, ); } + + /// The path to the Dart specified file in the 'lib' directory, e.g, + /// "/workstation/webdev/fixtures/_testSound/lib/library.dart": + String dartLibFilePath(String dartLibFileName) => absolutePath( + pathFromFixtures: p.joinAll( + [ + packageDirectory, + 'lib', + dartLibFileName, + ], + ), + ); } diff --git a/dwds/test/reload_correctness_test.dart b/dwds/test/reload_correctness_test.dart new file mode 100644 index 000000000..98c1648bc --- /dev/null +++ b/dwds/test/reload_correctness_test.dart @@ -0,0 +1,203 @@ +// Copyright (c) 2024, 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. + +@Tags(['daily']) +@TestOn('vm') +@Timeout(Duration(minutes: 5)) +import 'package:dwds/dwds.dart'; +import 'package:test/test.dart'; +import 'package:test_common/logging.dart'; +import 'package:test_common/test_sdk_configuration.dart'; +import 'package:test_common/utilities.dart'; +import 'package:vm_service/vm_service.dart'; + +import 'fixtures/context.dart'; +import 'fixtures/project.dart'; +import 'fixtures/utilities.dart'; + +const originalString = 'variableToModifyToForceRecompile = 23'; +const newString = 'variableToModifyToForceRecompile = 45'; + +const constantSuccessString = 'ConstantEqualitySuccess'; +const constantFailureString = 'ConstantEqualityFailure'; + +void main() { + // set to true for debug logging. + final debug = false; + + final provider = TestSdkConfigurationProvider(verbose: debug); + tearDownAll(provider.dispose); + + final testHotRestart2 = TestProject.testHotRestart2; + final context = TestContext(testHotRestart2, provider); + + Future makeEditAndWaitForRebuild() async { + context.makeEditToDartLibFile( + libFileName: 'library2.dart', + toReplace: originalString, + replaceWith: newString, + ); + await context.waitForSuccessfulBuild(propagateToBrowser: true); + } + + void undoEdit() { + context.makeEditToDartLibFile( + libFileName: 'library2.dart', + toReplace: newString, + replaceWith: originalString, + ); + } + + group( + 'Injected client', + () { + setUp(() async { + setCurrentLogWriter(debug: debug); + await context.setUp( + testSettings: TestSettings( + enableExpressionEvaluation: true, + ), + ); + }); + + tearDown(() async { + await context.tearDown(); + undoEdit(); + }); + + test( + 'properly compares constants after hot restart via the service extension', + () async { + final client = context.debugConnection.vmService; + await client.streamListen('Isolate'); + + var source = await context.webDriver.pageSource; + expect( + source, + contains( + 'ConstObject(reloadVariable: 23, ConstantEqualitySuccess)', + ), + ); + + await makeEditAndWaitForRebuild(); + + final eventsDone = expectLater( + client.onIsolateEvent, + emitsThrough( + emitsInOrder([ + _hasKind(EventKind.kIsolateExit), + _hasKind(EventKind.kIsolateStart), + _hasKind(EventKind.kIsolateRunnable), + ]), + ), + ); + + expect( + await client.callServiceExtension('hotRestart'), + const TypeMatcher(), + ); + + await eventsDone; + + source = await context.webDriver.pageSource; + if (dartSdkIsAtLeast('3.4.0-61.0.dev')) { + expect( + source, + contains( + 'ConstObject(reloadVariable: 45, ConstantEqualitySuccess)', + ), + ); + } + }); + }, + timeout: Timeout.factor(2), + ); + + group( + 'Injected client with hot restart', + () { + group('and with debugging', () { + setUp(() async { + setCurrentLogWriter(debug: debug); + await context.setUp( + testSettings: TestSettings( + reloadConfiguration: ReloadConfiguration.hotRestart, + ), + ); + }); + + tearDown(() async { + await context.tearDown(); + undoEdit(); + }); + + test('properly compares constants after hot restart', () async { + var source = await context.webDriver.pageSource; + expect( + source, + contains( + 'ConstObject(reloadVariable: 23, ConstantEqualitySuccess)', + ), + ); + + await makeEditAndWaitForRebuild(); + + source = await context.webDriver.pageSource; + if (dartSdkIsAtLeast('3.4.0-61.0.dev')) { + expect( + source, + contains( + 'ConstObject(reloadVariable: 45, ConstantEqualitySuccess)', + ), + ); + } + }); + }); + + group('and without debugging', () { + setUp(() async { + setCurrentLogWriter(debug: debug); + await context.setUp( + testSettings: TestSettings( + reloadConfiguration: ReloadConfiguration.hotRestart, + ), + debugSettings: + TestDebugSettings.noDevTools().copyWith(enableDebugging: false), + ); + }); + + tearDown(() async { + await context.tearDown(); + undoEdit(); + }); + + test('properly compares constants after hot restart', () async { + var source = await context.webDriver.pageSource; + expect( + source, + contains( + 'ConstObject(reloadVariable: 23, ConstantEqualitySuccess)', + ), + ); + + await makeEditAndWaitForRebuild(); + + source = await context.webDriver.pageSource; + if (dartSdkIsAtLeast('3.4.0-61.0.dev')) { + expect( + source, + contains( + 'ConstObject(reloadVariable: 45, ConstantEqualitySuccess)', + ), + ); + } + }); + }); + }, + timeout: Timeout.factor(2), + ); +} + +TypeMatcher _hasKind(String kind) => + isA().having((e) => e.kind, 'kind', kind); diff --git a/dwds/test/reload_test.dart b/dwds/test/reload_test.dart index 8a0d8eb10..32b969326 100644 --- a/dwds/test/reload_test.dart +++ b/dwds/test/reload_test.dart @@ -2,6 +2,7 @@ // 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. +@Tags(['daily']) @TestOn('vm') @Timeout(Duration(minutes: 5)) import 'package:dwds/dwds.dart'; diff --git a/fixtures/_testHotRestart1Sound/lib/library1.dart b/fixtures/_testHotRestart1Sound/lib/library1.dart new file mode 100644 index 000000000..e2018bfcc --- /dev/null +++ b/fixtures/_testHotRestart1Sound/lib/library1.dart @@ -0,0 +1,10 @@ +// Copyright (c) 2024, 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. + +class B { + final int x; + const B(this.x); +} + +B get value1 => const B(2); diff --git a/fixtures/_testHotRestart1Sound/pubspec.yaml b/fixtures/_testHotRestart1Sound/pubspec.yaml new file mode 100644 index 000000000..c250bb72e --- /dev/null +++ b/fixtures/_testHotRestart1Sound/pubspec.yaml @@ -0,0 +1,17 @@ +name: _test_hot_restart1 +version: 1.0.0 +description: >- + A fake package used for testing hot restart. +publish_to: none + +environment: + sdk: ^3.2.0-36.0.dev + +dependencies: + intl: ^0.17.0 + path: ^1.6.1 + +dev_dependencies: + build_runner: ^2.4.0 + build_web_compilers: ^4.0.4 + diff --git a/fixtures/_testHotRestart2Sound/lib/library2.dart b/fixtures/_testHotRestart2Sound/lib/library2.dart new file mode 100644 index 000000000..2e3198f2c --- /dev/null +++ b/fixtures/_testHotRestart2Sound/lib/library2.dart @@ -0,0 +1,8 @@ +// Copyright (c) 2024, 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:_test_hot_restart1/library1.dart'; + +int variableToModifyToForceRecompile = 23; +B get value2 => const B(2); diff --git a/fixtures/_testHotRestart2Sound/pubspec.yaml b/fixtures/_testHotRestart2Sound/pubspec.yaml new file mode 100644 index 000000000..a57b8dfad --- /dev/null +++ b/fixtures/_testHotRestart2Sound/pubspec.yaml @@ -0,0 +1,19 @@ +name: _test_hot_restart2 +version: 1.0.0 +description: >- + A fake package used for testing hot restart. +publish_to: none + +environment: + sdk: ^3.2.0-36.0.dev + +dependencies: + intl: ^0.17.0 + path: ^1.6.1 + _test_hot_restart1: + path: ../_testHotRestart1Sound + +dev_dependencies: + build_runner: ^2.4.0 + build_web_compilers: ^4.0.4 + diff --git a/fixtures/_testHotRestart2Sound/web/base_index.html b/fixtures/_testHotRestart2Sound/web/base_index.html new file mode 100644 index 000000000..affdbcb5c --- /dev/null +++ b/fixtures/_testHotRestart2Sound/web/base_index.html @@ -0,0 +1,8 @@ + + + + + + + + diff --git a/fixtures/_testHotRestart2Sound/web/index.html b/fixtures/_testHotRestart2Sound/web/index.html new file mode 100644 index 000000000..d93440a94 --- /dev/null +++ b/fixtures/_testHotRestart2Sound/web/index.html @@ -0,0 +1,7 @@ + + + + + + + diff --git a/fixtures/_testHotRestart2Sound/web/main.dart b/fixtures/_testHotRestart2Sound/web/main.dart new file mode 100644 index 000000000..d17b154a0 --- /dev/null +++ b/fixtures/_testHotRestart2Sound/web/main.dart @@ -0,0 +1,34 @@ +// Copyright (c) 2019, 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 'dart:core'; +import 'dart:html'; + +import 'package:_test_hot_restart1/library1.dart'; +import 'package:_test_hot_restart2/library2.dart'; + +/// Tests for constant semantics across hot restart in DDC. +/// +/// DDC has multiple layers of constant caching. Failing to clear them can +/// result in stale constants being referenced across hot restarts. +/// +/// Cases tested include: +/// 1) Failing to clear all constant caches. +/// An old 'ConstObject' is returned, which fails to reflect the edited +/// 'variableToModifyToForceRecompile'. +/// 2) Clearing constant caches but failing to clear constant containers. +/// Constants in reloaded modules fail to compare with constants in stale +/// constant containers, causing 'ConstantEqualityFailure's. + +class ConstObject { + const ConstObject(); + String get text => 'ConstObject(' + 'reloadVariable: $variableToModifyToForceRecompile, ' + '${value1 == value2 ? 'ConstantEqualitySuccess' : 'ConstantEqualityFailure'})'; +} + +void main() { + document.body!.innerHtml = + 'Program is running!\n${const ConstObject().text}\n'; +}