Skip to content

Adding tests for cross-module constant equality after hot restart in DDC #2349

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

Merged
merged 14 commits into from
Jan 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions dwds/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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)
Expand Down
10 changes: 10 additions & 0 deletions dwds/test/fixtures/context.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
32 changes: 32 additions & 0 deletions dwds/test/fixtures/project.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
],
),
);
}
203 changes: 203 additions & 0 deletions dwds/test/reload_correctness_test.dart
Original file line number Diff line number Diff line change
@@ -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);
1 change: 1 addition & 0 deletions dwds/test/reload_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down
10 changes: 10 additions & 0 deletions fixtures/_testHotRestart1Sound/lib/library1.dart
Original file line number Diff line number Diff line change
@@ -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);
17 changes: 17 additions & 0 deletions fixtures/_testHotRestart1Sound/pubspec.yaml
Original file line number Diff line number Diff line change
@@ -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

8 changes: 8 additions & 0 deletions fixtures/_testHotRestart2Sound/lib/library2.dart
Original file line number Diff line number Diff line change
@@ -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);
19 changes: 19 additions & 0 deletions fixtures/_testHotRestart2Sound/pubspec.yaml
Original file line number Diff line number Diff line change
@@ -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

8 changes: 8 additions & 0 deletions fixtures/_testHotRestart2Sound/web/base_index.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<html>

<head>
<base href="/abc/">
<script defer src="main.dart.js"></script>
</head>

</html>
7 changes: 7 additions & 0 deletions fixtures/_testHotRestart2Sound/web/index.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
<html>

<head>
<script defer src="main.dart.js"></script>
</head>

</html>
34 changes: 34 additions & 0 deletions fixtures/_testHotRestart2Sound/web/main.dart
Original file line number Diff line number Diff line change
@@ -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';
}