Title: [282654] trunk/Tools
Revision
282654
Author
[email protected]
Date
2021-09-17 04:02:51 -0700 (Fri, 17 Sep 2021)

Log Message

REGRESSION(r275267) [GLIB] API test /webkit/WebKitWebsiteData/configuration is failing
https://bugs.webkit.org/show_bug.cgi?id=224175

Reviewed by Carlos Garcia Campos.

Some WebsiteData tests rely on checking whether some specific files
are created in the background. Currently, this is done through
waitUntilFileChanged(), which first g_file_query() whether the file
exists before entering a main loop which uses GFileMonitor. While this
worked most of the time, some tests were flaky due to likely the file
being created between the query call and the monitoring starting,
especially after revisions like r275267.

This commit replaces the waitUntilFileChanged calls with an explicit
check loop, like was done for the applicationCache file in the
configuration test.

Also, for the ITP test, there's no need to check for the file to be
deleted, as the ResourceLoadStatistics just clears the database and
recreates the schema, reusing the existing file.

Covered by existing tests.

* TestWebKitAPI/Tests/WebKitGLib/TestWebsiteData.cpp:
(testWebsiteDataConfiguration):
(testWebsiteDataITP):
(testWebsiteDataDOMCache):
* TestWebKitAPI/glib/TestExpectations.json:
* TestWebKitAPI/glib/WebKitGLib/WebViewTest.cpp:
(WebViewTest::assertFileIsCreated):
(WebViewTest::assertJavaScriptBecomesTrue):
* TestWebKitAPI/glib/WebKitGLib/WebViewTest.h:

Modified Paths

Diff

Modified: trunk/Tools/ChangeLog (282653 => 282654)


--- trunk/Tools/ChangeLog	2021-09-17 10:33:33 UTC (rev 282653)
+++ trunk/Tools/ChangeLog	2021-09-17 11:02:51 UTC (rev 282654)
@@ -1,3 +1,38 @@
+2021-09-17  Lauro Moura  <[email protected]>
+
+        REGRESSION(r275267) [GLIB] API test /webkit/WebKitWebsiteData/configuration is failing
+        https://bugs.webkit.org/show_bug.cgi?id=224175
+
+        Reviewed by Carlos Garcia Campos.
+
+        Some WebsiteData tests rely on checking whether some specific files
+        are created in the background. Currently, this is done through
+        waitUntilFileChanged(), which first g_file_query() whether the file
+        exists before entering a main loop which uses GFileMonitor. While this
+        worked most of the time, some tests were flaky due to likely the file
+        being created between the query call and the monitoring starting,
+        especially after revisions like r275267.
+
+        This commit replaces the waitUntilFileChanged calls with an explicit
+        check loop, like was done for the applicationCache file in the
+        configuration test.
+
+        Also, for the ITP test, there's no need to check for the file to be
+        deleted, as the ResourceLoadStatistics just clears the database and
+        recreates the schema, reusing the existing file.
+
+        Covered by existing tests.
+
+        * TestWebKitAPI/Tests/WebKitGLib/TestWebsiteData.cpp:
+        (testWebsiteDataConfiguration):
+        (testWebsiteDataITP):
+        (testWebsiteDataDOMCache):
+        * TestWebKitAPI/glib/TestExpectations.json:
+        * TestWebKitAPI/glib/WebKitGLib/WebViewTest.cpp:
+        (WebViewTest::assertFileIsCreated):
+        (WebViewTest::assertJavaScriptBecomesTrue):
+        * TestWebKitAPI/glib/WebKitGLib/WebViewTest.h:
+
 2021-09-17  Carlos Garcia Campos  <[email protected]>
 
         [GTK][a11y] Add a build option to enable ATSPI

Modified: trunk/Tools/TestWebKitAPI/Tests/WebKitGLib/TestWebsiteData.cpp (282653 => 282654)


--- trunk/Tools/TestWebKitAPI/Tests/WebKitGLib/TestWebsiteData.cpp	2021-09-17 10:33:33 UTC (rev 282653)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKitGLib/TestWebsiteData.cpp	2021-09-17 11:02:51 UTC (rev 282654)
@@ -174,7 +174,7 @@
     test->waitUntilLoadFinished();
     test->runJavaScriptAndWaitUntilFinished("window.indexedDB.open('TestDatabase');", nullptr);
     GUniquePtr<char> indexedDBDirectory(g_build_filename(Test::dataDirectory(), "indexeddb", nullptr));
-    test->waitUntilFileChanged(indexedDBDirectory.get(), G_FILE_MONITOR_EVENT_CREATED);
+    test->assertFileIsCreated(indexedDBDirectory.get());
     g_assert_cmpstr(indexedDBDirectory.get(), ==, webkit_website_data_manager_get_indexeddb_directory(test->m_manager));
     g_assert_true(g_file_test(indexedDBDirectory.get(), G_FILE_TEST_IS_DIR));
 
@@ -183,10 +183,7 @@
     GUniquePtr<char> applicationCacheDirectory(g_build_filename(Test::dataDirectory(), "appcache", nullptr));
     g_assert_cmpstr(applicationCacheDirectory.get(), ==, webkit_website_data_manager_get_offline_application_cache_directory(test->m_manager));
     GUniquePtr<char> applicationCacheDatabase(g_build_filename(applicationCacheDirectory.get(), "ApplicationCache.db", nullptr));
-    unsigned triesCount = 4;
-    while (!g_file_test(applicationCacheDatabase.get(), G_FILE_TEST_IS_REGULAR) && --triesCount)
-        test->wait(0.25);
-    g_assert_cmpuint(triesCount, >, 0);
+    test->assertFileIsCreated(applicationCacheDatabase.get());
 
     GUniquePtr<char> diskCacheDirectory(g_build_filename(Test::dataDirectory(), "disk-cache", nullptr));
     g_assert_cmpstr(diskCacheDirectory.get(), ==, webkit_website_data_manager_get_disk_cache_directory(test->m_manager));
@@ -201,16 +198,19 @@
 
     test->runJavaScriptAndWaitUntilFinished("navigator.serviceWorker.register('./some-dummy.js');", nullptr);
     GUniquePtr<char> swRegistrationsDirectory(g_build_filename(Test::dataDirectory(), "serviceworkers", nullptr));
-    test->waitUntilFileChanged(swRegistrationsDirectory.get(), G_FILE_MONITOR_EVENT_CREATED);
+    test->assertFileIsCreated(swRegistrationsDirectory.get());
     g_assert_cmpstr(swRegistrationsDirectory.get(), ==, webkit_website_data_manager_get_service_worker_registrations_directory(test->m_manager));
     g_assert_true(g_file_test(swRegistrationsDirectory.get(), G_FILE_TEST_IS_DIR));
 
-    test->runJavaScriptAndWaitUntilFinished("caches.open('my-cache');", nullptr);
+    test->runJavaScriptAndWaitUntilFinished("let domCacheOpened = false; caches.open('my-cache').then(() => { domCacheOpened = true});", nullptr);
     GUniquePtr<char> domCacheDirectory(g_build_filename(Test::dataDirectory(), "dom-cache", nullptr));
-    test->waitUntilFileChanged(domCacheDirectory.get(), G_FILE_MONITOR_EVENT_CREATED);
+    test->assertFileIsCreated(domCacheDirectory.get());
     g_assert_cmpstr(domCacheDirectory.get(), ==, webkit_website_data_manager_get_dom_cache_directory(test->m_manager));
     g_assert_true(g_file_test(domCacheDirectory.get(), G_FILE_TEST_IS_DIR));
 
+    // Make sure the cache was opened before asking to clear it to avoid it being re-created behind our backs.
+    test->assertJavaScriptBecomesTrue("domCacheOpened");
+
     // Clear all persistent caches, since the data dir is common to all test cases. Note: not cleaning the HSTS cache here as its data
     // is needed for the HSTS tests, where data cleaning will be tested.
     static const WebKitWebsiteDataTypes persistentCaches = static_cast<WebKitWebsiteDataTypes>(WEBKIT_WEBSITE_DATA_DISK_CACHE | WEBKIT_WEBSITE_DATA_LOCAL_STORAGE
@@ -708,8 +708,7 @@
 
     test->loadURI(kServer->getURIForPath("/empty").data());
     test->waitUntilLoadFinished();
-
-    test->waitUntilFileChanged(itpDatabaseFile.get(), G_FILE_MONITOR_EVENT_CREATED);
+    test->assertFileIsCreated(itpDatabaseFile.get());
     g_assert_true(g_file_test(itpDirectory, G_FILE_TEST_IS_DIR));
     g_assert_true(g_file_test(itpDatabaseFile.get(), G_FILE_TEST_IS_REGULAR));
 
@@ -734,7 +733,6 @@
     // Clear all.
     static const WebKitWebsiteDataTypes cacheAndITP = static_cast<WebKitWebsiteDataTypes>(WEBKIT_WEBSITE_DATA_ITP | WEBKIT_WEBSITE_DATA_MEMORY_CACHE | WEBKIT_WEBSITE_DATA_DISK_CACHE);
     test->clear(cacheAndITP, 0);
-    test->waitUntilFileChanged(itpDatabaseFile.get(), G_FILE_MONITOR_EVENT_DELETED);
 
     webkit_website_data_manager_set_itp_enabled(test->m_manager, FALSE);
     g_assert_false(webkit_website_data_manager_get_itp_enabled(test->m_manager));
@@ -779,8 +777,11 @@
 
     test->loadURI(kServer->getURIForPath("/").data());
     test->waitUntilLoadFinished();
-    test->runJavaScriptAndWaitUntilFinished("async function openDOMCache() { await window.caches.open('TestDOMCache'); } openDOMCache();", nullptr);
+    test->runJavaScriptAndWaitUntilFinished("let domCacheOpened = false; window.caches.open('TestDOMCache').then(() => { domCacheOpened = true});", nullptr);
 
+    // Make sure the cache was opened before asking for it
+    test->assertJavaScriptBecomesTrue("domCacheOpened");
+
     dataList = test->fetch(WEBKIT_WEBSITE_DATA_DOM_CACHE);
     g_assert_nonnull(dataList);
     g_assert_cmpuint(g_list_length(dataList), ==, 1);

Modified: trunk/Tools/TestWebKitAPI/glib/TestExpectations.json (282653 => 282654)


--- trunk/Tools/TestWebKitAPI/glib/TestExpectations.json	2021-09-17 10:33:33 UTC (rev 282653)
+++ trunk/Tools/TestWebKitAPI/glib/TestExpectations.json	2021-09-17 11:02:51 UTC (rev 282654)
@@ -106,23 +106,6 @@
             },
             "/webkit/WebKitWebsiteData/storage": {
                 "expected": { "wpe": {"status": ["PASS", "FAIL"]}}
-            },
-            "/webkit/WebKitWebsiteData/dom-cache": {
-                "expected": {
-                    "gtk": {"status": ["PASS", "FAIL"], "bug": "webkit.org/b/213785"},
-                    "wpe": {"status": ["PASS", "FAIL"], "bug": "webkit.org/b/213785"}
-                }
-            },
-            "/webkit/WebKitWebsiteData/itp": {
-                "expected": {
-                    "all_before_bug_229807": {"status": ["PASS", "TIMEOUT"], "bug": "webkit.org/b/224175"},
-                    "all": {"status": ["FAIL"], "bug": "webkit.org/b/229807"}
-                }
-            },
-            "/webkit/WebKitWebsiteData/configuration": {
-                "expected": {
-                    "all": {"status": ["FAIL"], "bug": "webkit.org/b/224175"}
-                }
             }
         }
     },

Modified: trunk/Tools/TestWebKitAPI/glib/WebKitGLib/WebViewTest.cpp (282653 => 282654)


--- trunk/Tools/TestWebKitAPI/glib/WebKitGLib/WebViewTest.cpp	2021-09-17 10:33:33 UTC (rev 282653)
+++ trunk/Tools/TestWebKitAPI/glib/WebKitGLib/WebViewTest.cpp	2021-09-17 11:02:51 UTC (rev 282654)
@@ -253,22 +253,30 @@
     webkit_web_view_set_editable(m_webView, editable);
 }
 
-static void directoryChangedCallback(GFileMonitor*, GFile* file, GFile*, GFileMonitorEvent event, WebViewTest* test)
+void WebViewTest::assertFileIsCreated(const char *filename)
 {
-    if (event == test->m_expectedFileChangeEvent && g_file_equal(file, test->m_monitoredFile.get()))
-        test->quitMainLoop();
+    constexpr double intervalInSeconds = 0.25;
+    unsigned tries = 4;
+    while (!g_file_test(filename, G_FILE_TEST_EXISTS) && --tries)
+        wait(intervalInSeconds);
+    g_assert_true(g_file_test(filename, G_FILE_TEST_EXISTS));
 }
 
-void WebViewTest::waitUntilFileChanged(const char* filename, GFileMonitorEvent event)
+void WebViewTest::assertJavaScriptBecomesTrue(const char* _javascript_)
 {
-    m_monitoredFile = adoptGRef(g_file_new_for_path(filename));
-    m_expectedFileChangeEvent = event;
-    GRefPtr<GFile> directory = adoptGRef(g_file_get_parent(m_monitoredFile.get()));
-    GRefPtr<GFileMonitor> monitor = adoptGRef(g_file_monitor_directory(directory.get(), G_FILE_MONITOR_NONE, nullptr, nullptr));
-    g_assert(monitor.get());
-    g_signal_connect(monitor.get(), "changed", G_CALLBACK(directoryChangedCallback), this);
-    if (!g_file_query_exists(m_monitoredFile.get(), nullptr))
-        g_main_loop_run(m_mainLoop);
+    unsigned triesCount = 4;
+    bool becameTrue = false;
+    while (!becameTrue && triesCount--) {
+        auto jsResult = runJavaScriptAndWaitUntilFinished(_javascript_, nullptr);
+        auto jsValue = webkit_javascript_result_get_js_value(jsResult);
+        if (jsc_value_is_boolean(jsValue) && jsc_value_to_boolean(jsValue)) {
+            becameTrue = true;
+            break;
+        }
+
+        wait(0.25);
+    }
+    g_assert_true(becameTrue);
 }
 
 static void resourceGetDataCallback(GObject* object, GAsyncResult* result, gpointer userData)

Modified: trunk/Tools/TestWebKitAPI/glib/WebKitGLib/WebViewTest.h (282653 => 282654)


--- trunk/Tools/TestWebKitAPI/glib/WebKitGLib/WebViewTest.h	2021-09-17 10:33:33 UTC (rev 282653)
+++ trunk/Tools/TestWebKitAPI/glib/WebKitGLib/WebViewTest.h	2021-09-17 11:02:51 UTC (rev 282654)
@@ -52,8 +52,9 @@
     void waitUntilLoadFinished();
     void waitUntilTitleChangedTo(const char* expectedTitle);
     void waitUntilTitleChanged();
-    void waitUntilFileChanged(const char*, GFileMonitorEvent);
     void waitUntilIsWebProcessResponsiveChanged();
+    void assertFileIsCreated(const char*);
+    void assertJavaScriptBecomesTrue(const char*);
     void resizeView(int width, int height);
     void hideView();
     void selectAll();
@@ -109,8 +110,6 @@
     size_t m_resourceDataSize { 0 };
     cairo_surface_t* m_surface { nullptr };
     bool m_expectedWebProcessCrash { false };
-    GRefPtr<GFile> m_monitoredFile;
-    GFileMonitorEvent m_expectedFileChangeEvent;
 
 #if PLATFORM(GTK)
     GtkWidget* m_parentWindow { nullptr };
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to