Skip to content
This repository was archived by the owner on Feb 22, 2023. It is now read-only.

upgraded usage of BinaryMessenger #4451

Merged
gaaclarke merged 5 commits into from
Oct 26, 2021
Merged

upgraded usage of BinaryMessenger #4451

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
d0d9362
upgraded usage of BinaryMessenger
gaaclarke Oct 25, 2021
755e897
added script to massage stable channel tests
gaaclarke Oct 25, 2021
a5021dd
[url_launcher_web] Regenerate mocks used by integration tests.
ditman Oct 26, 2021
8c69fa1
stuart feedback:
gaaclarke Oct 26, 2021
34980e1
stuart feedback
gaaclarke Oct 26, 2021
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 .cirrus.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,8 @@ task:
- export CIRRUS_CHANGE_MESSAGE=""
- export CIRRUS_COMMIT_MESSAGE=""
- ./script/tool_runner.sh lint-android # must come after build-examples
stable_channel_conditional_script:
- dart ./ci/stable_conditional.dart $CHANNEL
Comment thread
gaaclarke marked this conversation as resolved.
Outdated
native_unit_test_script:
# Unsetting CIRRUS_CHANGE_MESSAGE and CIRRUS_COMMIT_MESSAGE as they
# might include non-ASCII characters which makes Gradle crash.
Expand Down
69 changes: 69 additions & 0 deletions ci/stable_conditional.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
// Copyright 2013 The Flutter Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
//
// stable_conditional.dart
//
// Performs simple find and replace operations for conditional compilation
// before executing stable channel tests.
//
// Example input:
// int main() {
// // FLUTTER_STABLE_CHANNEL_BEGIN
Copy link
Copy Markdown
Contributor

@stuartmorgan-g stuartmorgan-g Oct 26, 2021

Choose a reason for hiding this comment

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

This naming is really confusing to me, since the name right after this is what is not going to be on the stable channel.

What if we reverse it and do:

// FLUTTER_STABLE_CHANNEL_BEGIN
// [stable things]
// FLUTTER_STABLE_CHANNEL_ELSE
[not-stable things]
// FLUTTER_STABLE_CHANNEL_END

Copy link
Copy Markdown
Member Author

@gaaclarke gaaclarke Oct 26, 2021

Choose a reason for hiding this comment

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

Yea, that makes sense. With this naming "FLUTTER_STABLE_CHANNEL_" was just meant as a prefix for who is operating and not meant to be read as having meaning for the operation. Is there some way we can name it so that the master channel is on the top? I think it's easier to read the code if what is going to be happening the majority of the time is closer to the place where it will be. The exceptional code should be farther away.

//  FLUTTER_STABLE_CONDITIONAL_IF_NOT_STABLE
//  FLUTTER_STABLE_CONDITIONAL_ELSE
//  FLUTTER_STABLE_CONDITIONAL_ENDIF

That way the prefix matches the name of the script, it keeps the naming similar to what you want but keeps the exceptional case at the bottom.

Copy link
Copy Markdown
Member Author

@gaaclarke gaaclarke Oct 26, 2021

Choose a reason for hiding this comment

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

done

// printf("hello world\n");
// // FLUTTER_STABLE_CHANNEL_REPLACE
// // printf("goodbye world\n");
// // FLUTTER_STABLE_CHANNEL_END
// }
//
// Example output:
// int main() {
// printf("goodbye world\n");
// }

import 'dart:convert' show LineSplitter;
import 'dart:io' show Directory, FileSystemEntity, File;

final RegExp _isSourceRegex =
RegExp(r'\.cc$|\.java$|\.m$\.h$|\.c$|\.swift$|\.kt$');
final RegExp _replacer = RegExp(
r'^\s*// FLUTTER_STABLE_CHANNEL_BEGIN(.*?)^\s*// FLUTTER_STABLE_CHANNEL_REPLACE(.*?)^\s*// FLUTTER_STABLE_CHANNEL_END',
multiLine: true,
dotAll: true);
final RegExp _commentRemover = RegExp(r'^(\s*)\/+\s*(.*)');
Comment thread
gaaclarke marked this conversation as resolved.
Outdated
const String _newline = '\n';

bool _isSourceFile(FileSystemEntity entity) =>
_isSourceRegex.hasMatch(entity.path);
Comment thread
gaaclarke marked this conversation as resolved.
Outdated

void _process(FileSystemEntity entity) {
const LineSplitter splitter = LineSplitter();
final String text = File(entity.path).readAsStringSync();
String replaced = '';
int index = 0;
for (final RegExpMatch match in _replacer.allMatches(text)) {
replaced += text.substring(index, match.start);
for (final String line in splitter.convert(match.group(2)!)) {
final RegExpMatch? commentRemoverMatch = _commentRemover.firstMatch(line);
if (commentRemoverMatch != null) {
replaced += commentRemoverMatch.group(1)! +
commentRemoverMatch.group(2)! +
_newline;
}
}
index = match.end;
}
if (replaced.isNotEmpty) {
replaced += text.substring(index, text.length);
File(entity.path).writeAsStringSync(replaced);
print('modified: ${entity.path}');
}
}

void main(List<String> args) {
final String channel = args[0];
if (channel == 'stable') {
final Directory dir = Directory('.');
dir.list(recursive: true).where(_isSourceFile).forEach(_process);
Comment thread
gaaclarke marked this conversation as resolved.
Outdated
}
}
28 changes: 23 additions & 5 deletions ...ent/android/src/test/java/io/flutter/plugins/androidintent/MethodCallHandlerImplTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,11 @@ public void startListening_registersChannel() {
methodCallHandler.startListening(messenger);

verify(messenger, times(1))
.setMessageHandler(eq(CHANNEL_NAME), any(BinaryMessageHandler.class));
// FLUTTER_STABLE_CHANNEL_BEGIN
.setMessageHandler(eq(CHANNEL_NAME), any(BinaryMessageHandler.class), eq(null));
// FLUTTER_STABLE_CHANNEL_REPLACE
// .setMessageHandler(eq(CHANNEL_NAME), any(BinaryMessageHandler.class));
Comment thread
gaaclarke marked this conversation as resolved.
Outdated
// FLUTTER_STABLE_CHANNEL_END
}

@Test
Expand All @@ -67,9 +71,15 @@ public void startListening_unregistersExistingChannel() {
methodCallHandler.startListening(secondMessenger);

// Unregisters the first and then registers the second.
verify(firstMessenger, times(1)).setMessageHandler(CHANNEL_NAME, null);
// FLUTTER_STABLE_CHANNEL_BEGIN
verify(firstMessenger, times(1)).setMessageHandler(CHANNEL_NAME, null, null);
verify(secondMessenger, times(1))
.setMessageHandler(eq(CHANNEL_NAME), any(BinaryMessageHandler.class));
.setMessageHandler(eq(CHANNEL_NAME), any(BinaryMessageHandler.class), eq(null));
// FLUTTER_STABLE_CHANNEL_REPLACE
// verify(firstMessenger, times(1)).setMessageHandler(CHANNEL_NAME, null);
// verify(secondMessenger, times(1))
// .setMessageHandler(eq(CHANNEL_NAME), any(BinaryMessageHandler.class));
// FLUTTER_STABLE_CHANNEL_END
}

@Test
Expand All @@ -79,7 +89,11 @@ public void stopListening_unregistersExistingChannel() {

methodCallHandler.stopListening();

verify(messenger, times(1)).setMessageHandler(CHANNEL_NAME, null);
// FLUTTER_STABLE_CHANNEL_BEGIN
verify(messenger, times(1)).setMessageHandler(CHANNEL_NAME, null, null);
// FLUTTER_STABLE_CHANNEL_REPLACE
// verify(messenger, times(1)).setMessageHandler(CHANNEL_NAME, null);
// FLUTTER_STABLE_CHANNEL_END
}

@Test
Expand All @@ -88,7 +102,11 @@ public void stopListening_doesNothingWhenUnset() {

methodCallHandler.stopListening();

verify(messenger, never()).setMessageHandler(CHANNEL_NAME, null);
// FLUTTER_STABLE_CHANNEL_BEGIN
verify(messenger, never()).setMessageHandler(CHANNEL_NAME, null, null);
// FLUTTER_STABLE_CHANNEL_REPLACE
// verify(messenger, never()).setMessageHandler(CHANNEL_NAME, null);
// FLUTTER_STABLE_CHANNEL_END
}

@Test
Expand Down
21 changes: 20 additions & 1 deletion ...ages/camera/camera/android/src/test/java/io/flutter/plugins/camera/DartMessengerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@

import android.os.Handler;
import androidx.annotation.NonNull;
import androidx.annotation.Nullable;
import io.flutter.embedding.engine.systemchannels.PlatformChannel;
import io.flutter.plugin.common.BinaryMessenger;
import io.flutter.plugin.common.MethodCall;
Expand All @@ -31,6 +32,15 @@ public class DartMessengerTest {
private static class FakeBinaryMessenger implements BinaryMessenger {
private final List<ByteBuffer> sentMessages = new ArrayList<>();

// TODO(aaclarke): Remove when https://github.com/flutter/engine/pull/29147 is on master.
Comment thread
gaaclarke marked this conversation as resolved.
Outdated
// FLUTTER_STABLE_CHANNEL_BEGIN
@Override
public BinaryMessenger.TaskQueue makeBackgroundTaskQueue() {
return null;
}
// FLUTTER_STABLE_CHANNEL_REPLACE
// FLUTTER_STABLE_CHANNEL_END

@Override
public void send(@NonNull String channel, ByteBuffer message) {
sentMessages.add(message);
Expand All @@ -41,8 +51,17 @@ public void send(@NonNull String channel, ByteBuffer message, BinaryReply callba
send(channel, message);
}

// TODO(aaclarke): Remove when https://github.com/flutter/engine/pull/29147 is on master.
// FLUTTER_STABLE_CHANNEL_BEGIN
@Override
public void setMessageHandler(@NonNull String channel, BinaryMessageHandler handler) {}
public void setMessageHandler(
@NonNull String channel,
BinaryMessageHandler handler,
@Nullable BinaryMessenger.TaskQueue taskQueue) {}
// FLUTTER_STABLE_CHANNEL_REPLACE
// @Override
// public void setMessageHandler(@NonNull String channel, BinaryMessageHandler handler) {}
// FLUTTER_STABLE_CHANNEL_END

List<ByteBuffer> getMessages() {
return new ArrayList<>(sentMessages);
Expand Down
22 changes: 21 additions & 1 deletion ...quick_actions/android/src/test/java/io/flutter/plugins/quickactions/QuickActionsTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,15 @@ public class QuickActionsTest {
private static class TestBinaryMessenger implements BinaryMessenger {
public MethodCall lastMethodCall;

// TODO(aaclarke): Remove when https://github.com/flutter/engine/pull/29147 is on master.
// FLUTTER_STABLE_CHANNEL_BEGIN
@Override
public BinaryMessenger.TaskQueue makeBackgroundTaskQueue() {
return null;
}
// FLUTTER_STABLE_CHANNEL_REPLACE
// FLUTTER_STABLE_CHANNEL_END

@Override
public void send(@NonNull String channel, @Nullable ByteBuffer message) {
send(channel, message, null);
Expand All @@ -49,10 +58,21 @@ public void send(
}
}

// TODO(aaclarke): Remove when https://github.com/flutter/engine/pull/29147 is on master.
// FLUTTER_STABLE_CHANNEL_BEGIN
@Override
public void setMessageHandler(@NonNull String channel, @Nullable BinaryMessageHandler handler) {
public void setMessageHandler(
@NonNull String channel,
@Nullable BinaryMessageHandler handler,
@Nullable BinaryMessenger.TaskQueue taskQueue) {
// Do nothing.
}
// FLUTTER_STABLE_CHANNEL_REPLACE
// @Override
// public void setMessageHandler(
// @NonNull String channel,
// @Nullable BinaryMessageHandler handler) {}
// FLUTTER_STABLE_CHANNEL_END
}

static final int SUPPORTED_BUILD = 25;
Expand Down
28 changes: 23 additions & 5 deletions ...ncher/android/src/test/java/io/flutter/plugins/urllauncher/MethodCallHandlerImplTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,11 @@ public void startListening_registersChannel() {
methodCallHandler.startListening(messenger);

verify(messenger, times(1))
.setMessageHandler(eq(CHANNEL_NAME), any(BinaryMessageHandler.class));
// FLUTTER_STABLE_CHANNEL_BEGIN
.setMessageHandler(eq(CHANNEL_NAME), any(BinaryMessageHandler.class), eq(null));
// FLUTTER_STABLE_CHANNEL_REPLACE
// .setMessageHandler(eq(CHANNEL_NAME), any(BinaryMessageHandler.class));
// FLUTTER_STABLE_CHANNEL_END
}

@Test
Expand All @@ -56,9 +60,15 @@ public void startListening_unregistersExistingChannel() {
methodCallHandler.startListening(secondMessenger);

// Unregisters the first and then registers the second.
verify(firstMessenger, times(1)).setMessageHandler(CHANNEL_NAME, null);
// FLUTTER_STABLE_CHANNEL_BEGIN
verify(firstMessenger, times(1)).setMessageHandler(CHANNEL_NAME, null, null);
verify(secondMessenger, times(1))
.setMessageHandler(eq(CHANNEL_NAME), any(BinaryMessageHandler.class));
.setMessageHandler(eq(CHANNEL_NAME), any(BinaryMessageHandler.class), eq(null));
// FLUTTER_STABLE_CHANNEL_REPLACE
// verify(firstMessenger, times(1)).setMessageHandler(CHANNEL_NAME, null);
// verify(secondMessenger, times(1))
// .setMessageHandler(eq(CHANNEL_NAME), any(BinaryMessageHandler.class));
// FLUTTER_STABLE_CHANNEL_END
}

@Test
Expand All @@ -68,7 +78,11 @@ public void stopListening_unregistersExistingChannel() {

methodCallHandler.stopListening();

verify(messenger, times(1)).setMessageHandler(CHANNEL_NAME, null);
// FLUTTER_STABLE_CHANNEL_BEGIN
verify(messenger, times(1)).setMessageHandler(CHANNEL_NAME, null, null);
// FLUTTER_STABLE_CHANNEL_REPLACE
// verify(messenger, times(1)).setMessageHandler(CHANNEL_NAME, null);
// FLUTTER_STABLE_CHANNEL_END
}

@Test
Expand All @@ -77,7 +91,11 @@ public void stopListening_doesNothingWhenUnset() {

methodCallHandler.stopListening();

verify(messenger, never()).setMessageHandler(CHANNEL_NAME, null);
// FLUTTER_STABLE_CHANNEL_BEGIN
verify(messenger, never()).setMessageHandler(CHANNEL_NAME, null, null);
// FLUTTER_STABLE_CHANNEL_REPLACE
// verify(messenger, never()).setMessageHandler(CHANNEL_NAME, null);
// FLUTTER_STABLE_CHANNEL_END
}

@Test
Expand Down
Loading