[webkit-reviews] review denied: [Bug 34449] [Gtk] Evaluate and create tests for all the AtkRole's implemented by WebKitGtk : [Attachment 47899] add tests for the form-roles - THIS IS PART 2

[bugzilla-daemon at webkit.org]

Xan Lopez <xan.lopez at gmail.com> has denied Joanmarie Diggs
<joanmarie.diggs at gmail.com>'s request for review:
Bug 34449: [Gtk] Evaluate and create tests for all the AtkRole's implemented by
WebKitGtk
https://bugs.webkit.org/show_bug.cgi?id=34449

Attachment 47899: add tests for the form-roles - THIS IS PART 2
https://bugs.webkit.org/attachment.cgi?id=47899&action=review

------- Additional Comments from Xan Lopez <xan.lopez at gmail.com>
First of all, thanks a lot for working of this. No doubt we should also work in
improving our DRT coverage, but I think there's no reason at all to not have
our own ATK unit tests, especially if someone has gone through the effort
already of writing them.

I just have a couple of general comments that I believe apply to all your
patches:

- You should use g_test_add, which allows you to pass a couple of functions to
setup/finalize things. This should allow you to reuse a bunch of repeated code
you have all over to place. As an example you can check how it's done in
testloading.c

- Do not use a timeout to bail out of the test. You can add queue an idle, run
the mainloop, and in the idle callback do your stuff and then quit. Otherwise
the 100ms keeps adding up and the tests would take a lot longer to run than
needed.