[webkit-reviews] review denied: [Bug 68280] [Qt] QDeskWebView missing loadProgress tests : [Attachment 107853] updated patch with reviewer comments

[bugzilla-daemon at webkit.org]

Andreas Kling <kling at webkit.org> has denied Gopal Raghavan
<gopal.1.raghavan at nokia.com>'s request for review:
Bug 68280: [Qt] QDeskWebView missing loadProgress tests
https://bugs.webkit.org/show_bug.cgi?id=68280

Attachment 107853: updated patch with reviewer comments
https://bugs.webkit.org/attachment.cgi?id=107853&action=review

------- Additional Comments from Andreas Kling <kling at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=107853&action=review


>
Source/WebKit2/UIProcess/API/qt/tests/qmltests/DesktopWebView/tst_loadProgress.
qml:24
> +	   function test_loadProgress() {
> +	       compare(spy.count, 0)
> +	       var testUrl = Qt.resolvedUrl("../common/test1.html")
> +	       webView.load(testUrl)
> +	       compare(webView.loadProgress, 0)
> +	       spy.wait()
> +	       compare(webView.loadProgress, 100)
> +	   }

A couple of things:

- The 'testUrl' variable is not necessary, just
webView.load(Qt.resolvedUrl(...))
- We should check the value of loadProgress before calling load() as well.
- This test only exercises the READ function of the loadProgress property. Why
not also test that we get notified when it changes?