Skip to content

[pigeon] Consolidate mock handler tests #4642

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

Merged
merged 5 commits into from
Aug 4, 2023
Merged

[pigeon] Consolidate mock handler tests #4642

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
f47c2d0
Copy the test, and make its import work correctly
stuartmorgan-g Aug 3, 2023
f85d481
Fix channel names
stuartmorgan-g Aug 3, 2023
7333b0c
Delete mock_handler_tester
stuartmorgan-g Aug 3, 2023
4894094
Delete the test suite
stuartmorgan-g Aug 3, 2023
d97a3d6
Bump version
stuartmorgan-g Aug 3, 2023
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
4 changes: 4 additions & 0 deletions packages/pigeon/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
## 10.1.5

* Fixes import in generated Dart test output when overriding package name.

## 10.1.4

* Adds package name to method channel strings to avoid potential collisions between plugins.
Expand Down
3 changes: 2 additions & 1 deletion packages/pigeon/lib/dart_generator.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -599,6 +599,7 @@ if (replyList == null) {
Root root,
StringSink sink, {
required String dartPackageName,
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this used anywhere now? Could we adjust the argument instead of adding a second param?

Copy link
Contributor Author

@stuartmorgan-g stuartmorgan-g Aug 4, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

It's still used in all of the channel names that the test implementation listens to. That's what makes this so fun: the channels have to use the user-provided value or the code will fail at runtime, but the import has to use the actual enclosing package, or the code will fail at compile time, and they aren't always the same.

(It's possible that this will only ever actually affect our tests in practice, but it could happen to anyone with an equally niche setup.)

required String dartOutputPackageName,
}) {
final Indent indent = Indent(sink);
final String sourceOutPath = generatorOptions.sourceOutPath ?? '';
Expand All @@ -620,7 +621,7 @@ if (replyList == null) {
} else {
final String path =
relativeDartPath.replaceFirst(RegExp(r'^.*/lib/'), '');
indent.writeln("import 'package:$dartPackageName/$path';");
indent.writeln("import 'package:$dartOutputPackageName/$path';");
}
for (final Api api in root.apis) {
if (api.location == ApiLocation.host && api.dartHostTestHandler != null) {
Expand Down
2 changes: 1 addition & 1 deletion packages/pigeon/lib/generator_tools.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import 'ast.dart';
/// The current version of pigeon.
///
/// This must match the version in pubspec.yaml.
const String pigeonVersion = '10.1.4';
const String pigeonVersion = '10.1.5';

/// Read all the content from [stdin] to a String.
String readStdin() {
Expand Down
5 changes: 5 additions & 0 deletions packages/pigeon/lib/pigeon_lib.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -510,11 +510,16 @@ class DartTestGeneratorAdapter implements GeneratorAdapter {
basePath: options.basePath ?? '',
);
const DartGenerator testGenerator = DartGenerator();
// The test code needs the actual package name of the Dart output, even if
// the package name has been overridden for other uses.
final String outputPackageName =
deducePackageName(options.dartOut ?? '') ?? options.getPackageName();
testGenerator.generateTest(
dartOptionsWithHeader,
root,
sink,
dartPackageName: options.getPackageName(),
dartOutputPackageName: outputPackageName,
);
}

Expand Down
44 changes: 0 additions & 44 deletions packages/pigeon/mock_handler_tester/.gitignore
View file
Open in desktop

This file was deleted.

10 changes: 0 additions & 10 deletions packages/pigeon/mock_handler_tester/.metadata
View file
Open in desktop

This file was deleted.

3 changes: 0 additions & 3 deletions packages/pigeon/mock_handler_tester/README.md
View file
Open in desktop

This file was deleted.

9 changes: 0 additions & 9 deletions packages/pigeon/mock_handler_tester/lib/main.dart
View file
Open in desktop

This file was deleted.

20 changes: 0 additions & 20 deletions packages/pigeon/mock_handler_tester/pubspec.yaml
View file
Open in desktop

This file was deleted.

Loading