Bug 67155

Summary: [Qt] QDesktopWebView url property test missing
Product: WebKit Reporter: Gopal Raghavan <gopal.1.raghavan>
Component: WebKit2Assignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: menard, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
url property test for DesktopWebView
noam: review-, noam: commit-queue-
updated desktopwebview patch none

Gopal Raghavan
Reported 2011-08-29 14:31:48 PDT
Need test for url property in QDesktopWebView
Attachments
url property test for DesktopWebView (1.38 KB, patch)
2011-08-29 14:37 PDT, Gopal Raghavan
noam: review-
noam: commit-queue-
updated desktopwebview patch (1.47 KB, patch)
2011-08-29 15:03 PDT, Gopal Raghavan
no flags
Gopal Raghavan
Comment 1 2011-08-29 14:37:39 PDT
Created attachment 105523 [details] url property test for DesktopWebView Here is the test results: ********* Start testing of tst_CommonViewTests ********* Config: Using QTest library 5.0.0, Qt 5.0.0 PASS : tst_CommonViewTests::initTestCase() PASS : tst_CommonViewTests::baseUrl() PASS : tst_CommonViewTests::loadEmptyUrl() QWARN : tst_CommonViewTests::loadEmptyPageViewVisible() QSocketNotifier: socket notifiers cannot be enabled from another thread QWARN : tst_CommonViewTests::loadEmptyPageViewVisible() QSocketNotifier: socket notifiers cannot be enabled from another thread QWARN : tst_CommonViewTests::loadEmptyPageViewVisible() QObject::startTimer: timers cannot be started from another thread PASS : tst_CommonViewTests::loadEmptyPageViewVisible() QWARN : tst_CommonViewTests::loadEmptyPageViewHidden() QSocketNotifier: socket notifiers cannot be enabled from another thread PASS : tst_CommonViewTests::loadEmptyPageViewHidden() QWARN : tst_CommonViewTests::loadNonexistentFileUrl() QSocketNotifier: socket notifiers cannot be enabled from another thread PASS : tst_CommonViewTests::loadNonexistentFileUrl() QWARN : tst_CommonViewTests::backAndForward() QSocketNotifier: socket notifiers cannot be enabled from another thread PASS : tst_CommonViewTests::backAndForward() QWARN : tst_CommonViewTests::reload() QSocketNotifier: socket notifiers cannot be enabled from another thread PASS : tst_CommonViewTests::reload() PASS : tst_CommonViewTests::stop() PASS : tst_CommonViewTests::loadProgress() QWARN : tst_CommonViewTests::show() QSocketNotifier: socket notifiers cannot be enabled from another thread QWARN : tst_CommonViewTests::show() QObject::startTimer: timers cannot be started from another thread PASS : tst_CommonViewTests::show() PASS : tst_CommonViewTests::cleanupTestCase() Totals: 12 passed, 0 failed, 0 skipped ********* Finished testing of tst_CommonViewTests ********* ********* Start testing of tst_QTouchWebView ********* Config: Using QTest library 5.0.0, Qt 5.0.0 PASS : tst_QTouchWebView::initTestCase() PASS : tst_QTouchWebView::accessPage() PASS : tst_QTouchWebView::navigationActionsStatusAtStartup() PASS : tst_QTouchWebView::cleanupTestCase() Totals: 4 passed, 0 failed, 0 skipped ********* Finished testing of tst_QTouchWebView ********* ********* Start testing of tst_QDesktopWebView ********* Config: Using QTest library 5.0.0, Qt 5.0.0 PASS : tst_QDesktopWebView::initTestCase() PASS : tst_QDesktopWebView::navigationActionsStatusAtStartup() QWARN : tst_QDesktopWebView::stopActionEnabledAfterLoadStarted() QSocketNotifier: socket notifiers cannot be enabled from another thread PASS : tst_QDesktopWebView::stopActionEnabledAfterLoadStarted() PASS : tst_QDesktopWebView::cleanupTestCase() Totals: 4 passed, 0 failed, 0 skipped ********* Finished testing of tst_QDesktopWebView ********* ********* Start testing of qmltests ********* Config: Using QTest library 5.0.0, Qt 5.0.0 PASS : DesktopWebViewNavigationPolicyForUrl::initTestCase() QWARN : DesktopWebViewNavigationPolicyForUrl::test_ignorePolicy() QObject::startTimer: timers cannot be started from another thread PASS : DesktopWebViewNavigationPolicyForUrl::test_ignorePolicy() PASS : DesktopWebViewNavigationPolicyForUrl::test_usePolicy() PASS : DesktopWebViewNavigationPolicyForUrl::cleanupTestCase() PASS : DesktopWebViewProperties::initTestCase() PASS : DesktopWebViewProperties::test_title() PASS : DesktopWebViewProperties::test_url() PASS : DesktopWebViewProperties::cleanupTestCase() PASS : TouchWebViewProperties::initTestCase() PASS : TouchWebViewProperties::test_title() PASS : TouchWebViewProperties::cleanupTestCase() Totals: 11 passed, 0 failed, 0 skipped ********* Finished testing of qmltests ********* ********************************************************************** ** TOTALS: 31 passed, 0 failed, 0 skipped ** **********************************************************************
Noam Rosenthal
Comment 2 2011-08-29 14:52:18 PDT
Comment on attachment 105523 [details] url property test for DesktopWebView View in context: https://bugs.webkit.org/attachment.cgi?id=105523&action=review > Source/WebKit2/ChangeLog:8 > + Added qml test case to check url property Period at end of sentence please. > Source/WebKit2/UIProcess/API/qt/tests/qmltests/DesktopWebView/tst_properties.qml:30 > + var tstUrl = Qt.resolvedUrl("../common/test1.html") > + webView.load(tstUrl) > + spy.wait() > + compare(webView.url, tstUrl) > + } This doesn't test anything. Please test that the page actually loaded, e.g. by testing that the title has changed (like the test above it). Also, use testUrl instead of tstUrl. Abbvs don't smplfy reading the code :)
Gopal Raghavan
Comment 3 2011-08-29 14:56:56 PDT
webView.url is a read only property. All it does is to check and see if the url is set properly. This is not a title test. That is already take care of in previous test case. I can take care of tstUrl. Thanks
Gopal Raghavan
Comment 4 2011-08-29 15:03:40 PDT
Created attachment 105526 [details] updated desktopwebview patch Taken care of reviewer comments.
Alexis Menard (darktears)
Comment 5 2011-08-29 15:32:05 PDT
(In reply to comment #3) > webView.url is a read only property. All it does is to check and see if the url is set properly. > This is not a title test. That is already take care of in previous test case. > > I can take care of tstUrl. > Thanks What Noam said is that load is async therefore in theory testing the property right away is not valid.
Gopal Raghavan
Comment 6 2011-08-29 15:34:40 PDT
That's why we wait on spy for signal "loadSucceeded".
Alexis Menard (darktears)
Comment 7 2011-08-29 15:40:20 PDT
Comment on attachment 105526 [details] updated desktopwebview patch Ah yes perfect then.
WebKit Review Bot
Comment 8 2011-08-29 16:19:56 PDT
Comment on attachment 105526 [details] updated desktopwebview patch Clearing flags on attachment: 105526 Committed r94027: <http://trac.webkit.org/changeset/94027>
WebKit Review Bot
Comment 9 2011-08-29 16:20:01 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.