Skip to content

notif: Add more tests for messaging style notif and fix for fetchBitmap #813

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 3 commits into from
Jul 30, 2024
Merged

notif: Add more tests for messaging style notif and fix for fetchBitmap #813

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
98374b5
notif: Ensure fetchBitmap succeeds only on HTTP 200 status
rajveermalviya Jul 28, 2024
2c2775a
notif test: Add more tests for messaging style notif
rajveermalviya Jul 28, 2024
915b166
notif [nfc]: Move messageStyleMessages asserts out to `checkNotificat…
rajveermalviya Jul 28, 2024
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
7 changes: 5 additions & 2 deletions lib/notifications/display.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import 'dart:convert';
import 'dart:io';

import 'package:http/http.dart' as http;
import 'package:collection/collection.dart';
Expand Down Expand Up @@ -279,10 +280,12 @@ class NotificationDisplayManager {
try {
// TODO timeout to prevent waiting indefinitely
final resp = await http.get(url);
return resp.bodyBytes;
if (resp.statusCode == HttpStatus.ok) {
return resp.bodyBytes;
}
} catch (e) {
// TODO(log)
return null;
}
return null;
}
}
230 changes: 212 additions & 18 deletions test/notifications/display_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import 'dart:convert';
import 'dart:io';
import 'dart:typed_data';

import 'package:checks/checks.dart';
Expand All @@ -8,6 +9,8 @@ import 'package:firebase_messaging/firebase_messaging.dart';
import 'package:flutter/widgets.dart';
import 'package:flutter_local_notifications/flutter_local_notifications.dart' hide Message, Person;
import 'package:flutter_test/flutter_test.dart';
import 'package:http/http.dart' as http;
import 'package:http/testing.dart' as http_testing;
import 'package:zulip/api/model/model.dart';
import 'package:zulip/api/notifications.dart';
import 'package:zulip/host/android_notifications.dart';
Expand All @@ -25,6 +28,7 @@ import 'package:zulip/widgets/theme.dart';
import '../fake_async.dart';
import '../model/binding.dart';
import '../example_data.dart' as eg;
import '../test_images.dart';
import '../test_navigation.dart';
import '../widgets/message_list_checks.dart';
import '../widgets/page_checks.dart';
Expand Down Expand Up @@ -74,6 +78,24 @@ void main() {
TestZulipBinding.ensureInitialized();
final zulipLocalizations = GlobalLocalizations.zulipLocalizations;

http.Client makeFakeHttpClient({http.Response? response, Exception? exception}) {
return http_testing.MockClient((request) async {
assert((response != null) ^ (exception != null));
if (exception != null) throw exception;
return response!; // TODO return 404 on non avatar urls
});
}

final fakeHttpClientGivingSuccess = makeFakeHttpClient(
response: http.Response.bytes(kSolidBlueAvatar, HttpStatus.ok));

T runWithHttpClient<T>(
T Function() callback, {
http.Client Function()? httpClientFactory,
}) {
return http.runWithClient(callback, httpClientFactory ?? () => fakeHttpClientGivingSuccess);
}

Future<void> init() async {
addTearDown(testBinding.reset);
testBinding.firebaseMessagingInitialToken = '012abc';
Expand Down Expand Up @@ -102,7 +124,11 @@ void main() {
required String expectedTitle,
required String expectedTagComponent,
required bool expectedIsGroupConversation,
List<int>? expectedIconBitmap = kSolidBlueAvatar,
}) {
assert(messageStyleMessages.every((e) => e.userId == data.userId));
assert(messageStyleMessages.every((e) => e.realmUri == data.realmUri));

final expectedTag = '${data.realmUri}|${data.userId}|$expectedTagComponent';
final expectedGroupKey = '${data.realmUri}|${data.userId}';
final expectedId =
Expand All @@ -113,17 +139,15 @@ void main() {

final messageStyleMessagesChecks =
messageStyleMessages.mapIndexed((i, messageData) {
assert(messageData.realmUri == data.realmUri);
assert(messageData.userId == data.userId);

final expectedSenderKey =
'${messageData.realmUri}|${messageData.senderId}';
final isLast = i == (messageStyleMessages.length - 1);
return (Subject<Object?> it) => it.isA<MessagingStyleMessage>()
..text.equals(messageData.content)
..timestampMs.equals(messageData.time * 1000)
..person.which((it) => it.isNotNull()
..iconBitmap.which((it) => isLast ? it.isNotNull() : it.isNull())
..iconBitmap.which((it) => (isLast && expectedIconBitmap != null)
? it.isNotNull().deepEquals(expectedIconBitmap) : it.isNull())
..key.equals(expectedSenderKey)
..name.equals(messageData.senderFullName));
});
Expand Down Expand Up @@ -209,17 +233,17 @@ void main() {
async.flushMicrotasks();
}

test('stream message', () => awaitFakeAsync((async) async {
test('stream message', () => runWithHttpClient(() => awaitFakeAsync((async) async {
await init();
final stream = eg.stream();
final message = eg.streamMessage(stream: stream);
await checkNotifications(async, messageFcmMessage(message, streamName: stream.name),
expectedIsGroupConversation: true,
expectedTitle: '#${stream.name} > ${message.topic}',
expectedTagComponent: 'stream:${message.streamId}:${message.topic}');
}));
})));

test('stream message: multiple messages, same topic', () => awaitFakeAsync((async) async {
test('stream message: multiple messages, same topic', () => runWithHttpClient(() => awaitFakeAsync((async) async {
await init();
final stream = eg.stream();
const topic = 'topic 1';
Expand Down Expand Up @@ -253,54 +277,224 @@ void main() {
expectedIsGroupConversation: true,
expectedTitle: expectedTitle,
expectedTagComponent: expectedTagComponent);
}));
})));

test('stream message: multiple messages, different topics', () => runWithHttpClient(() => awaitFakeAsync((async) async {
Copy link
Collaborator

Choose a reason for hiding this comment

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

notif test: Add more tests for messaging style notif

Add more tests for Android messaging style notif implementation,
listed here:
    #718 (review)

It looks like you meant to link to a specific comment in #718, but there isn't a link to the comment.

await init();
final stream = eg.stream();
const topicA = 'topic A';
const topicB = 'topic B';
final message1 = eg.streamMessage(topic: topicA, stream: stream);
final data1 = messageFcmMessage(message1, streamName: stream.name);
final message2 = eg.streamMessage(topic: topicB, stream: stream);
final data2 = messageFcmMessage(message2, streamName: stream.name);
final message3 = eg.streamMessage(topic: topicA, stream: stream);
final data3 = messageFcmMessage(message3, streamName: stream.name);

await receiveFcmMessage(async, data1);
checkNotification(data1,
messageStyleMessages: [data1],
expectedIsGroupConversation: true,
expectedTitle: '#${stream.name} > $topicA',
expectedTagComponent: 'stream:${stream.streamId}:$topicA');

await receiveFcmMessage(async, data2);
checkNotification(data2,
messageStyleMessages: [data2],
expectedIsGroupConversation: true,
expectedTitle: '#${stream.name} > $topicB',
expectedTagComponent: 'stream:${stream.streamId}:$topicB');

test('stream message: stream name omitted', () => awaitFakeAsync((async) async {
await receiveFcmMessage(async, data3);
checkNotification(data3,
messageStyleMessages: [data1, data3],
expectedIsGroupConversation: true,
expectedTitle: '#${stream.name} > $topicA',
expectedTagComponent: 'stream:${stream.streamId}:$topicA');
})));

test('stream message: conversation stays same when stream is renamed', () => runWithHttpClient(() => awaitFakeAsync((async) async {
await init();
var stream = eg.stream(streamId: 1, name: 'Before');
const topic = 'topic';
final message1 = eg.streamMessage(topic: topic, stream: stream);
final data1 = messageFcmMessage(message1, streamName: stream.name);

await receiveFcmMessage(async, data1);
checkNotification(data1,
messageStyleMessages: [data1],
expectedIsGroupConversation: true,
expectedTitle: '#Before > $topic',
expectedTagComponent: 'stream:${stream.streamId}:$topic');

stream = eg.stream(streamId: 1, name: 'After');
final message2 = eg.streamMessage(topic: topic, stream: stream);
final data2 = messageFcmMessage(message2, streamName: stream.name);

await receiveFcmMessage(async, data2);
checkNotification(data2,
messageStyleMessages: [data1, data2],
expectedIsGroupConversation: true,
expectedTitle: '#After > $topic',
expectedTagComponent: 'stream:${stream.streamId}:$topic');
})));

test('stream message: stream name omitted', () => runWithHttpClient(() => awaitFakeAsync((async) async {
await init();
final stream = eg.stream();
final message = eg.streamMessage(stream: stream);
await checkNotifications(async, messageFcmMessage(message, streamName: null),
expectedIsGroupConversation: true,
expectedTitle: '#(unknown channel) > ${message.topic}',
expectedTagComponent: 'stream:${message.streamId}:${message.topic}');
}));
})));

test('group DM: 3 users', () => awaitFakeAsync((async) async {
test('group DM: 3 users', () => runWithHttpClient(() => awaitFakeAsync((async) async {
await init();
final message = eg.dmMessage(from: eg.thirdUser, to: [eg.otherUser, eg.selfUser]);
await checkNotifications(async, messageFcmMessage(message),
expectedIsGroupConversation: true,
expectedTitle: "${eg.thirdUser.fullName} to you and 1 other",
expectedTagComponent: 'dm:${message.allRecipientIds.join(",")}');
}));
})));

test('group DM: more than 3 users', () => awaitFakeAsync((async) async {
test('group DM: more than 3 users', () => runWithHttpClient(() => awaitFakeAsync((async) async {
await init();
final message = eg.dmMessage(from: eg.thirdUser,
to: [eg.otherUser, eg.selfUser, eg.fourthUser]);
await checkNotifications(async, messageFcmMessage(message),
expectedIsGroupConversation: true,
expectedTitle: "${eg.thirdUser.fullName} to you and 2 others",
expectedTagComponent: 'dm:${message.allRecipientIds.join(",")}');
}));
})));

test('1:1 DM', () => awaitFakeAsync((async) async {
test('group DM: title updates with latest sender', () => runWithHttpClient(() => awaitFakeAsync((async) async {
await init();
final message1 = eg.dmMessage(from: eg.otherUser, to: [eg.selfUser, eg.thirdUser]);
final data1 = messageFcmMessage(message1);
final message2 = eg.dmMessage(from: eg.thirdUser, to: [eg.selfUser, eg.otherUser]);
final data2 = messageFcmMessage(message2);

final expectedTagComponent = 'dm:${message1.allRecipientIds.join(",")}';

await receiveFcmMessage(async, data1);
checkNotification(data1,
messageStyleMessages: [data1],
expectedIsGroupConversation: true,
expectedTitle: "${eg.otherUser.fullName} to you and 1 other",
expectedTagComponent: expectedTagComponent);

await receiveFcmMessage(async, data2);
checkNotification(data2,
messageStyleMessages: [data1, data2],
expectedIsGroupConversation: true,
expectedTitle: "${eg.thirdUser.fullName} to you and 1 other",
expectedTagComponent: expectedTagComponent);
})));

test('1:1 DM', () => runWithHttpClient(() => awaitFakeAsync((async) async {
await init();
final message = eg.dmMessage(from: eg.otherUser, to: [eg.selfUser]);
await checkNotifications(async, messageFcmMessage(message),
expectedIsGroupConversation: false,
expectedTitle: eg.otherUser.fullName,
expectedTagComponent: 'dm:${message.allRecipientIds.join(",")}');
}));
})));

test('1:1 DM: title updates when sender name changes', () => runWithHttpClient(() => awaitFakeAsync((async) async {
await init();
final otherUser = eg.user(fullName: 'Before');
final message1 = eg.dmMessage(from: otherUser, to: [eg.selfUser]);
final data1 = messageFcmMessage(message1);

final expectedTagComponent = 'dm:${message1.allRecipientIds.join(",")}';

await receiveFcmMessage(async, data1);
checkNotification(data1,
messageStyleMessages: [data1],
expectedIsGroupConversation: false,
expectedTitle: 'Before',
expectedTagComponent: expectedTagComponent);

test('self-DM', () => awaitFakeAsync((async) async {
otherUser.fullName = 'After';
final message2 = eg.dmMessage(from: otherUser, to: [eg.selfUser]);
final data2 = messageFcmMessage(message2);

await receiveFcmMessage(async, data2);
checkNotification(data2,
messageStyleMessages: [data1, data2],
expectedIsGroupConversation: false,
expectedTitle: 'After',
expectedTagComponent: expectedTagComponent);
})));

test('1:1 DM: conversation stays same when sender email changes', () => runWithHttpClient(() => awaitFakeAsync((async) async {
await init();
final otherUser = eg.user(email: '[email protected]');
final message1 = eg.dmMessage(from: otherUser, to: [eg.selfUser]);
final data1 = messageFcmMessage(message1);

final expectedTagComponent = 'dm:${message1.allRecipientIds.join(",")}';

await receiveFcmMessage(async, data1);
checkNotification(data1,
messageStyleMessages: [data1],
expectedIsGroupConversation: false,
expectedTitle: otherUser.fullName,
expectedTagComponent: expectedTagComponent);

otherUser.email = '[email protected]';
Copy link
Member

Choose a reason for hiding this comment

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

Ah good catch — I missed this in my review of the previous revision 🙂

final message2 = eg.dmMessage(from: otherUser, to: [eg.selfUser]);
final data2 = messageFcmMessage(message2);

await receiveFcmMessage(async, data2);
checkNotification(data2,
messageStyleMessages: [data1, data2],
expectedIsGroupConversation: false,
expectedTitle: otherUser.fullName,
expectedTagComponent: expectedTagComponent);
})));

test('1:1 DM: sender avatar loading fails, remote error', () => runWithHttpClient(
() => awaitFakeAsync((async) async {
await init();
final message = eg.dmMessage(from: eg.otherUser, to: [eg.selfUser]);
final data = messageFcmMessage(message);
await receiveFcmMessage(async, data);
checkNotification(data,
messageStyleMessages: [data],
expectedIsGroupConversation: false,
expectedTitle: eg.otherUser.fullName,
expectedTagComponent: 'dm:${message.allRecipientIds.join(",")}',
expectedIconBitmap: null); // Failed to fetch avatar photo
}),
httpClientFactory: () => makeFakeHttpClient(
response: http.Response.bytes([], HttpStatus.internalServerError))));

test('1:1 DM: sender avatar loading fails, local error', () => runWithHttpClient(
() => awaitFakeAsync((async) async {
await init();
final message = eg.dmMessage(from: eg.otherUser, to: [eg.selfUser]);
final data = messageFcmMessage(message);
await receiveFcmMessage(async, data);
checkNotification(data,
messageStyleMessages: [data],
expectedIsGroupConversation: false,
expectedTitle: eg.otherUser.fullName,
expectedTagComponent: 'dm:${message.allRecipientIds.join(",")}',
expectedIconBitmap: null); // Failed to fetch avatar photo
}),
httpClientFactory: () => makeFakeHttpClient(
exception: http.ClientException('Network failure'))));

test('self-DM', () => runWithHttpClient(() => awaitFakeAsync((async) async {
await init();
final message = eg.dmMessage(from: eg.selfUser, to: []);
await checkNotifications(async, messageFcmMessage(message),
expectedIsGroupConversation: false,
expectedTitle: eg.selfUser.fullName,
expectedTagComponent: 'dm:${message.allRecipientIds.join(",")}');
}));
})));
});

group('NotificationDisplayManager open', () {
Expand Down