Opened 9 years ago

Closed 9 years ago

Last modified 9 years ago

#11969 closed defect (fixed)

BorderContainer_complex.html test problems

Reported by: bill Owned by: Katie Vance
Priority: high Milestone: 1.6
Component: Dijit Version: 1.5
Keywords: Cc:
Blocked By: Blocking:

Description

I'm seeing a number of problems with this test:

  1. When run on FF3.6/win I get an error message about dojo.global.dijit._curFocus.id is not defined.
  1. Test should not be using console.error() to report errors, use the DOH assertions. I see that there is a doh.t(passed) at the very end of each test but why have that "passed" variable at all? doh.t(), doh.f(), and doh.is() are standard.
  1. need comments for all assertions, otherwise if there's a failure for example in this set of tests we don't know which one failed:
doh.t(dijit.byId("tab2").selected);
doh.f(dijit.byId("tab1").selected);
doh.f(dijit.byId("tab3").selected);
doh.f(dijit.byId("tab4").selected);
doh.f(dijit.byId("tab5").selected);
doh.f(dijit.byId("tab6").selected);

All those doh.t() and doh.f() calls should have a second argument like "tab2", "tab1", etc.

  1. dojo coding style: need brackets for all if(), else, while(), etc. There were also a lot of extra tabs inside of every doh.robot.sequence(), you only need one tab to indent.

Attachments (1)

BorderContainer_complex.patch (17.8 KB) - added by Katie Vance 9 years ago.

Download all attachments as: .zip

Change History (5)

Changed 9 years ago by Katie Vance

comment:1 Changed 9 years ago by Katie Vance

You are right, this test was one of the first that I automated and it did not follow the standards. Attached is how the test should have been written. I did not remove dojo.global.dijit._curFocus.id because it is working for me. Many other dijit robot tests use the same method for determining what has focus. Let me know if you still have problems with that.

comment:2 Changed 9 years ago by bill

Thanks, that looks much better. Using dojo.global.dijit._curFocus is fine, it's just that I was getting a test failure for some reason. Maybe robot flakiness ignoring the first click event, not sure, it seems to be working now.

comment:3 Changed 9 years ago by bill

Resolution: fixed
Status: newclosed

(In [23227]) Test cleanup from Katie, fixes #11969, thanks!

comment:4 Changed 9 years ago by bill

(In [23863]) Avoid spurious timing failures in AccordionContainer tests plus other cleanup, refs #11969.

Note: See TracTickets for help on using tickets.