Skip to content

Commit

Permalink
Fix flaky Web Extension exception tests on debug
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=270997
rdar://124632659

Reviewed by Timothy Hatcher.

It appears that these error tests were timing out on the bots. This PR fixes them in two ways:
1) Upping the timeout on debug builds to 6 seconds (since debug builds are slower to run these tests)
2) Splitting the tests that are timing out into separate tests

* Source/WebKit/UIProcess/Extensions/Cocoa/WebExtensionContextCocoa.mm:
(WebKit::WebExtensionContext::scheduleBackgroundContentToUnload):
* Tools/TestWebKitAPI/Tests/WebKitCocoa/WKWebExtensionAPICookies.mm:
(TestWebKitAPI::TEST):
* Tools/TestWebKitAPI/Tests/WebKitCocoa/WKWebExtensionAPIScripting.mm:
(TestWebKitAPI::TEST):

Canonical link: https://commits.webkit.org/276126@main
  • Loading branch information
b-weinstein authored and xeenon committed Mar 14, 2024
1 parent e88ecb4 commit 2a19d02
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -3191,7 +3191,14 @@ static inline bool isNotRunningInTestRunner()
if (!m_backgroundWebView || extension().backgroundContentIsPersistent())
return;

static const auto delayBeforeUnloading = isNotRunningInTestRunner() ? 30_s : 3_s;

#ifdef NDEBUG
static const auto testRunnerDelayBeforeUnloading = 3_s;
#else
static const auto testRunnerDelayBeforeUnloading = 6_s;
#endif

static const auto delayBeforeUnloading = isNotRunningInTestRunner() ? 30_s : testRunnerDelayBeforeUnloading;

RELEASE_LOG_DEBUG(Extensions, "Scheduling background content to unload in %{public}.0f seconds", delayBeforeUnloading.seconds());

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@
@"permissions": @[ @"cookies" ],
};

TEST(WKWebExtensionAPICookies, Errors)
TEST(WKWebExtensionAPICookies, ErrorsRead)
{
auto *backgroundScript = Util::constructScript(@[
@"browser.test.assertThrows(() => browser.cookies.get({ url: 123, name: 'Test' }), /'url' is expected to be a string, but a number was provided/i)",
Expand All @@ -70,6 +70,15 @@
@"browser.test.assertThrows(() => browser.cookies.getAll({ session: 'bad' }), /'session' is expected to be a boolean, but a string was provided/i)",
@"browser.test.assertThrows(() => browser.cookies.getAll({ storeId: 123 }), /'storeId' is expected to be a string, but a number was provided/i)",

@"browser.test.notifyPass()",
]);

Util::loadAndRunExtension(cookiesManifest, @{ @"background.js": backgroundScript });
}

TEST(WKWebExtensionAPICookies, ErrorsWrite)
{
auto *backgroundScript = Util::constructScript(@[
@"browser.test.assertThrows(() => browser.cookies.set({ url: 123 }), /'url' is expected to be a string, but a number was provided/i)",
@"browser.test.assertThrows(() => browser.cookies.set({ url: '' }), /'url' value is invalid, because it must not be empty/i)",
@"browser.test.assertThrows(() => browser.cookies.set({ url: 'bad' }), /'url' value is invalid, because 'bad' is not a valid URL/i)",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@
},
};

TEST(WKWebExtensionAPIScripting, Errors)
TEST(WKWebExtensionAPIScripting, ErrorsExecuteScript)
{
auto *backgroundScript = Util::constructScript(@[
@"browser.test.assertThrows(() => browser.scripting.executeScript(), /a required argument is missing/i)",
Expand All @@ -76,6 +76,15 @@

@"browser.test.assertThrows(() => browser.scripting.executeScript({'target': { 'tabId': 0 }, world: 'world', files: ['path/to/file']}), /it must specify either 'ISOLATED' or 'MAIN'./i)",

@"browser.test.notifyPass()"
]);

Util::loadAndRunExtension(scriptingManifest, @{ @"background.js": backgroundScript });
}

TEST(WKWebExtensionAPIScripting, ErrorsCSS)
{
auto *backgroundScript = Util::constructScript(@[
@"browser.test.assertThrows(() => browser.scripting.insertCSS(), /a required argument is missing./i)",
@"browser.test.assertThrows(() => browser.scripting.insertCSS({}), /missing required keys: 'target'./i)",
@"browser.test.assertThrows(() => browser.scripting.insertCSS({ target: {} }), /missing required keys: 'tabId'./i)",
Expand All @@ -90,6 +99,15 @@
@"browser.test.assertThrows(() => browser.scripting.removeCSS({target: { tabId: 0 } }), /it must specify either 'css' or 'files'./i)",
@"browser.test.assertThrows(() => browser.scripting.removeCSS({target: { tabId: '0' }, files: ['path/to/file'], css: 'css'}), /'tabId' is expected to be a number, but a string was provided./i)",

@"browser.test.notifyPass()"
]);

Util::loadAndRunExtension(scriptingManifest, @{ @"background.js": backgroundScript });
}

TEST(WKWebExtensionAPIScripting, ErrorsRegisteredContentScript)
{
auto *backgroundScript = Util::constructScript(@[
@"browser.test.assertThrows(() => browser.scripting.registerContentScripts(), /a required argument is missing/i)",
@"browser.test.assertThrows(() => browser.scripting.registerContentScripts({}), /an array is expected/i)",
@"browser.test.assertThrows(() => browser.scripting.registerContentScripts([{}]), /it is missing required keys: 'id'/i)",
Expand Down

0 comments on commit 2a19d02

Please sign in to comment.