[webkit-reviews] review granted: [Bug 224175] REGRESSION(r275267) [GLIB] API test /webkit/WebKitWebsiteData/configuration is failing : [Attachment 438007] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Sun Sep 12 23:14:25 PDT 2021


Carlos Garcia Campos <cgarcia at igalia.com> has granted Lauro Moura
<lmoura at igalia.com>'s request for review:
Bug 224175: REGRESSION(r275267) [GLIB] API test
/webkit/WebKitWebsiteData/configuration is failing
https://bugs.webkit.org/show_bug.cgi?id=224175

Attachment 438007: Patch

https://bugs.webkit.org/attachment.cgi?id=438007&action=review




--- Comment #3 from Carlos Garcia Campos <cgarcia at igalia.com> ---
Comment on attachment 438007
  --> https://bugs.webkit.org/attachment.cgi?id=438007
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=438007&action=review

> Tools/TestWebKitAPI/Tests/WebKitGLib/TestWebsiteData.cpp:807
> +    // Make sure the cache was opened before asking for it
> +    {
> +	   unsigned triesCount = 4;
> +	   bool createdCache = false;
> +	   while (!createdCache && triesCount--) {
> +	       auto domCacheResult =
test->runJavaScriptAndWaitUntilFinished("domCacheOpened", nullptr);
> +	       auto jsResult =
webkit_javascript_result_get_js_value(domCacheResult);
> +	       g_assert_true(jsc_value_is_boolean(jsResult));
> +	       if (jsc_value_to_boolean(jsResult))
> +		   createdCache = true;
> +	       else
> +		   test->wait(0.25);
> +	   }

I guess we could move this duplicated code to WebsiteDataTest class.

> Tools/TestWebKitAPI/glib/WebKitGLib/WebViewTest.cpp:256
> +void WebViewTest::waitUntilFileExists(const char *filename, double
intervalInSeconds, unsigned tries)

Why are intervalInSeconds and tries parameters if we never pass them
explicitly? or did I miss any call?

> Tools/TestWebKitAPI/glib/WebKitGLib/WebViewTest.cpp:260
> +    g_assert_true(g_file_test(filename, G_FILE_TEST_EXISTS));

I'm not sure about asserting here in a method called wait until. I would either
leave the infinite loop and test will timeout if file is never created, or keep
the tries but assert in the callers. Or keep the asser here, but rename the
method as something like assertFileIsCreated()


More information about the webkit-reviews mailing list