From a4156add49dc4f00d33562d6c3dcbfac941b671d Mon Sep 17 00:00:00 2001 From: Andy Balaam Date: Tue, 12 Sep 2023 04:22:04 +0100 Subject: [PATCH] Tests for thread roots and reading history (#11588) * Minor improvements to unread tests * Test for reading an older message in a room * Test for reading an old message in a thread Also a mostly-trivial test for read status being preserved over restarts. * Give the reason for a failing test * Fix a failing test by making it wait for a thread to be read * More tests for thread roots --- cypress/e2e/read-receipts/high-level.spec.ts | 229 +++++++++++++++++-- 1 file changed, 207 insertions(+), 22 deletions(-) diff --git a/cypress/e2e/read-receipts/high-level.spec.ts b/cypress/e2e/read-receipts/high-level.spec.ts index 99dc90f9fa..3bcb51dd50 100644 --- a/cypress/e2e/read-receipts/high-level.spec.ts +++ b/cypress/e2e/read-receipts/high-level.spec.ts @@ -103,11 +103,19 @@ describe("Read receipts", () => { }); } - function jumpTo(room: string, message: string) { + /** + * Find and display a message. + * + * @param room the name of the room to look inside + * @param message the content of the message to fine + * @param includeThreads look for messages inside threads, not just the main timeline + */ + function jumpTo(room: string, message: string, includeThreads = false) { + cy.log("Jump to message", room, message, includeThreads); cy.getClient().then((cli) => { findRoomByName(room).then(async ({ roomId }) => { - const room = cli.getRoom(roomId); - const foundMessage = await getMessage(room, message); + const roomObject = cli.getRoom(roomId); + const foundMessage = await getMessage(roomObject, message, includeThreads); cy.visit(`/#/room/${roomId}/${foundMessage.getId()}`); }); }); @@ -256,6 +264,22 @@ describe("Read receipts", () => { })(); } + /** + * Generate MessageContentSpecs to send multiple threaded responses into a room. + * + * @param rootMessage - the body of the thread root message to send a response to + * @param newMessages - the contents of the messages + */ + function manyThreadedOff(rootMessage: string, newMessages: Array): Array { + return newMessages.map((body) => threadedOff(rootMessage, body)); + } + + /** + * Generate strings with the supplied prefix, suffixed with numbers. + * + * @param prefix the prefix of each string + * @param howMany the number of strings to generate + */ function many(prefix: string, howMany: number): Array { return Array.from(Array(howMany).keys()).map((i) => prefix + i.toFixed()); } @@ -351,8 +375,29 @@ describe("Read receipts", () => { }); } + /** + * Assert a given room is marked as unread, and the number of unread + * messages is less than the supplied count. + * + * @param room - the name of the room to check + * @param lessThan - the number of unread messages that is too many + */ + function assertUnreadLessThan(room: string, lessThan: number) { + cy.log("Assert room some unread", room); + return getRoomListTile(room).within(() => { + cy.get(".mx_NotificationBadge_count").should(($count) => + expect(parseInt($count.get(0).textContent, 10)).to.be.lessThan(lessThan), + ); + }); + } + + function backToThreadsList() { + cy.log("Back to threads list"); + cy.get(".mx_RightPanel").findByTitle("Threads").click(); + } + function openThreadList() { - cy.log("Open thread list"); + cy.log("Open threads list"); cy.findByTestId("threadsButton", { log: false }).then(($button) => { if ($button?.attr("aria-current") !== "true") { cy.findByTestId("threadsButton", { log: false }).click(); @@ -429,7 +474,19 @@ describe("Read receipts", () => { // Then the room becomes read assertRead(room2); }); - it.skip("Reading an older message leaves the room unread", () => {}); + // XXX: fails (sometimes!) because the unread count stays high + it.skip("Reading an older message leaves the room unread", () => { + // Given there are lots of messages in a room + goTo(room1); + receiveMessages(room2, many("Msg", 30)); + assertUnread(room2, 30); + + // When I jump to one of the older messages + jumpTo(room2, "Msg1"); + + // Then the room is still unread, but some messages were read + assertUnreadLessThan(room2, 30); + }); it("Marking a room as read makes it read", () => { // Given I have some unread messages goTo(room1); @@ -471,7 +528,21 @@ describe("Read receipts", () => { // Then I still have an unread message assertUnread(room2, 1); }); - it.skip("A room where all messages are read is still read after restart", () => {}); + it("A room where all messages are read is still read after restart", () => { + // Given I have read all messages + goTo(room1); + assertRead(room2); + receiveMessages(room2, ["Msg1"]); + assertUnread(room2, 1); + goTo(room2); + assertRead(room2); + + // When I restart + saveAndReload(); + + // Then all messages are still read + assertRead(room2); + }); it("A room that was marked as read is still read after restart", () => { // Given I have marked all messages as read goTo(room1); @@ -549,7 +620,25 @@ describe("Read receipts", () => { assertReadThread("Msg1"); assertRead(room2); }); - it.skip("Reading an older thread message (via permalink) leaves the thread unread", () => {}); + it("Reading an older thread message leaves the thread unread", () => { + // Given there are many messages in a thread + goTo(room1); + receiveMessages(room2, ["ThreadRoot", ...manyThreadedOff("ThreadRoot", many("InThread", 20))]); + assertUnread(room2, 21); + + // When I read an older message in the thread + jumpTo(room2, "InThread1", true); + assertUnreadLessThan(room2, 21); + // TODO: for some reason, we can't find the first message + // "InThread0", so I am using the second here. Also, they appear + // out of order, with "InThread2" before "InThread1". Might be a + // clue to the sporadic reports we have had of messages going + // missing in threads? + + // Then the thread is still marked as unread + backToThreadsList(); + assertUnreadThread("ThreadRoot"); + }); it("Reading only one thread's message does not make the room read", () => { // Given two threads are unread goTo(room1); @@ -717,6 +806,7 @@ describe("Read receipts", () => { goTo(room1); receiveMessages(room2, ["Msg1", replyTo("Msg1", "Reply1")]); goTo(room2); + assertRead(room2); goTo(room1); assertRead(room2); @@ -727,16 +817,20 @@ describe("Read receipts", () => { assertUnread(room2, 1); }); it("Reading a thread whose root is a reply makes the room read", () => { + // Given an unread thread off a reply exists goTo(room1); receiveMessages(room2, ["Msg1", replyTo("Msg1", "Reply1"), threadedOff("Reply1", "Resp1")]); assertUnread(room2, 3); - goTo(room2); assertUnread(room2, 1); assertUnreadThread("Reply1"); + // When I read the thread openThread("Reply1"); + + // Then the room and thread are read assertRead(room2); + assertReadThread("Reply1"); }); }); }); @@ -899,38 +993,47 @@ describe("Read receipts", () => { describe("in threads", () => { // XXX: fails because we see a dot instead of an unread number - probably the server and client disagree it.skip("An edit of a threaded message makes the room unread", () => { + // Given we have read the thread goTo(room1); receiveMessages(room2, ["Msg1", threadedOff("Msg1", "Resp1")]); assertUnread(room2, 2); - goTo(room2); openThread("Msg1"); assertRead(room2); goTo(room1); + // When a message inside it is edited receiveMessages(room2, [editOf("Resp1", "Edit1")]); + + // Then the room and thread are unread assertUnread(room2, 1); + goTo(room2); + assertUnreadThread("Msg1"); }); // XXX: fails because we see a dot instead of an unread number - probably the server and client disagree it.skip("Reading an edit of a threaded message makes the room read", () => { + // Given an edited thread message is making the room unread goTo(room1); receiveMessages(room2, ["Msg1", threadedOff("Msg1", "Resp1")]); assertUnread(room2, 2); - goTo(room2); openThread("Msg1"); assertRead(room2); goTo(room1); - receiveMessages(room2, [editOf("Resp1", "Edit1")]); assertUnread(room2, 1); + // When I read it goTo(room2); openThread("Msg1"); + + // Then the room and thread are read assertRead(room2); + assertReadThread("Msg1"); }); // XXX: fails because the room is still "bold" even though the notification counts all disappear it.skip("Marking a room as read after an edit in a thread makes it read", () => { + // Given an edit in a thread is making the room unread goTo(room1); receiveMessages(room2, ["Msg1", threadedOff("Msg1", "Resp1"), editOf("Resp1", "Edit1")]); assertUnread(room2, 3); // TODO: the edit counts as a message! @@ -941,7 +1044,7 @@ describe("Read receipts", () => { // Then it is read assertRead(room2); }); - // XXX: fails because the room is still "bold" even though the notification counts all disappear + // XXX: fails because the unread dot remains after marking as read it.skip("Editing a thread message after marking as read makes the room unread", () => { // Given a room is marked as read goTo(room1); @@ -958,12 +1061,18 @@ describe("Read receipts", () => { }); // XXX: fails because we see a dot instead of an unread number - probably the server and client disagree it.skip("A room with an edited threaded message is still unread after restart", () => { + // Given an edit in a thread is making a room unread goTo(room1); - receiveMessages(room2, ["Msg1", threadedOff("Msg1", "Resp1"), editOf("Resp1", "Edit1")]); - assertUnread(room2, 3); + receiveMessages(room2, ["Msg1", threadedOff("Msg1", "Resp1")]); + markAsRead(room2); + receiveMessages(room2, [editOf("Resp1", "Edit1")]); + assertUnread(room2, 1); + // When I restart saveAndReload(); - assertUnread(room2, 3); + + // Then is it still unread + assertUnread(room2, 1); }); it("A room where all threaded edits are read is still read after restart", () => { goTo(room2); @@ -982,11 +1091,13 @@ describe("Read receipts", () => { goTo(room1); receiveMessages(room2, ["Msg1", threadedOff("Msg1", "Resp1"), editOf("Resp1", "Edit1")]); assertUnread(room2, 3); - markAsRead(room2); assertRead(room2); + // When I restart saveAndReload(); + + // It is still read assertRead(room2); }); }); @@ -994,23 +1105,30 @@ describe("Read receipts", () => { describe("thread roots", () => { // XXX: fails because we see a dot instead of an unread number - probably the server and client disagree it.skip("An edit of a thread root makes the room unread", () => { + // Given I have read a thread goTo(room1); receiveMessages(room2, ["Msg1", threadedOff("Msg1", "Resp1")]); assertUnread(room2, 2); - goTo(room2); openThread("Msg1"); assertRead(room2); goTo(room1); + // When the thread root is edited receiveMessages(room2, [editOf("Msg1", "Edit1")]); + + // Then the room is unread but not the thread assertUnread(room2, 1); + goTo(room2); + assertRead(room2); + assertReadThread("Msg1"); }); - it.skip("Reading an edit of a thread root makes the room read", () => { + it("Reading an edit of a thread root makes the room read", () => { // Given a fully-read thread exists goTo(room2); receiveMessages(room2, ["Msg1", threadedOff("Msg1", "Resp1")]); openThread("Msg1"); + assertRead(room2); goTo(room1); assertRead(room2); @@ -1025,10 +1143,77 @@ describe("Read receipts", () => { goTo(room1); assertRead(room2); }); - it.skip("Marking a room as read after an edit of a thread root makes it read", () => {}); - it.skip("Editing a thread root after marking as read makes the room unread", () => {}); - it.skip("Marking a room as read after an edit of a thread root that is a reply makes it read", () => {}); - it.skip("Editing a thread root that is a reply after marking as read makes the room unread but not the thread", () => {}); + // XXX: fails because it shows a dot instead of unread count + it.skip("Editing a thread root after reading makes the room unread", () => { + // Given a fully-read thread exists + goTo(room2); + receiveMessages(room2, ["Msg1", threadedOff("Msg1", "Resp1")]); + openThread("Msg1"); + assertRead(room2); + goTo(room1); + + // When the thread root is edited + receiveMessages(room2, [editOf("Msg1", "Msg1 Edit1")]); + + // Then the room becomes unread + assertUnread(room2, 1); + }); + // XXX: fails because the room has an unread dot after I marked it as read + it.skip("Marking a room as read after an edit of a thread root makes it read", () => { + // Given a fully-read thread exists + goTo(room2); + receiveMessages(room2, ["Msg1", threadedOff("Msg1", "Resp1")]); + openThread("Msg1"); + assertRead(room2); + goTo(room1); + assertRead(room2); + + // When the thread root is edited + receiveMessages(room2, [editOf("Msg1", "Msg1 Edit1")]); + + // And I mark the room as read + markAsRead(room2); + + // Then the room becomes read and stays read + assertRead(room2); + goTo(room1); + assertRead(room2); + }); + // XXX: fails because the room has an unread dot after I marked it as read + it.skip("Editing a thread root that is a reply after marking as read makes the room unread but not the thread", () => { + // Given a thread based on a reply exists and is read because it is marked as read + goTo(room1); + receiveMessages(room2, ["Msg", replyTo("Msg", "Reply"), threadedOff("Reply", "InThread")]); + assertUnread(room2, 3); + markAsRead(room2); + assertRead(room2); + + // When I edit the thread root + receiveMessages(room1, [editOf("Reply", "Edited Reply")]); + + // Then the room is unread + assertUnread(room2, 1); + goTo(room2); + + // But the thread is still read (because the root is not part of the thread) + assertReadThread("EditedReply"); + }); + // XXX: fails because the room has an unread dot after I marked it as read + it.skip("Marking a room as read after an edit of a thread root that is a reply makes it read", () => { + // Given a thread based on a reply exists and the reply has been edited + goTo(room1); + receiveMessages(room2, ["Msg", replyTo("Msg", "Reply"), threadedOff("Reply", "InThread")]); + receiveMessages(room2, [editOf("Reply", "Edited Reply")]); + assertUnread(room2, 3); + + // When I mark the room as read + markAsRead(room2); + + // Then the room and thread are read + assertRead(room2); + goTo(room2); + assertReadThread("Edited Reply"); + }); }); });