Code review comment for lp:~gerboland/unity/refactor-wm-and-test

Revision history for this message
Michael Zanetti (mzanetti) wrote :

765 + //hides: [launcher, panel.indicators]

This will never be used in here... can go away

1454 + function rightEdgeSwipe(distance) {
1455 + if (distance == undefined) distance = units.gu(stageManagerUnderTest.width)
1456 +
1457 + var x = stageManagerUnderTest.width - (stageManagerUnderTest.edgeHandleSize / 2)
1458 + var y = stageManagerUnderTest.height / 2
1459 + mouseFlick(stageManagerUnderTest, x, y,
1460 + x - distance, y)
1461 + }
1462 +
1463 + function sideStageHandleRightSwipe(distance) {
1464 + if (distance == undefined) distance = units.gu(sideStage.width)
1465 +
1466 + var x = sideStage.x - (sideStage.rightEdgeDraggingAreaWidth / 2)
1467 + var y = sideStage.height / 2
1468 + mouseFlick(stageManagerUnderTest, x, y,
1469 + x + distance, y)
1470 + }

whats the difference between those two? wouldn't the first one be enough?

1474 + compare(stageManagerUnderTest.shown, false)

would it make sense to add a message to compare()s for easier debugging in case the test fails? I think for some of your compare()s it does make sense, not necessarily for all tho.

836 + skip("FIXME: application not unfocused when dismissed with left edge swipe")
837 + tryCompare(applicationManager, "mainStageFocusedApplication", null)

This merge seems to be right place to fix this, unless it causes lots of other changes too. Also, when this is fixed, the tryCompare() seems like a good addition to checkStageManagerOffScreen().

1026 + width: units.gu(160)

This seems a bit much and not really needed. Can we make this a bit smaller so the tests can run and seen on smaller displays too?

When running the test files in qmlscene (for manual testing/debuggin) they segfault:
# qmlscene ../tests/qmltests/Stages/tst_StageManager-tablet.qml -I ./tests/mocks/ -I tests/utils/modules/
ASSERT failure in QList<T>::operator[]: "index out of range", file /usr/include/qt5/QtCore/qlist.h, line 462
Aborted (core dumped)

We try to create testcases that are able to run standalone for manual testing/debugging which I think is quite useful once a test fails.

Overall really nice test set. Well done!

review: Needs Fixing

« Back to merge proposal