From 6ddf0d1f7b03b0e20793c18a483e625c66e3122c Mon Sep 17 00:00:00 2001 From: MarkZ Date: Thu, 18 Jan 2024 17:15:04 -0800 Subject: [PATCH 01/12] Adding tests for cross-module constant equality after hot restart in DDC. --- dwds/test/fixtures/context.dart | 10 ++ dwds/test/fixtures/project.dart | 30 ++++ dwds/test/reload_correctness_test.dart | 158 ++++++++++++++++++ fixtures/_testHotRestart1/lib/library1.dart | 12 ++ fixtures/_testHotRestart1/pubspec.yaml | 17 ++ fixtures/_testHotRestart2/lib/library2.dart | 10 ++ fixtures/_testHotRestart2/pubspec.yaml | 19 +++ fixtures/_testHotRestart2/web/base_index.html | 8 + fixtures/_testHotRestart2/web/index.html | 7 + fixtures/_testHotRestart2/web/main.dart | 16 ++ 10 files changed, 287 insertions(+) create mode 100644 dwds/test/reload_correctness_test.dart create mode 100644 fixtures/_testHotRestart1/lib/library1.dart create mode 100644 fixtures/_testHotRestart1/pubspec.yaml create mode 100644 fixtures/_testHotRestart2/lib/library2.dart create mode 100644 fixtures/_testHotRestart2/pubspec.yaml create mode 100644 fixtures/_testHotRestart2/web/base_index.html create mode 100644 fixtures/_testHotRestart2/web/index.html create mode 100644 fixtures/_testHotRestart2/web/main.dart 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..0afd60d4a 100644 --- a/dwds/test/fixtures/project.dart +++ b/dwds/test/fixtures/project.dart @@ -206,6 +206,24 @@ class TestProject { nullSafety: NullSafety.sound, ); + static const testHotRestart1 = TestProject._( + packageName: '_test_hot_restart1', + packageDirectory: '_testHotRestart1', + webAssetsPath: 'web', + dartEntryFileName: 'main.dart', + htmlEntryFileName: 'index.html', + nullSafety: NullSafety.sound, + ); + + static const testHotRestart2 = TestProject._( + packageName: '_test_hot_restart2', + packageDirectory: '_testHotRestart2', + webAssetsPath: 'web', + dartEntryFileName: 'main.dart', + htmlEntryFileName: 'index.html', + nullSafety: NullSafety.sound, + ); + const TestProject._({ required this.packageName, required this.packageDirectory, @@ -234,4 +252,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..783d99b8b --- /dev/null +++ b/dwds/test/reload_correctness_test.dart @@ -0,0 +1,158 @@ +// 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. + +@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: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'); + 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; + + final source = await context.webDriver.pageSource; + expect(source, contains(constantSuccessString)); + expect(source, isNot(contains(constantFailureString))); + }); + }, + timeout: Timeout.factor(3), + ); + + 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 { + await makeEditAndWaitForRebuild(); + + final source = await context.webDriver.pageSource; + expect(source, contains(constantSuccessString)); + expect(source, isNot(contains(constantFailureString))); + }); + }); + + 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 { + await makeEditAndWaitForRebuild(); + + final source = await context.webDriver.pageSource; + expect(source, contains(constantSuccessString)); + expect(source, isNot(contains(constantFailureString))); + }); + }); + }, + timeout: Timeout.factor(3), + ); +} + +TypeMatcher _hasKind(String kind) => + isA().having((e) => e.kind, 'kind', kind); diff --git a/fixtures/_testHotRestart1/lib/library1.dart b/fixtures/_testHotRestart1/lib/library1.dart new file mode 100644 index 000000000..ab9425ab9 --- /dev/null +++ b/fixtures/_testHotRestart1/lib/library1.dart @@ -0,0 +1,12 @@ +// 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. + +library _test_hot_restart1; + +class B { + final int x; + const B(this.x); +} + +B get value1 => const B(2); diff --git a/fixtures/_testHotRestart1/pubspec.yaml b/fixtures/_testHotRestart1/pubspec.yaml new file mode 100644 index 000000000..c250bb72e --- /dev/null +++ b/fixtures/_testHotRestart1/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/_testHotRestart2/lib/library2.dart b/fixtures/_testHotRestart2/lib/library2.dart new file mode 100644 index 000000000..c9b575cdc --- /dev/null +++ b/fixtures/_testHotRestart2/lib/library2.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. + +library _test_hot_restart2; + +import 'package:_test_hot_restart1/library1.dart'; + +int variableToModifyToForceRecompile = 23; +B get value2 => const B(2); diff --git a/fixtures/_testHotRestart2/pubspec.yaml b/fixtures/_testHotRestart2/pubspec.yaml new file mode 100644 index 000000000..26ed74f2b --- /dev/null +++ b/fixtures/_testHotRestart2/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: ../_testHotRestart1 + +dev_dependencies: + build_runner: ^2.4.0 + build_web_compilers: ^4.0.4 + diff --git a/fixtures/_testHotRestart2/web/base_index.html b/fixtures/_testHotRestart2/web/base_index.html new file mode 100644 index 000000000..affdbcb5c --- /dev/null +++ b/fixtures/_testHotRestart2/web/base_index.html @@ -0,0 +1,8 @@ + + + + + + + + diff --git a/fixtures/_testHotRestart2/web/index.html b/fixtures/_testHotRestart2/web/index.html new file mode 100644 index 000000000..d93440a94 --- /dev/null +++ b/fixtures/_testHotRestart2/web/index.html @@ -0,0 +1,7 @@ + + + + + + + diff --git a/fixtures/_testHotRestart2/web/main.dart b/fixtures/_testHotRestart2/web/main.dart new file mode 100644 index 000000000..3b158d54a --- /dev/null +++ b/fixtures/_testHotRestart2/web/main.dart @@ -0,0 +1,16 @@ +// 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'; + +void main() { + document.body!.appendText('Program is running!\n'); + document.body!.appendText(value1 == value2 + ? 'ConstantEqualitySuccess\n' + : 'ConstantEqualityFailure\n'); +} From eab8804b05d05ef192b725a219956b1c3f2d5fb3 Mon Sep 17 00:00:00 2001 From: MarkZ Date: Thu, 18 Jan 2024 17:23:00 -0800 Subject: [PATCH 02/12] updating changelog --- dwds/CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/dwds/CHANGELOG.md b/dwds/CHANGELOG.md index 3fb766c54..0bf14eb1c 100644 --- a/dwds/CHANGELOG.md +++ b/dwds/CHANGELOG.md @@ -1,5 +1,7 @@ ## 23.3.0-wip +- Adding tests for constants in DDC after a hot restart - [#2349](https://github.com/dart-lang/webdev/pull/2349) + ## 23.2.0 - Send untruncated `dart:developer` logs to debugging clients. - [#2333](https://github.com/dart-lang/webdev/pull/2333) From 2e24c01674fd0ea75cdaea0affcb6fd349ef2e50 Mon Sep 17 00:00:00 2001 From: MarkZ Date: Fri, 19 Jan 2024 10:32:13 -0800 Subject: [PATCH 03/12] Adding "sound" to fixture directory name --- dwds/test/fixtures/project.dart | 4 ++-- .../lib/library1.dart | 0 .../{_testHotRestart1 => _testHotRestart1Sound}/pubspec.yaml | 0 .../lib/library2.dart | 0 .../{_testHotRestart2 => _testHotRestart2Sound}/pubspec.yaml | 2 +- .../web/base_index.html | 0 .../web/index.html | 0 .../{_testHotRestart2 => _testHotRestart2Sound}/web/main.dart | 0 8 files changed, 3 insertions(+), 3 deletions(-) rename fixtures/{_testHotRestart1 => _testHotRestart1Sound}/lib/library1.dart (100%) rename fixtures/{_testHotRestart1 => _testHotRestart1Sound}/pubspec.yaml (100%) rename fixtures/{_testHotRestart2 => _testHotRestart2Sound}/lib/library2.dart (100%) rename fixtures/{_testHotRestart2 => _testHotRestart2Sound}/pubspec.yaml (89%) rename fixtures/{_testHotRestart2 => _testHotRestart2Sound}/web/base_index.html (100%) rename fixtures/{_testHotRestart2 => _testHotRestart2Sound}/web/index.html (100%) rename fixtures/{_testHotRestart2 => _testHotRestart2Sound}/web/main.dart (100%) diff --git a/dwds/test/fixtures/project.dart b/dwds/test/fixtures/project.dart index 0afd60d4a..9ba663959 100644 --- a/dwds/test/fixtures/project.dart +++ b/dwds/test/fixtures/project.dart @@ -208,7 +208,7 @@ class TestProject { static const testHotRestart1 = TestProject._( packageName: '_test_hot_restart1', - packageDirectory: '_testHotRestart1', + packageDirectory: '_testHotRestart1Sound', webAssetsPath: 'web', dartEntryFileName: 'main.dart', htmlEntryFileName: 'index.html', @@ -217,7 +217,7 @@ class TestProject { static const testHotRestart2 = TestProject._( packageName: '_test_hot_restart2', - packageDirectory: '_testHotRestart2', + packageDirectory: '_testHotRestart2Sound', webAssetsPath: 'web', dartEntryFileName: 'main.dart', htmlEntryFileName: 'index.html', diff --git a/fixtures/_testHotRestart1/lib/library1.dart b/fixtures/_testHotRestart1Sound/lib/library1.dart similarity index 100% rename from fixtures/_testHotRestart1/lib/library1.dart rename to fixtures/_testHotRestart1Sound/lib/library1.dart diff --git a/fixtures/_testHotRestart1/pubspec.yaml b/fixtures/_testHotRestart1Sound/pubspec.yaml similarity index 100% rename from fixtures/_testHotRestart1/pubspec.yaml rename to fixtures/_testHotRestart1Sound/pubspec.yaml diff --git a/fixtures/_testHotRestart2/lib/library2.dart b/fixtures/_testHotRestart2Sound/lib/library2.dart similarity index 100% rename from fixtures/_testHotRestart2/lib/library2.dart rename to fixtures/_testHotRestart2Sound/lib/library2.dart diff --git a/fixtures/_testHotRestart2/pubspec.yaml b/fixtures/_testHotRestart2Sound/pubspec.yaml similarity index 89% rename from fixtures/_testHotRestart2/pubspec.yaml rename to fixtures/_testHotRestart2Sound/pubspec.yaml index 26ed74f2b..a57b8dfad 100644 --- a/fixtures/_testHotRestart2/pubspec.yaml +++ b/fixtures/_testHotRestart2Sound/pubspec.yaml @@ -11,7 +11,7 @@ dependencies: intl: ^0.17.0 path: ^1.6.1 _test_hot_restart1: - path: ../_testHotRestart1 + path: ../_testHotRestart1Sound dev_dependencies: build_runner: ^2.4.0 diff --git a/fixtures/_testHotRestart2/web/base_index.html b/fixtures/_testHotRestart2Sound/web/base_index.html similarity index 100% rename from fixtures/_testHotRestart2/web/base_index.html rename to fixtures/_testHotRestart2Sound/web/base_index.html diff --git a/fixtures/_testHotRestart2/web/index.html b/fixtures/_testHotRestart2Sound/web/index.html similarity index 100% rename from fixtures/_testHotRestart2/web/index.html rename to fixtures/_testHotRestart2Sound/web/index.html diff --git a/fixtures/_testHotRestart2/web/main.dart b/fixtures/_testHotRestart2Sound/web/main.dart similarity index 100% rename from fixtures/_testHotRestart2/web/main.dart rename to fixtures/_testHotRestart2Sound/web/main.dart From 109ea6c015594ecab95e504f00e384d5773b4959 Mon Sep 17 00:00:00 2001 From: MarkZ Date: Fri, 19 Jan 2024 10:50:43 -0800 Subject: [PATCH 04/12] Removing library statements --- fixtures/_testHotRestart1Sound/lib/library1.dart | 2 -- fixtures/_testHotRestart2Sound/lib/library2.dart | 2 -- 2 files changed, 4 deletions(-) diff --git a/fixtures/_testHotRestart1Sound/lib/library1.dart b/fixtures/_testHotRestart1Sound/lib/library1.dart index ab9425ab9..e2018bfcc 100644 --- a/fixtures/_testHotRestart1Sound/lib/library1.dart +++ b/fixtures/_testHotRestart1Sound/lib/library1.dart @@ -2,8 +2,6 @@ // 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. -library _test_hot_restart1; - class B { final int x; const B(this.x); diff --git a/fixtures/_testHotRestart2Sound/lib/library2.dart b/fixtures/_testHotRestart2Sound/lib/library2.dart index c9b575cdc..2e3198f2c 100644 --- a/fixtures/_testHotRestart2Sound/lib/library2.dart +++ b/fixtures/_testHotRestart2Sound/lib/library2.dart @@ -2,8 +2,6 @@ // 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. -library _test_hot_restart2; - import 'package:_test_hot_restart1/library1.dart'; int variableToModifyToForceRecompile = 23; From b330acf61279bc5a41e420badceae97bf7ba4f76 Mon Sep 17 00:00:00 2001 From: MarkZ Date: Fri, 19 Jan 2024 10:50:58 -0800 Subject: [PATCH 05/12] adding a helper comment to hot restart tests --- dwds/test/fixtures/project.dart | 2 ++ 1 file changed, 2 insertions(+) diff --git a/dwds/test/fixtures/project.dart b/dwds/test/fixtures/project.dart index 9ba663959..f4f0c54e6 100644 --- a/dwds/test/fixtures/project.dart +++ b/dwds/test/fixtures/project.dart @@ -215,6 +215,8 @@ class TestProject { 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', From 8f412c5c80bfac4868bb962b95495428076257b8 Mon Sep 17 00:00:00 2001 From: MarkZ Date: Fri, 19 Jan 2024 10:51:13 -0800 Subject: [PATCH 06/12] tagging hot restart tests to run daily --- dwds/test/reload_correctness_test.dart | 1 + dwds/test/reload_test.dart | 1 + 2 files changed, 2 insertions(+) diff --git a/dwds/test/reload_correctness_test.dart b/dwds/test/reload_correctness_test.dart index 783d99b8b..3f5c3d6ee 100644 --- a/dwds/test/reload_correctness_test.dart +++ b/dwds/test/reload_correctness_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/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'; From ba839de332ee4e02be7e118770894907af04775c Mon Sep 17 00:00:00 2001 From: MarkZ Date: Mon, 22 Jan 2024 12:51:00 -0800 Subject: [PATCH 07/12] Adding additional tests --- dwds/test/reload_correctness_test.dart | 35 +++++++++++++++++--- fixtures/_testHotRestart2Sound/web/main.dart | 20 +++++++++-- 2 files changed, 47 insertions(+), 8 deletions(-) diff --git a/dwds/test/reload_correctness_test.dart b/dwds/test/reload_correctness_test.dart index 3f5c3d6ee..3b3d1c1f3 100644 --- a/dwds/test/reload_correctness_test.dart +++ b/dwds/test/reload_correctness_test.dart @@ -70,6 +70,13 @@ void main() { () async { final client = context.debugConnection.vmService; await client.streamListen('Isolate'); + + var source = await context.webDriver.pageSource; + expect(source, contains(constantSuccessString)); + expect(source, isNot(contains(constantFailureString))); + expect(source, contains('reloadVariable: 23')); + expect(source, isNot(contains('reloadVariable: 45'))); + await makeEditAndWaitForRebuild(); final eventsDone = expectLater( @@ -90,12 +97,14 @@ void main() { await eventsDone; - final source = await context.webDriver.pageSource; + source = await context.webDriver.pageSource; expect(source, contains(constantSuccessString)); expect(source, isNot(contains(constantFailureString))); + expect(source, contains('reloadVariable: 23')); + expect(source, contains('reloadVariable: 45')); }); }, - timeout: Timeout.factor(3), + timeout: Timeout.factor(2), ); group( @@ -117,11 +126,19 @@ void main() { }); test('properly compares constants after hot restart', () async { + var source = await context.webDriver.pageSource; + expect(source, contains(constantSuccessString)); + expect(source, isNot(contains(constantFailureString))); + expect(source, contains('reloadVariable: 23')); + expect(source, isNot(contains('reloadVariable: 45'))); + await makeEditAndWaitForRebuild(); - final source = await context.webDriver.pageSource; + source = await context.webDriver.pageSource; expect(source, contains(constantSuccessString)); expect(source, isNot(contains(constantFailureString))); + expect(source, contains('reloadVariable: 23')); + expect(source, contains('reloadVariable: 45')); }); }); @@ -143,15 +160,23 @@ void main() { }); test('properly compares constants after hot restart', () async { + var source = await context.webDriver.pageSource; + expect(source, contains(constantSuccessString)); + expect(source, isNot(contains(constantFailureString))); + expect(source, contains('reloadVariable: 23')); + expect(source, isNot(contains('reloadVariable: 45'))); + await makeEditAndWaitForRebuild(); - final source = await context.webDriver.pageSource; + source = await context.webDriver.pageSource; expect(source, contains(constantSuccessString)); expect(source, isNot(contains(constantFailureString))); + expect(source, contains('reloadVariable: 23')); + expect(source, contains('reloadVariable: 45')); }); }); }, - timeout: Timeout.factor(3), + timeout: Timeout.factor(2), ); } diff --git a/fixtures/_testHotRestart2Sound/web/main.dart b/fixtures/_testHotRestart2Sound/web/main.dart index 3b158d54a..88008a787 100644 --- a/fixtures/_testHotRestart2Sound/web/main.dart +++ b/fixtures/_testHotRestart2Sound/web/main.dart @@ -8,9 +8,23 @@ import 'dart:html'; import 'package:_test_hot_restart1/library1.dart'; import 'package:_test_hot_restart2/library2.dart'; +class ConstObject { + const ConstObject(); + String get text => 'ConstObject, ' + 'reloadVariable: $variableToModifyToForceRecompile, ' + 'comparison: ${value1 == value2 ? 'ConstantEqualitySuccess' : 'ConstantEqualityFailure'}'; +} + +class ConstObjectWithField { + final int? a = null; + const ConstObjectWithField(); + String get text => 'ConstObjectWithField, ' + 'reloadVariable: $variableToModifyToForceRecompile, ' + 'comparison: ${value1 == value2 ? 'ConstantEqualitySuccess' : 'ConstantEqualityFailure'}'; +} + void main() { document.body!.appendText('Program is running!\n'); - document.body!.appendText(value1 == value2 - ? 'ConstantEqualitySuccess\n' - : 'ConstantEqualityFailure\n'); + document.body!.appendText('${const ConstObject().text}\n'); + document.body!.appendText('${const ConstObjectWithField().text}\n'); } From f5096e027e4f9a8f056ee803d592f400f8ffca3d Mon Sep 17 00:00:00 2001 From: MarkZ Date: Mon, 22 Jan 2024 14:32:47 -0800 Subject: [PATCH 08/12] Updating test comparison strings --- dwds/test/reload_correctness_test.dart | 60 ++++++++++++-------- fixtures/_testHotRestart2Sound/web/main.dart | 17 ++---- 2 files changed, 40 insertions(+), 37 deletions(-) diff --git a/dwds/test/reload_correctness_test.dart b/dwds/test/reload_correctness_test.dart index 3b3d1c1f3..b3206b068 100644 --- a/dwds/test/reload_correctness_test.dart +++ b/dwds/test/reload_correctness_test.dart @@ -72,10 +72,12 @@ void main() { await client.streamListen('Isolate'); var source = await context.webDriver.pageSource; - expect(source, contains(constantSuccessString)); - expect(source, isNot(contains(constantFailureString))); - expect(source, contains('reloadVariable: 23')); - expect(source, isNot(contains('reloadVariable: 45'))); + expect( + source, + contains( + 'ConstObject(reloadVariable: 23, ConstantEqualitySuccess)', + ), + ); await makeEditAndWaitForRebuild(); @@ -98,10 +100,12 @@ void main() { await eventsDone; source = await context.webDriver.pageSource; - expect(source, contains(constantSuccessString)); - expect(source, isNot(contains(constantFailureString))); - expect(source, contains('reloadVariable: 23')); - expect(source, contains('reloadVariable: 45')); + expect( + source, + contains( + 'ConstObject(reloadVariable: 45, ConstantEqualitySuccess)', + ), + ); }); }, timeout: Timeout.factor(2), @@ -127,18 +131,22 @@ void main() { test('properly compares constants after hot restart', () async { var source = await context.webDriver.pageSource; - expect(source, contains(constantSuccessString)); - expect(source, isNot(contains(constantFailureString))); - expect(source, contains('reloadVariable: 23')); - expect(source, isNot(contains('reloadVariable: 45'))); + expect( + source, + contains( + 'ConstObject(reloadVariable: 23, ConstantEqualitySuccess)', + ), + ); await makeEditAndWaitForRebuild(); source = await context.webDriver.pageSource; - expect(source, contains(constantSuccessString)); - expect(source, isNot(contains(constantFailureString))); - expect(source, contains('reloadVariable: 23')); - expect(source, contains('reloadVariable: 45')); + expect( + source, + contains( + 'ConstObject(reloadVariable: 45, ConstantEqualitySuccess)', + ), + ); }); }); @@ -161,18 +169,22 @@ void main() { test('properly compares constants after hot restart', () async { var source = await context.webDriver.pageSource; - expect(source, contains(constantSuccessString)); - expect(source, isNot(contains(constantFailureString))); - expect(source, contains('reloadVariable: 23')); - expect(source, isNot(contains('reloadVariable: 45'))); + expect( + source, + contains( + 'ConstObject(reloadVariable: 23, ConstantEqualitySuccess)', + ), + ); await makeEditAndWaitForRebuild(); source = await context.webDriver.pageSource; - expect(source, contains(constantSuccessString)); - expect(source, isNot(contains(constantFailureString))); - expect(source, contains('reloadVariable: 23')); - expect(source, contains('reloadVariable: 45')); + expect( + source, + contains( + 'ConstObject(reloadVariable: 45, ConstantEqualitySuccess)', + ), + ); }); }); }, diff --git a/fixtures/_testHotRestart2Sound/web/main.dart b/fixtures/_testHotRestart2Sound/web/main.dart index 88008a787..ab132f97e 100644 --- a/fixtures/_testHotRestart2Sound/web/main.dart +++ b/fixtures/_testHotRestart2Sound/web/main.dart @@ -10,21 +10,12 @@ import 'package:_test_hot_restart2/library2.dart'; class ConstObject { const ConstObject(); - String get text => 'ConstObject, ' + String get text => 'ConstObject(' 'reloadVariable: $variableToModifyToForceRecompile, ' - 'comparison: ${value1 == value2 ? 'ConstantEqualitySuccess' : 'ConstantEqualityFailure'}'; -} - -class ConstObjectWithField { - final int? a = null; - const ConstObjectWithField(); - String get text => 'ConstObjectWithField, ' - 'reloadVariable: $variableToModifyToForceRecompile, ' - 'comparison: ${value1 == value2 ? 'ConstantEqualitySuccess' : 'ConstantEqualityFailure'}'; + '${value1 == value2 ? 'ConstantEqualitySuccess' : 'ConstantEqualityFailure'})'; } void main() { - document.body!.appendText('Program is running!\n'); - document.body!.appendText('${const ConstObject().text}\n'); - document.body!.appendText('${const ConstObjectWithField().text}\n'); + document.body!.innerHtml = + 'Program is running!\n${const ConstObject().text}\n'; } From 59fa336ba35fa0d869571d34242426bd739ef5ce Mon Sep 17 00:00:00 2001 From: MarkZ Date: Mon, 22 Jan 2024 16:57:05 -0800 Subject: [PATCH 09/12] Adding test comments --- fixtures/_testHotRestart2Sound/web/main.dart | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/fixtures/_testHotRestart2Sound/web/main.dart b/fixtures/_testHotRestart2Sound/web/main.dart index ab132f97e..0a44de79f 100644 --- a/fixtures/_testHotRestart2Sound/web/main.dart +++ b/fixtures/_testHotRestart2Sound/web/main.dart @@ -8,6 +8,19 @@ 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. +/// Reloaded constants fail to compare with constants in the cache, +/// causing 'ConstantEqualityFailure's. + class ConstObject { const ConstObject(); String get text => 'ConstObject(' From b47770af6424434ce25351cd5328f9d6b3359f33 Mon Sep 17 00:00:00 2001 From: MarkZ Date: Tue, 23 Jan 2024 09:56:46 -0800 Subject: [PATCH 10/12] Updating comments --- fixtures/_testHotRestart2Sound/web/main.dart | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/fixtures/_testHotRestart2Sound/web/main.dart b/fixtures/_testHotRestart2Sound/web/main.dart index 0a44de79f..d17b154a0 100644 --- a/fixtures/_testHotRestart2Sound/web/main.dart +++ b/fixtures/_testHotRestart2Sound/web/main.dart @@ -18,8 +18,8 @@ import 'package:_test_hot_restart2/library2.dart'; /// An old 'ConstObject' is returned, which fails to reflect the edited /// 'variableToModifyToForceRecompile'. /// 2) Clearing constant caches but failing to clear constant containers. -/// Reloaded constants fail to compare with constants in the cache, -/// causing 'ConstantEqualityFailure's. +/// Constants in reloaded modules fail to compare with constants in stale +/// constant containers, causing 'ConstantEqualityFailure's. class ConstObject { const ConstObject(); From 3531da4e7b0fb71be1ac46d4e7e5278582c0036a Mon Sep 17 00:00:00 2001 From: MarkZ Date: Tue, 23 Jan 2024 13:26:11 -0800 Subject: [PATCH 11/12] Locking expectations to 3.4.0-61.0.dev --- dwds/test/reload_correctness_test.dart | 43 +++++++++++++++----------- 1 file changed, 25 insertions(+), 18 deletions(-) diff --git a/dwds/test/reload_correctness_test.dart b/dwds/test/reload_correctness_test.dart index b3206b068..98c1648bc 100644 --- a/dwds/test/reload_correctness_test.dart +++ b/dwds/test/reload_correctness_test.dart @@ -9,6 +9,7 @@ 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'; @@ -100,12 +101,14 @@ void main() { await eventsDone; source = await context.webDriver.pageSource; - expect( - source, - contains( - 'ConstObject(reloadVariable: 45, ConstantEqualitySuccess)', - ), - ); + if (dartSdkIsAtLeast('3.4.0-61.0.dev')) { + expect( + source, + contains( + 'ConstObject(reloadVariable: 45, ConstantEqualitySuccess)', + ), + ); + } }); }, timeout: Timeout.factor(2), @@ -141,12 +144,14 @@ void main() { await makeEditAndWaitForRebuild(); source = await context.webDriver.pageSource; - expect( - source, - contains( - 'ConstObject(reloadVariable: 45, ConstantEqualitySuccess)', - ), - ); + if (dartSdkIsAtLeast('3.4.0-61.0.dev')) { + expect( + source, + contains( + 'ConstObject(reloadVariable: 45, ConstantEqualitySuccess)', + ), + ); + } }); }); @@ -179,12 +184,14 @@ void main() { await makeEditAndWaitForRebuild(); source = await context.webDriver.pageSource; - expect( - source, - contains( - 'ConstObject(reloadVariable: 45, ConstantEqualitySuccess)', - ), - ); + if (dartSdkIsAtLeast('3.4.0-61.0.dev')) { + expect( + source, + contains( + 'ConstObject(reloadVariable: 45, ConstantEqualitySuccess)', + ), + ); + } }); }); }, From 97a81cfc5261c521da36e74ff41607b94dba1e12 Mon Sep 17 00:00:00 2001 From: MarkZ Date: Wed, 24 Jan 2024 12:59:11 -0800 Subject: [PATCH 12/12] Update CHANGELOG.md --- dwds/CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/dwds/CHANGELOG.md b/dwds/CHANGELOG.md index cc22a3133..42179d692 100644 --- a/dwds/CHANGELOG.md +++ b/dwds/CHANGELOG.md @@ -1,8 +1,9 @@ ## 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 -- Adding tests for constants in DDC after a hot restart - [#2349](https://github.com/dart-lang/webdev/pull/2349) - Filter out internal type properties from the new DDC type system. - [#2348](https://github.com/dart-lang/webdev/pull/2348) - Fix errors on displaying getters when debugging in VS Code. - [#2343](https://github.com/dart-lang/webdev/pull/2343)