Added puppeteer tests for `recent topic view`. #18170

aryanshridhar · 2021-04-15T20:00:44Z

Added puppeteer tests for Recent topic View as listed within #18031 and fixed a minor typo within drafts.ts.

Updated checklist:

Filters selection -- ce7b593
Search for topics -- 88c65f4
Clicking on row elements like stream and topic takes them to appropriate narrow -- 3c0f9a7
Read and mute action buttons work properly -- f3886f5
Compose actions work as intended.

Issues:

Added few comments below addressing the issues.

Would appreciate a review @amanagr.

Also, Thanks to @Riken-Shah for guiding me with the hurdles faced with nondeterminstic failures.(Still need to fix them though!)

Rectified a minor typo by renaming `dissapear` to `disappear`.

Added puppeteer test for `recent-topic` view at the time of initial page load. Here, `initial page load` refers to the point when puppeteer navigates to recent topic view using the left sidebar.

…view. Added puppeteer tests for elliptical filters within recent topic view. The commit adds the following tests: Single selection: - test_all_filter_button: Tests for `All` elliptical filter button. - test_include_muted_filter_button: Tests for `Include muted` filter. - test_unread_filter_button: Tests for `Unread` filter button. - test_participated_filter_button: Tests for `Participated` elliptical filter. Multiple selection (when multiple filters are triggered): - test_include_muted_and_unread_filter_button: Tests for `Include muted` as well as `Unread` filter button. - test_unread_and_participated_filter_button: Tests for `Unread` and `Participated` filter buttons. - test_include_muted_and_participated_filter_button: Tests for `Include Muted` and `Participated` elliptical filters. - test_all_filters: Tests when all the elliptical filter buttons are triggered. Fixes part of zulip#18031.

The following commit adds tests for stream and topic navigation within recent topic view. When clicked row elements like stream/ topic it navigates to their appropriate narrow. The tests include following functions: - test_stream_navigation: Tests for stream navigation, i.e, when clicked upon certain stream from recent topic view. - test_topic_navigation: Tests for topic navigation when clicked upon a certain topic. - test_recent_topic_hotkey: Tests for `t` keyboard hotkey which navigates back to `Recent topic view` from a particular narrow. Fixes part of zulip#18031.

Added puppeteer tests for search input field within recent topic view, which filters the topics with respect to the text passed within the input field. The commit adds the following test function: - test_filter_topic_input: Filters the recent topic feed when text `nas` is passed within the `filter topic` input field. Fixes part of zulip#18031.

…pic. Added tests for read and mute topic row actions within recent topic view. The commit adds the following tests: - test_mark_as_read_button: Tests for `mark as read` button by marking the `plotter` topic as read. - test_muted_topic_button: Tests for `mute topic` button by muting the `URLS` topic row. Fixes part of zulip#18031.

aryanshridhar · 2021-04-15T20:06:40Z

frontend_tests/puppeteer_tests/recent-topic.ts

+        recent_topic_page,
+        no_of_topics,
+    );
+}


This is causing a nondeterminstic failure when targeted it to run for 100x in CI. Here's the link to workflow which is failing at around 20ish iteration.
As the workflow suggestes, it's causing because of this (wait_for_topics_to_filter) block of function since the waitForFunction isn't returning a truth value and hence causing a TimeoutError error.

Looking to the artifacts generated, It's surprising to see that the filter input still contains some text. Here's the screenshot:

Would love any suggestions regarding it :)

Edit: The main purpose for this function is to wait until all the topics are filtered once any query is typed in filter input field (Since it takes a few milliseconds for the recent topic ListWidget to filter).

Edit 2: Added a different workaround for the function and triggered the workflow file again. Let's hope I get lucky this time :)

Edit 3: Ahh! Still failing!

After checking the screenshot it looks like the input value was not fully typed.

So, it's not the issue with this code.

The most probably fix for this is to when you type in filter input make sure you click it first then wait until it is focused then start writing.

Check the last point of this https://zulip.readthedocs.io/en/latest/testing/testing-with-puppeteer.html#debugging-puppeteer-tests.

aryanshridhar · 2021-04-15T20:10:39Z

frontend_tests/puppeteer_tests/recent-topic.ts

+    await trigger_filter_button(page, unread_filter_button);
+
+    const stream_names = await get_stream_names(page);
+    assert.deepStrictEqual(stream_names, []);


By default there isn't any unread topic/muted topic when the puppeteer logs in. This leads to Unread filter button returns a empty array of topics and Include Muted filter button to return the same result as All filter button.
(Mentioned this as I somehow felt like this isn't the right way to proceed.)

amanagr · 2021-04-16T07:01:46Z

@Riken-Shah can you take point in reviewing this? I am not the best person to review these tests. I will take a look later.

Riken-Shah

@aryanshridhar posted a batch of comments.

Let me know if you need any help!

Riken-Shah · 2021-04-24T15:16:33Z

frontend_tests/puppeteer_tests/recent-topic.ts

+const recent_topic_page = "#recent_topics_view";
+
+// Returns an array based on all text found within the selector passed.
+async function get_selector_text(page: Page, selector: string): Promise<string[]> {


The function name can be improved.
Something like get_all_element_text_from_selector?

Riken-Shah · 2021-04-24T15:21:24Z

frontend_tests/puppeteer_tests/recent-topic.ts

+
+import common from "../puppeteer_lib/common";
+
+const recent_topic_page = "#recent_topics_view";


We haven't done something like this on other tests.
I really like the idea of declaring common selectors at the top.

Riken-Shah · 2021-04-24T15:31:25Z

frontend_tests/puppeteer_tests/recent-topic.ts

@@ -16,6 +20,11 @@ async function get_selector_text(page: Page, selector: string): Promise<string[]
    return text;
 }

+async function trigger_filter_button(page: Page, selector: string): Promise<void> {


I don't think so we need this function here.
(Also, the function name doesn't justify what it does.)

But I really like this idea.
Maybe we can add this function in common.ts by the name wait_and_click.
And refactor all the puppeteer tests to use this.
Thoughts @chdinesh1089, @andersk?

Riken-Shah · 2021-04-24T19:05:07Z

frontend_tests/puppeteer_tests/recent-topic.ts

+        recent_topic_page,
+        no_of_topics,
+    );
+}


After checking the screenshot it looks like the input value was not fully typed.

So, it's not the issue with this code.

The most probably fix for this is to when you type in filter input make sure you click it first then wait until it is focused then start writing.

Check the last point of this https://zulip.readthedocs.io/en/latest/testing/testing-with-puppeteer.html#debugging-puppeteer-tests.

Riken-Shah · 2021-04-24T19:07:29Z

frontend_tests/puppeteer_tests/recent-topic.ts

+async function wait_for_topics_to_filter(page: Page, no_of_topics: number): Promise<void> {
+    await page.waitForSelector(recent_topic_page);
+
+    await page.waitForFunction(


Insted of this how about,

// Subtracting 1 from $("table:first tr").length because of heading row. await page.waitForFunction((no_of_topics: number) => ($("table:first tr").length - 1) === no_of_topics,{}, no_of_topics);

Riken-Shah · 2021-04-24T19:47:47Z

frontend_tests/puppeteer_tests/recent-topic.ts

+
+    for (let i = 0; i < input_text.length; i += 1) {
+        await page.keyboard.press("Backspace");
+    }


As you are using this function to only clear the value of #recent_topics_search.

How about this,

await page.waitForSelector(recent_topic_page); const recent_topic_search_selector = "#recent_topics_search"; await page.click(recent_topic_search_selector); await page.waitForFunction(() => $(`#recent_topics_search:focus`).attr("id") === "recent_topics_search"); await common.clear_and_type(page, recent_topic_search_selector, " "); // Wait for all topics to load back again. await wait_for_topics_to_filter(page, 7);

Also, update the name of the function.
Maybe clear_recent_topic_search ?

Tip: When you want to clear the value and then type you can use common.clear_and_type.

Riken-Shah · 2021-04-24T20:02:38Z

frontend_tests/puppeteer_tests/recent-topic.ts

+    await page.evaluate((topic: string) => {
+        const mark_as_read_icon = $(`.on_hover_topic_read[data-topic-name='${topic}']`);
+        mark_as_read_icon.trigger("click");
+    }, topic);


Why not await page.click(`.on_hover_topic_read[data-topic-name='${topic}']`);?

Tip: Avoid using evaluated js as much as you can.

Riken-Shah · 2021-04-24T20:08:45Z

frontend_tests/puppeteer_tests/recent-topic.ts

+        const mute_icon = $(`.on_hover_topic_mute[data-topic-name='${topic}']`);
+        mute_icon.trigger("click");
+    }, topic);
+}


Why not await page.click(`.on_hover_topic_mute[data-topic-name='${topic}']`);?

Riken-Shah · 2021-04-24T20:12:26Z

frontend_tests/puppeteer_tests/recent-topic.ts

@@ -5,6 +5,10 @@ import type {Page} from "puppeteer";
 import common from "../puppeteer_lib/common";

 const recent_topic_page = "#recent_topics_view";
+const all_filter_button = ".btn-recent-filters[data-filter='all']";


I don't think so we need this as a global variable as it's only used once.

Riken-Shah · 2021-04-24T20:30:25Z

frontend_tests/puppeteer_tests/recent-topic.ts

+
+async function test_stream_navigation(page: Page, stream_name: string): Promise<void> {
+    await page.waitForSelector(recent_topic_page, {visible: true});
+


I don't think so we need this function.

As we are using it only once.

You can study this function about how to navigate with the stream,

zulip/frontend_tests/puppeteer_tests/navigation.ts

Line 12 in bf056c8

async function navigate_using_left_sidebar(

Riken-Shah · 2021-04-24T20:49:25Z

frontend_tests/puppeteer_tests/recent-topic.ts

+
+async function test_topic_navigation(page: Page, topic_name: string): Promise<void> {
+    await page.waitForSelector(recent_topic_page, {visible: true});
+


I don't think so we need this function.

Reason -- Same as this comment.

Riken-Shah · 2021-04-24T20:53:21Z

@aryanshridhar After you are done with the changes, can you also check it by running 100x in Firefox mode?

zulipbot · 2021-09-10T17:33:21Z

Heads up @aryanshridhar, we just merged some commits that conflict with the changes your made in this pull request! You can review this repository's recent commits to see where the conflicts occur. Please rebase your feature branch against the upstream/main branch and resolve your pull request's merge conflicts accordingly.

aryanshridhar added 6 commits April 16, 2021 00:38

puppeteer_tests: Fix minor typo in drafts.ts puppeteer test.

5ffc2a7

Rectified a minor typo by renaming `dissapear` to `disappear`.

puppeteer_tests: Add test for recent topic view at initial page load.

1af61be

Added puppeteer test for `recent-topic` view at the time of initial page load. Here, `initial page load` refers to the point when puppeteer navigates to recent topic view using the left sidebar.

zulipbot added the size: XL label Apr 15, 2021

aryanshridhar commented Apr 15, 2021

View reviewed changes

aryanshridhar mentioned this pull request Apr 16, 2021

Add puppeteer tests for recent topics. #18031

Closed

5 tasks

Riken-Shah reviewed Apr 24, 2021

View reviewed changes

zulipbot added the has conflicts label Sep 10, 2021

timabbott force-pushed the main branch 2 times, most recently from 4ec3636 to 88b200c Compare August 18, 2023 23:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added puppeteer tests for `recent topic view`. #18170

Added puppeteer tests for `recent topic view`. #18170

aryanshridhar commented Apr 15, 2021

aryanshridhar Apr 15, 2021 •

edited

Riken-Shah Apr 24, 2021

aryanshridhar Apr 15, 2021

amanagr commented Apr 16, 2021

Riken-Shah left a comment

Riken-Shah Apr 24, 2021

Riken-Shah Apr 24, 2021

Riken-Shah Apr 24, 2021

Riken-Shah Apr 24, 2021

Riken-Shah Apr 24, 2021

Riken-Shah Apr 24, 2021

Riken-Shah Apr 24, 2021

Riken-Shah Apr 24, 2021

Riken-Shah Apr 24, 2021

Riken-Shah Apr 24, 2021

Riken-Shah Apr 24, 2021

Riken-Shah commented Apr 24, 2021

zulipbot commented Sep 10, 2021


		import common from "../puppeteer_lib/common";

		const recent_topic_page = "#recent_topics_view";


		async function test_stream_navigation(page: Page, stream_name: string): Promise<void> {
		await page.waitForSelector(recent_topic_page, {visible: true});


		async function test_topic_navigation(page: Page, topic_name: string): Promise<void> {
		await page.waitForSelector(recent_topic_page, {visible: true});

Added puppeteer tests for recent topic view. #18170

Are you sure you want to change the base?

Added puppeteer tests for recent topic view. #18170

Conversation

aryanshridhar commented Apr 15, 2021

aryanshridhar Apr 15, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

amanagr commented Apr 16, 2021

Riken-Shah left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Riken-Shah commented Apr 24, 2021

zulipbot commented Sep 10, 2021

Added puppeteer tests for `recent topic view`. #18170

Added puppeteer tests for `recent topic view`. #18170

aryanshridhar Apr 15, 2021 •

edited