dojox.mobile: updates to DOH tests

[Adrian Vasiliu]

In trunk (Dojo 1.8), dojox.mobile's DOH tests need a number of updates to solve failures in various browsers. The attached patch solves failures due to sizes of various elements which are slightly different than in the past when running the current trunk in Chrome 18, Firefox 11, and IE9. Additional fixes are needed for running on mobile devices. I will reattach a new patch merging all changes (so the commit can wait for the final patch).

[Adrian Vasiliu]

I'm wondering whether the DOH tests should continue to check precise size/position numbers. Given that many of the actual results are different depending on the browser, and seem to evolve in time, this makes the tests hard to maintain. Maybe at least in some cases the test could avoid checking precise numbers and check instead some higher-level requirements. Anyway, this is just food for thoughts.

[ykami]

Adrian, thank you for the patch. But currently we do not see any DOH failures even on Chrome 18. Conversely, the DOH broke when we applied your patch. We've been testing on English Windows and Japanese Windows and getting the same results. Not sure what makes the difference, but perhaps we better relax some of the tests that check pixels to make them more robust.

[Adrian Vasiliu]

Well, on my side I did get failures when running against dojo-trunk (before my patch) in Chrome 18 on an English Windows 7 (thinkpad). Okay, I will relax some of the tests. You say the DOH broke with the patch applied, was there any error message? On my side it wasn't broken at least in Chrome 18, FF11 and IE9, using the dojo-trunk updated at that time. I will come back to the DOH tests when I'll be free of other higher priority work.

[Adrian Vasiliu]

Running the unpatched DOH tests (dojo-trunk version as of today), the results are as follows:

Chrome 19: 1 errors, 12 failures

FF 13: 0 errors, 1 failures

IE9: 0 errors, 6 failures

I've attached a new version of my patch for the DOH tests which now produces no error/failure on these 3 browsers.

[bill]

FYI, I tried on Chrome (windows V20) without the patch and I got a few failures:

EdgeToEdge category:

    Error: test timeout in ../../dojox/mobile/tests/doh/EdgeToEdgeCategory.html::dojox.mobile.test.doh.EdgeToEdgeList::EdgeToEdgeList Verification2
     ERROR IN:
 		 function (){
				var d = new doh.Deferred();
				var demoWidget = dijit.byId("dojox_mobile_EdgeToEdgeList_0");
				demoWidget.set({transition :"flip"});
				doh.assertEqual("flip", demoWidget.get("transition"));
				demoWidget.set({transition :"fade"});
				doh.assertEqual("fade", demoWidget.get("transition"));

				fireOnMouseDown("item3");
				fireOnMouseUp("item3");
				var view = dijit.byId("foo");
				dojo.connect(view, "onAfterTransitionOut", this, d.getTestCallback(function(){
					var demoWidget = dijit.byId("dojox_mobile_EdgeToEdgeCategory_0");
					doh.assertEqual('mblEdgeToEdgeCategory', demoWidget.domNode.className);
					doh.assertEqual('Applications', demoWidget.domNode.innerHTML);

					demoWidget = dijit.byId("dojox_mobile_EdgeToEdgeList_1");
					doh.assertEqual('mblEdgeToEdgeList', demoWidget.domNode.className);

					verifyListItem("dojox_mobile_ListItem_0", 'Video', 'Off', "", false, false, false);
					verifyListItem("dojox_mobile_ListItem_1", 'Maps', 'VPN', "", true, false, false);
					verifyListItem("dojox_mobile_ListItem_2", 'Phone Number', 'Off', "", false, false, false);
				}));
				return d;
			}
 FAILED test: ../../dojox/mobile/tests/doh/EdgeToEdgeCategory.html::dojox.mobile.test.doh.EdgeToEdgeList::EdgeToEdgeList Verification2 59332 ms

ContentPane:

GROUP "dojox.mobile.test.doh.ContentPane" has 2 tests to run
     _AssertFailure: assertNotEqual() failed: not expected |null| but got |undefined| with hint: 
 		ContentPane: Did not instantiate first child. id=view1-pane4
     ERROR IN:
 		 function (){
						var widget = registry.byId("view1-pane14");
						var d = new runner.Deferred();
						var handle1 = connect.connect(widget, "loadHandler", this, d.getTestCallback(function(){
							connect.disconnect(handle1);
							_assertCorrectContentPanes(["view1-pane1","view1-pane2","view1-pane3","view1-pane4","view1-pane5"]);
							_assertCorrectContentPanes(["view1-pane6","view1-pane7","view1-pane8","view1-pane9","view1-pane10"]);
							_assertCorrectContentPanes(["view1-pane11","view1-pane12","view1-pane13","view1-pane14","view1-pane15"]);
						}));
						return d;
					}
 FAILED test: ../../dojox/mobile/tests/doh/ContentPaneTests.html::dojox.mobile.test.doh.ContentPane::ContentPane Verification1 70 ms
     Error: test timeout in ../../dojox/mobile/tests/doh/ContentPaneTests.html::dojox.mobile.test.doh.ContentPane::ContentPane Verification2
     ERROR IN:
 		 function (){
						var widget = registry.byId("view2-pane14");
						var d = new runner.Deferred();
						var handle2 = connect.connect(widget, "loadHandler", this, d.getTestCallback(function(){
							connect.disconnect(handle2);
							_assertCorrectContentPanes(["view2-pane1","view2-pane2","view2-pane3","view2-pane4","view2-pane5"]);
							_assertCorrectContentPanes(["view2-pane6","view2-pane7","view2-pane8","view2-pane9","view2-pane10"]);
							_assertCorrectContentPanes(["view2-pane11","view2-pane12","view2-pane13","view2-pane14","view2-pane15"]);
						}));
						_showView2();
						return d;
					}
 FAILED test: ../../dojox/mobile/tests/doh/ContentPaneTests.html::dojox.mobile.test.doh.ContentPane::ContentPane Verification2 4010 ms

However, when I ran the tests again they all passed.

[cjolif]

I just got the ContentPane error and indeed it fires only the first time (probably because the referenced file is already in cache the second time so due to timing conditions the error does not appear anymore). I don't have any other errors. So before committing this I would like to better understand why the other changes are needed. On the ContentPane side I confirm the new test is fixing the issue however I see in the patch that we don't fully understand what's going on ("CHECKME"). Maybe we should before modifying the test?

[Adrian Vasiliu]

Yes, an attempt to understand it is ongoing. For ContentPane, the story is the following: the test creates and fills the ContentPane programmatically (for the failing case, with a "href"), then it used to check on the "loadHandler" event that the pane has a child. But this appears to be too early. I've tried to listen to "onLoad" instead of "loadHandler", but the behavior is the same. What did work is doing the checks in a setTimeout once getting the "loadHandler". So what needs to be clarified is the following: is the DOM supposed to be already filled when "loadHandler" (or "onLoad") are fired?

Will also try to understand what's going on with the other tests that failed on my side (without the patch) while didn't on yours (in particular on Christophe's).

[Adrian Vasiliu]

Besides the partially random behavior of the ContentPane test before the patch, the results were different on Chrome because... of browser's zoom (thanks edurocher for the good intuition!). I've attached a new version of the patch. (Produces zero error/failure in Chrome 19, FF 13, and IE9 - tested after insuring browser zoom=1:1.)

[ykami]

I confirmed the ContentPane failures. I also confirmed they did not happen in my one month old snapshot of the trunk. Did you figure out what made the difference?

[Adrian Vasiliu]

What I can say for now is that the old version of the test passes okay with the current Dojo trunk if I revert dojo/_base/xhr.js at revision 28611 (5 weeks ago). So the recent massive changes in xhr seems to be involved in this change of behavior of our ContentPane. Next step for me (right now) is to try to isolate and reproduce the issue in a minimal test case such that I can enter a defect (and maybe patch) for the new xhr. The issue being that there should still be a way to be notified when xhr's job is fully completed.

[Eric Durocher]

I investigated a little too, I think what causes the test to fail is that the new xhr implementation seems to call the load callbacks in a different order. The test relied on the fact that the last loaded content was for the view1-pane14 pane, and it only listens to the loadHandler of that pane to check everything. With the new xhr implementation, view1-pane14 is sometimes loaded before view1-pane4, and so at the time of the checks view1-pane4 is not yet loaded, thus the error.

I verified this by adding a console log in dojox/mobile/ContentPane.loadHandler. You can see that with the old XHR, view1-pane14 is always loaded last, whereas with the new implementation the order is different (and can vary from one run to another).

I don't think we can consider this a real regression of dojo/_base/xhr, but maybe I am wrong and there is actually a specified order? If not, the ContentPane test should not rely on a particular order (maybe it should wait for all loadHandlers to be called?).

[cjolif]

Thanke Eric for your analysis. I agree this does not sound like a regression but more like a test relying on a non specified behavior. So the test should indeed be re-written to not rely on the order on which the pane are actually loaded.

[ykami]

Thanks Adrian and edurocher for your investigation. Then I think the setTimeout fix is ok. I just didn't want to hide the root cause without knowing what's going on.

For other fixes, unfortunately, your patch didn't fix the ExpandingTextArea/TextArea errors on my IE8. I got errors as below.

 GROUP "dojox.mobile.test.doh.ExpandingTextArea" has 2 tests to run     _AssertFailure: assertEqual() failed: 	expected
		25px
	but got
		26px

GROUP "dojox.mobile.test.doh.TextArea" has 2 tests to run     _AssertFailure: assertEqual() failed: 	expected
		55
	but got
		58

Should we skip these tests when on IE? I'm afraid these two tests are too fragile.

[Adrian Vasiliu]

edurocher: thanks a lot for you lights and sorry for having been so blind...

ykami: a) Concerning the IE issues: my plan was to give the priority to IE9 over older IE. And the patch includes an added README with this note: "The dimensional checks performed by a few tests are calibrated for execution in IE9, hence may fail in older IE versions. This does not mean the tested widget is broken in the older IE; it only means such tests do not hold for older IE versions." (also included a note about the browser zoom which needs to be at 1:1). Now, of course we can also decide to simply skip these tests when on IE. I let erudorcher / cjolif decide.

b) Concerning the ContentPane - now that we know the reason of the failure, maybe we could replace the setTimeout trick by counting the number of calls of loadHandler (separately for each view) and doing the checks after the last call. However this would require to take care when adding more test cases. Not sure it's worth, so maybe the setTimeout it's good enough.

[ykami]

ok, then, let's skip those two tests when on IE8 or older. As for the ContentPane, I think setTimeout is good enough considering the purpose of the tests.

[Eric Durocher]

Yes I agree, setTimeout is good enough.

[Adrian Vasiliu]

DOH tests: updates for IE (updated dimensional values for IE9 and skip on older IE); avoid checking ContentPane? too early (with new xhr the loading order is undetermined); added test cases for Spinner with specified initial value; added a README with some explanations; now producing zero error/failure on Chrome 19, FF13, IE8 and IE9. - Adrian Vasiliu, IBM, CCLA

[cjolif]

fixed in [29129]