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

@@ -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)

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<void> waitForSuccessfulBuild({
     Duration? timeout,
     bool propagateToBrowser = false,

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,
+          ],
+        ),
+      );
 }

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<void> 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<Success>(),
+        );
+
+        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<Event> _hasKind(String kind) =>
+    isA<Event>().having((e) => e.kind, 'kind', kind);

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';

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);

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
+

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);

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
+

fixtures/_testHotRestart2Sound/web/base_index.html

@@ -0,0 +1,8 @@
+<html>
+
+<head>
+  <base href="/abc/">
+  <script defer src="main.dart.js"></script>
+</head>
+
+</html>

fixtures/_testHotRestart2Sound/web/index.html

@@ -0,0 +1,7 @@
+<html>
+
+<head>
+  <script defer src="main.dart.js"></script>
+</head>
+
+</html>

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';
+}