TestCafe wait for results #878

yen-tt · 2021-07-12T20:56:19Z

Update cafeTest acceptance test

Added waitOnSearchComplete in verticalResults.js to wait till query responses return status 200 and searchComplete state appears on page.
Register requestLogger to each test to track vertical request and response to Answers API endpoint
Added IE11nocachehook from SDK
From oliver's acceptance test failure investigation..
- first point: the test first function call is verticalResults.scrollToBottom(), which wait on this._resultsWrapper when perform scroll on page. This resultsWrapper will always exist because it's set in our page template. The test didn't account for waiting on the actual search result div. Calling waitOnSearchComplete before scrolling would prevent this issue.
- second point: removed the arbitrary wait time 1s in dragLeft(). Replaced with waiting for the response to be returned first using searchComplete state.
Updated tests to wait for results before calling getNumResults() which check if results exists after a search. (From testCafe documentation, Selector wait/timeouts functionality have no effect on Selector.exists and Selector.count properties. These properties are evaluated immediately.)

J=SLAP-1425
TEST= auto

ran testCafe locally to verify tests and wait time in selectors

J=SLAP-1425 TEST= manual & auto

coveralls · 2021-07-12T20:58:23Z

Coverage remained the same at 6.67% when pulling 0881390 on dev/testcafe-acceptance-test into 1cb987e on develop.

tests/acceptance/blocks/verticalresults.js

tmeyer2115 · 2021-07-13T13:43:18Z

tests/acceptance/blocks/verticalresults.js

+   * (default selector timeout of 10 seconds)
+   */
+   async waitOnSearchComplete () {
+    if(!this._preSearch.exists) {


For my own edification, what is preSearch?

it's one of the search states from SDK (presearch, loading, completed). I originally thought initial search on page may go from presearch to completed, skipping loading state. Turns out it's just testCafe unable to catch the loading transition. Updated to track query request/responses instead!

tmeyer2115 · 2021-07-13T13:45:57Z

tests/acceptance/blocks/verticalresults.js

+      await t.expect(this._searchLoading.exists).ok();
+    }
+    await this._searchComplete();
+    await t.expect(this._searchComplete.exists).ok();


Does await this._searchComplete() being successful imply t.expect(this._searchComplete.exists).ok() automatically?

test would still consider passed if await this._searchComplete() doesn't find any matching element

tmeyer2115 · 2021-07-13T13:48:47Z

tests/acceptance/blocks/verticalresults.js

+   * wait for results to return based on search complete state 
+   * (default selector timeout of 10 seconds)
+   */
+   async waitOnSearchComplete () {


If someone calls this method, but no search has actually been executed, will it timeout? I'm wondering if we should handle that situation gracefully, like it returns immediately. Maybe not something we need to worry about, just a thought.

in this method, and in the updated one, it will timeout eventually. I think if no search is executed and someone uses this method, meaning they are expecting a search or result returned of some sort, it make sense to indicate a failure here to notify that the expected behavior is not met

cea2aj · 2021-07-13T17:20:50Z

tests/acceptance/blocks/verticalresults.js

+    const waitTimeInterval = 200;
+    let totalWaitTime = 0;
+    while (totalWaitTime < responseWaitTimeout 
+      && this._queryRequestLogger.requests.find(r => !r.response || r.response.statusCode !== 200)) {


Is this code waiting while an invalid response exists? is there a reason for that approach rather than waiting until a successful response has been logged?

Is it possible to extract the query logger lookup into a variable or a function so that this logic is a litter easier to follow?

Logger will hold all request/response throughout a test so just checking for a successful response might accidentally check on previous requests. But, after some thoughts, I updated this method check for a successful response in the loop, and clear out the logger at the end of this method. This make sure we are always verifying the latest response.

tests/acceptance/blocks/verticalresults.js

oshi97 · 2021-07-14T03:44:23Z

tests/acceptance/suites/vertical-full-page-map-with-filters.js


 fixture`Vertical Full Page Map with Filters and Clusters`
  .page(`http://localhost:${PORT}/locations_full_page_map_with_filters`)
+  .beforeEach(async t => {
+    await VerticalResults.registerLogger(t);
+    await registerIE11NoCacheHook(t);


should the ie11 hook go inside registerLogger? like, is there ever a time we would call registerLogger without also registering the ie11 hook?

yeah, that would make sense. Updated!

oshi97 · 2021-07-14T15:55:37Z

tests/acceptance/blocks/verticalresults.js

+   * Wait for results to load on page by checking query response status and searchComplete state
+   * (timeout is set to 10 seconds)
+   */
+  async waitOnSearchComplete() {


Sorry I know I just recently approved this, but after looking at the SDK half of this item maybe this waitOnSearchComplete should go into its own request logger class instead of inside the VerticalResults component's class?

I agree, if we can move this to it's own class we could easily expand it to universal search. Yen, if we have a new class such as SearchRequestLogger, we could have a registerVerticalSearchLogger and a registerUniversalSearchLogger function so that we could set it up for either type of search

I see, yeah that sounds good

TestCafe wait for results

a4c042e

J=SLAP-1425 TEST= manual & auto

tmeyer2115 reviewed Jul 13, 2021

View reviewed changes

yen-tt added the WIP label Jul 13, 2021

track response status instead of loading state

52a14a2

yen-tt removed the WIP label Jul 13, 2021

yen-tt requested review from tmeyer2115, oshi97, cea2aj and cjiang2000 July 13, 2021 16:24

handle ie11 no cache policy

ad31563

cea2aj reviewed Jul 13, 2021

View reviewed changes

tests/acceptance/blocks/verticalresults.js Outdated Show resolved Hide resolved

Yen Truong added 2 commits July 13, 2021 13:57

update handle ie11nocachehook

cf58836

update waitOnSearchComplete

80d526e

yen-tt requested a review from cea2aj July 13, 2021 21:04

oshi97 reviewed Jul 14, 2021

View reviewed changes

Yen Truong added 2 commits July 14, 2021 08:44

move ie11 hook call to registerLogger

1c3f480

equal comparison

e0fb742

yen-tt requested a review from oshi97 July 14, 2021 13:54

wait for contains

914e2f9

oshi97 approved these changes Jul 14, 2021

View reviewed changes

oshi97 reviewed Jul 14, 2021

View reviewed changes

yen-tt requested a review from oshi97 July 14, 2021 16:59

yen-tt force-pushed the dev/testcafe-acceptance-test branch from f4362c7 to f8e9b26 Compare July 14, 2021 17:03

Yen Truong added 2 commits July 14, 2021 13:22

extract logger and wait on result to new class

e48ac04

constant name change

d27a6a6

yen-tt force-pushed the dev/testcafe-acceptance-test branch 2 times, most recently from bafb612 to b496623 Compare July 14, 2021 20:14

remove extra check

0881390

yen-tt force-pushed the dev/testcafe-acceptance-test branch from b496623 to 0881390 Compare July 14, 2021 20:15

oshi97 approved these changes Jul 14, 2021

View reviewed changes

yen-tt merged commit bb86a14 into develop Jul 15, 2021

yen-tt deleted the dev/testcafe-acceptance-test branch July 15, 2021 12:51

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

TestCafe wait for results #878

TestCafe wait for results #878

yen-tt commented Jul 12, 2021 •

edited

Loading

coveralls commented Jul 12, 2021 •

edited

Loading

tmeyer2115 Jul 13, 2021

yen-tt Jul 13, 2021

tmeyer2115 Jul 13, 2021

yen-tt Jul 13, 2021

tmeyer2115 Jul 13, 2021

yen-tt Jul 13, 2021

cea2aj Jul 13, 2021

cea2aj Jul 13, 2021

yen-tt Jul 13, 2021

oshi97 Jul 14, 2021

yen-tt Jul 14, 2021

oshi97 Jul 14, 2021

cea2aj Jul 14, 2021

yen-tt Jul 14, 2021

TestCafe wait for results #878

TestCafe wait for results #878

Conversation

yen-tt commented Jul 12, 2021 • edited Loading

coveralls commented Jul 12, 2021 • edited Loading

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yen-tt commented Jul 12, 2021 •

edited

Loading

coveralls commented Jul 12, 2021 •

edited

Loading