Bug 46814

Summary: [Qt] Fix tst_QWebPage::geolocationRequestJS()
Product: WebKit Reporter: Benjamin Poulain <benjamin>
Component: Tools / TestsAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, benjamin, commit-queue, eric, kling, laszlo.gombos, maheshk, webkit.review.bot
Priority: P2 Keywords: Qt, QtTriaged
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on:    
Bug Blocks: 46810    
Attachments:
Description Flags
Disable the test
none
Patch
none
Ooops
none
re-enable the test
none
Patch
none
Patch none

Description Benjamin Poulain 2010-09-29 09:23:34 PDT
The test tst_QWebPage::geolocationRequestJS() always fail with a default build because ENABLE(GEOLOCATION) is false.

When the API is fixed, the test should be fixed accordingly to succeed for standard build.
Comment 1 Benjamin Poulain 2010-09-29 09:57:58 PDT
Created attachment 69206 [details]
Disable the test
Comment 2 Andreas Kling 2010-09-29 10:09:01 PDT
Comment on attachment 69206 [details]
Disable the test

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

> WebKit/qt/tests/qwebpage/tst_qwebpage.cpp:256
> +#if 0

Please don't disable this code. We want whoever alters the permissions API to update the test as well.
Comment 3 Mahesh Kulkarni 2010-09-29 10:41:37 PDT
one way to know if geolocation is enabled is by checking return value of  
frame->evaluateJavascript("navigator.geolocation == undefined"). Based on this we can ignore all assert checks.
Comment 4 Mahesh Kulkarni 2010-09-29 10:43:45 PDT
(In reply to comment #0)
> 
> When the API is fixed, the test should be fixed accordingly to succeed for standard build.

@Benjamin I did not get what you mean here? What has to be fixed in API?
Comment 5 Benjamin Poulain 2010-09-29 10:51:03 PDT
Created attachment 69219 [details]
Patch
Comment 6 Benjamin Poulain 2010-09-29 10:52:10 PDT
Created attachment 69220 [details]
Ooops
Comment 7 Benjamin Poulain 2010-09-29 10:53:35 PDT
(In reply to comment #4)
> > When the API is fixed, the test should be fixed accordingly to succeed for standard build.
> 
> @Benjamin I did not get what you mean here? What has to be fixed in API?

The API did not go through any API review, see: https://bugs.webkit.org/show_bug.cgi?id=46810

The API does not seem future proof at the moment. When the API will be better, this test should be re-enabled.
Comment 8 WebKit Commit Bot 2010-09-29 11:57:03 PDT
Comment on attachment 69220 [details]
Ooops

Rejecting patch 69220 from commit-queue.

Failed to run "['./WebKitTools/Scripts/webkit-patch', '--status-host=queues.webkit.org', 'build-and-test', '--no-clean', '--no-update', '--test', '--quiet', '--non-interactive']" exit_code: 2
Last 500 characters of output:
s successful.
Files=14, Tests=304,  1 wallclock secs ( 0.72 cusr +  0.16 csys =  0.88 CPU)
Running build-dumprendertree
Compiling Java tests
make: Nothing to be done for `default'.
Running tests from /Projects/CommitQueue/LayoutTests
Testing 21476 test cases.
plugins/plugin-initiate-popup-window.html -> failed

Exiting early after 1 failures. 17959 tests run.
321.64s total testing time

17958 test cases (99%) succeeded
1 test case (<1%) had incorrect layout
31 test cases (<1%) had stderr output

Full output: http://queues.webkit.org/results/4153011
Comment 9 Benjamin Poulain 2010-09-29 12:01:41 PDT
Comment on attachment 69220 [details]
Ooops

Trying cq+ again. This is just a QSKIP, it has not influence on other tests.
Comment 10 WebKit Commit Bot 2010-09-30 00:10:15 PDT
Comment on attachment 69220 [details]
Ooops

Clearing flags on attachment: 69220

Committed r68760: <http://trac.webkit.org/changeset/68760>
Comment 11 WebKit Commit Bot 2010-09-30 00:10:21 PDT
All reviewed patches have been landed.  Closing bug.
Comment 12 Benjamin Poulain 2010-09-30 00:45:41 PDT
Reopening since this bug is really about fixing the test. The patch disabling is temporary.
Comment 13 Laszlo Gombos 2010-12-15 20:14:48 PST
Created attachment 76732 [details]
re-enable the test

API review is finished; GEOLOCATION is enabled in the master bot.
Test will fail if GEOLOCATION is not enabled - I think this is the correct expected behavior from the test.
Comment 14 WebKit Commit Bot 2010-12-15 22:14:34 PST
Comment on attachment 76732 [details]
re-enable the test

Clearing flags on attachment: 76732

Committed r74173: <http://trac.webkit.org/changeset/74173>
Comment 15 WebKit Commit Bot 2010-12-15 22:14:41 PST
All reviewed patches have been landed.  Closing bug.
Comment 16 WebKit Review Bot 2010-12-16 15:38:04 PST
http://trac.webkit.org/changeset/74173 might have broken Leopard Intel Debug (Tests)
Comment 17 Benjamin Poulain 2011-01-10 14:56:31 PST
This test is failing. Did you take into account my comments?
"This test is disabled because it can only succeed if ENABLE(GEOLOCATION) is true."
Comment 18 Laszlo Gombos 2011-01-10 15:36:21 PST
(In reply to comment #17)
> This test is failing. Did you take into account my comments?

Failing where ? Seems to be passing on the bot - http://build.webkit.org/builders/Qt%20Linux%20Release/builds/26268/steps/API%20tests/logs/stdio
Comment 19 Benjamin Poulain 2011-01-10 15:42:12 PST
(In reply to comment #18)
> (In reply to comment #17)
> > This test is failing. Did you take into account my comments?
> 
> Failing where ? Seems to be passing on the bot - http://build.webkit.org/builders/Qt%20Linux%20Release/builds/26268/steps/API%20tests/logs/stdio

On my computer at least. :)
But my guess is it fails on any build done without mobility.
Comment 20 Benjamin Poulain 2011-01-10 16:05:47 PST
Created attachment 78466 [details]
Patch
Comment 21 Benjamin Poulain 2011-01-10 16:10:21 PST
Created attachment 78467 [details]
Patch
Comment 22 WebKit Commit Bot 2011-01-11 02:08:23 PST
Comment on attachment 78467 [details]
Patch

Clearing flags on attachment: 78467

Committed r75479: <http://trac.webkit.org/changeset/75479>
Comment 23 WebKit Commit Bot 2011-01-11 02:08:31 PST
All reviewed patches have been landed.  Closing bug.