Bug 142447 – Add some WorkQueue tests to TestWebKitAPI

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Brent Fulgham]

WorkQueue is used heavily in WebKit2 code, but we don't really have good test coverage for it. We should exercise it as part of our normal TestWebKitAPI tests.

[Brent Fulgham]

Created attachment 248181 [details]
Patch

[Brent Fulgham]

Comment on attachment 248181 [details]
Patch

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

> Tools/TestWebKitAPI/Tests/WTF/WorkQueue.cpp:110
> +    std::this_thread::sleep_for(std::chrono::nanoseconds(1000));

I'm just trying to simulate some lengthy bit of work here. Maybe there's a better way?

> Tools/TestWebKitAPI/Tests/WTF/WorkQueue.cpp:132
> +    queue1->dispatchAfter(std::chrono::milliseconds(100), finishedTests);

I want to verify that the two queues work independently. Not sure if there is a better way to do this? Maybe more condition variables to coordinate the order of things?

[Brent Fulgham]

Created attachment 248182 [details]
Patch v2 (Activate on Windows)

[Brent Fulgham]

Comment on attachment 248182 [details]
Patch v2 (Activate on Windows)

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

> Tools/TestWebKitAPI/Tests/WTF/WorkQueue.cpp:124
> +    queue1->dispatchAfter(std::chrono::milliseconds(100), finishedTests);

I'm trying to simulate two queues performing different amounts of work. Maybe there's a better way than these delays...

[Brent Fulgham]

Created attachment 248203 [details]
Patch v3 (Update per weinig comments)

[Sam Weinig]

Comment on attachment 248203 [details]
Patch v3 (Update per weinig comments)

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

> Tools/TestWebKitAPI/Tests/WTF/WorkQueue.cpp:191
> +    EXPECT_TRUE(done >= start + std::chrono::milliseconds(500));

I am not sure how guaranteed accurate dispatchAfter is.

[Sam Weinig]

Comment on attachment 248203 [details]
Patch v3 (Update per weinig comments)

The tests look good, though the DispatchAfter worries me that it could be flakey (as all timing depended tests are).  I would land it without testing that the actual timing matches up.

[Brent Fulgham]

Committed r181249: <http://trac.webkit.org/changeset/181249>

[Brent Fulgham]

EFL and GTK ports should probably add this test as well.

[Martin Robinson]

The corresponding change for CMake is here: https://bugs.webkit.org/show_bug.cgi?id=142473

[David Kilzer (:ddkilzer)]

Attempt to fix WTF_WorkQueue.TwoQueues timeout test failure
<http://trac.webkit.org/changeset/181265>

Easiest to see where it started failing here:
<https://build.webkit.org/builders/Apple%20Mavericks%20Debug%20WK2%20(Tests)?numbuilds=50>

[Brent Fulgham]

(In reply to comment #11)
> Attempt to fix WTF_WorkQueue.TwoQueues timeout test failure
> <http://trac.webkit.org/changeset/181265>
> 
> Easiest to see where it started failing here:
> <https://build.webkit.org/builders/
> Apple%20Mavericks%20Debug%20WK2%20(Tests)?numbuilds=50>

Looks like that worked. Thank you for fixing that!