Merge into trunk : fix-lazyImage-test-flakyness : Code : Unity 8

Status:	Superseded
Proposed branch:	lp:~cimi/unity8/fix-lazyImage-test-flakyness
Merge into:	lp:unity8
Diff against target:	71 lines (+23/-6) 2 files modified qml/Components/LazyImage.qml (+3/-0) tests/qmltests/Components/tst_LazyImage.qml (+20/-6)
To merge this branch:	bzr merge lp:~cimi/unity8/fix-lazyImage-test-flakyness
Related bugs:	Link a bug report

Reviewer	Review Type	Date Requested	Status
Unity8 CI Bot	continuous-integration		Approve on 2016-01-13
Michael Zanetti (community)		2015-11-13	Needs Information on 2015-11-27
PS Jenkins bot	continuous-integration		Needs Fixing on 2015-11-17
Review via email: mp+277459@code.launchpad.net

This proposal has been superseded by a proposal from 2016-01-28.

Commit Message

Should fix lazyimage test failures on fast machines

Description of the Change

* Are there any related MPs required for this MP to build/function as expected? Please list.
n
* Did you perform an exploratory manual test run of your code change and any related functionality?
y
* Did you make sure that your branch does not contain spurious tags?
y
* If you changed the packaging (debian), did you subscribe the ubuntu-unity team to this MP?
n/a
* If you changed the UI, has there been a design review?
n/a

Albert Astals Cid (aacid) wrote on 2015-11-16:

#

Not sure this is the correct fix, as far as i see it the problem is that the state of the LazyImage when changing goes

qml: STATE default
qml: STATE ready
qml: STATE loading
qml: STATE ready

The first ready seems to be wrong to me.

PS Jenkins bot (ps-jenkins) wrote on 2015-11-16:

#

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity8-ci/6708/rebuild

review: Needs Fixing (continuous-integration)

PS Jenkins bot (ps-jenkins) wrote on 2015-11-17:

#

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity8-ci/6719/rebuild

review: Needs Fixing (continuous-integration)

PS Jenkins bot (ps-jenkins) wrote on 2015-11-17:

#

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity8-ci/6723/rebuild

review: Needs Fixing (continuous-integration)

Michael Zanetti (mzanetti) wrote on 2015-11-27:

#

that signal seems to be just used for testing. If so, can't we use "findInvisibleChild()" to find the animation and use tryCompare() on the running property instead?

review: Needs Information

Unity8 CI Bot (unity8-ci-bot) wrote on 2016-01-13:

#

PASSED: Continuous integration, rev:2040
https://unity8-jenkins.ubuntu.com/job/lp-unity8-1-ci/31/
Executed test runs:

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-unity8-1-ci/31/rebuild

review: Approve (continuous-integration)

Michael Zanetti (mzanetti) wrote on 2016-01-28:

#

ok, had been looking at this and besides the fact that I cant repro the flakiness at all, it seems to me that the code actually properly does wait for the transition already.

If there's still some flakiness it's probably because of the render thread lagging a bit behind and I think a simple "waitForRendering(data.image)" should be enough to get this sorted.

 === modified file 'qml/Components/LazyImage.qml'
 --- qml/Components/LazyImage.qml	2015-11-04 14:57:13 +0000
 +++ qml/Components/LazyImage.qml	2015-11-13 16:01:53 +0000
@@ -40,6 +40,8 @@
      state: "default"
++    signal readyTransitionRunning(bool running)
++
      onSourceChanged: {
          if (state === "ready") {
              state = "default";
@@ -157,6 +159,7 @@
          Transition {
              to: "ready"
              objectName: "readyTransition"
++            onRunningChanged: root.readyTransitionRunning(running)
              SequentialAnimation {
                  PropertyAction { target: shape; property: "visible" }
                  ParallelAnimation {
 === modified file 'tests/qmltests/Components/tst_LazyImage.qml'
 --- tests/qmltests/Components/tst_LazyImage.qml	2015-07-15 15:07:19 +0000
 +++ tests/qmltests/Components/tst_LazyImage.qml	2015-11-13 16:01:53 +0000
@@ -112,6 +112,11 @@
+         }
+     }
++    SignalSpy {
++        id: signalSpy
++        signalName: "readyTransitionRunning"
++    }
++
      UT.UnityTestCase {
          name: "LazyImage"
          when: windowShown
@@ -144,8 +149,8 @@
                  {tag: "Height-bound Bad path", image: lazy3, func: controls3.badpath, transition: "genericTransition", width: units.gu(10), height: units.gu(12), imageWidth: units.gu(10), imageHeight: units.gu(12), initialWidth: units.gu(10), initialHeight: units.gu(12), placeholder: true, error: true},
                  {tag: "Fit Blank", image: lazy4, func: controls4.blank, width: units.gu(12), height: units.gu(12), imageWidth: units.gu(12), imageHeight: units.gu(12), initialWidth: units.gu(12), initialHeight: units.gu(12), placeholder: true},
                  {tag: "Fit Wide", image: lazy4, func: controls4.wide, transition: "readyTransition", width: units.gu(12), height: units.gu(12), imageWidth: units.gu(12), imageHeight: units.gu(6), initialWidth: units.gu(12), initialHeight: units.gu(12)},
--                {tag: "Fit Square", image: lazy4, func: controls4.square, transition: "readyTransition", width: units.gu(12), height: units.gu(12), imageWidth: units.gu(12), imageHeight: units.gu(12), initialWidth: units.gu(12), initialHeight: units.gu(12)},
--                {tag: "Fit Portrait", image: lazy4, func: controls4.portrait, transition: "readyTransition", width: units.gu(12), height: units.gu(12), imageWidth: units.gu(6), imageHeight: units.gu(12), initialWidth: units.gu(12), initialHeight: units.gu(12)},
++                {tag: "Fit Square", image: lazy4, func: controls4.square, transition: "readyTransition", transitionCount: 4, width: units.gu(12), height: units.gu(12), imageWidth: units.gu(12), imageHeight: units.gu(12), initialWidth: units.gu(12), initialHeight: units.gu(12)},
++                {tag: "Fit Portrait", image: lazy4, func: controls4.portrait, transition: "readyTransition", transitionCount: 4, width: units.gu(12), height: units.gu(12), imageWidth: units.gu(6), imageHeight: units.gu(12), initialWidth: units.gu(12), initialHeight: units.gu(12)},
                  {tag: "Fit Bad path", image: lazy4, func: controls4.badpath, transition: "genericTransition", width: units.gu(12), height: units.gu(12), imageWidth: units.gu(12), imageHeight: units.gu(12), initialWidth: units.gu(12), initialHeight: units.gu(12), placeholder: true, error: true},
+             ]
+         }
@@ -154,10 +159,19 @@
              data.func();
              if (data.transition) {
--                // wait for the transition to complete
--                var transition = findChildIn(data.image, "transitions", data.transition);
--                tryCompare(transition, "running", true);
--                tryCompare(transition, "running", false);
++                if (data.transition == "readyTransition") {
++                    signalSpy.target = data.image;
++                    signalSpy.clear();
++                    tryCompare(signalSpy, "count", data.transitionCount ? data.transitionCount : 2);
++                    for (var i = 0; i < signalSpy.count; i++) {
++                        compare(signalSpy.signalArguments[i][0], i % 2 == 0 ? true : false);
++                    }
++                } else {
++                    // wait for the transition to complete
++                    var transition = findChildIn(data.image, "transitions", data.transition);
++                    tryCompare(transition, "running", true);
++                    tryCompare(transition, "running", false);
++                }
+             }
              // check the dimensions

Unity 8

Merge lp:~cimi/unity8/fix-lazyImage-test-flakyness into lp:unity8

Commit Message

Description of the Change

Unmerged revisions

Preview Diff

Subscribers