Opened 8 years ago

Closed 8 years ago

Last modified 5 years ago

#14929 closed defect (fixed)

DOH Robot does not work with async loader

Reported by: Colin Snover Owned by: bill
Priority: undecided Milestone: 1.7.6
Component: TestFramework Version: 1.7.2
Keywords: Cc:
Blocked By: Blocking:

Description

  1. Attempting to use DOH Robot with async loader causes it to clobber the entire page because dojo/robotx and doh/runner use document.write.
  1. Attempting to write and use tests in a parent document that try to access widgets of the child document fail because things like dijit/registry are operating on the parent document and there is a mountain of code that relies heavily on the defined pattern of global object usage to try to fiddle with context to get these sorts of things to work “correctly”.

I am not sure this is actually fixable but I am logging a defect just in case since I spent a bunch of time tracking this stuff down.

Change History (27)

comment:1 Changed 8 years ago by Colin Snover

Status: newopen

For #2, it’s possible to create asynchronous tests and use an inner require along with a plugin like this to load the modules from the correct context (assuming the page being tested uses async mode):

define([ "dojo/_base/kernel" ], function (dojo) {
	return {
		load: function (id, require, callback) {
			dojo.global.require([ require.toAbsMid(id) ], callback);
		}
	};
});

and then in the test itself:

runTest: function () {
  var d = new doh.Deferred();

  require([ "./fromContext!dijit/registry" ], function (registry) {
    // do stuff with registry
    d.getTestCallback(function () { … });
  });

  return d;
}

comment:2 Changed 8 years ago by bill

Blocking: 14279 added

comment:3 Changed 8 years ago by bill

I tried using dojo.global.require() directly and that works nicely. The plugin idea looks even better.

Last edited 8 years ago by bill (previous) (diff)

comment:4 Changed 8 years ago by bill

In [28089]:

Some fixes for robot to be able to run against AMD (baseless) iframe. Refs #14929 1strict.

comment:5 Changed 8 years ago by bill

In [28271]:

For some reason [28224] broke how "dijit" in robot test files points to the dijit in the iframe, thus breaking dijit/tests/tree/robot/Tree_selector.html (and also intermittently the Tree_dnd.html test) on IE8.

Referencing dojo.global.require("dijit/registry") instead of dijit. Eventually all these files should be converted to full AMD.

Refs #14929, #14986 !strict.

comment:6 Changed 8 years ago by bill

In [28333]:

Make util/doh/robot.js factory return the doh.robot object. Convert references of doh.robot to plain robot, since they are separate modules and will probably be completely separated in the future.

Make Robot.html call require("doh/robot") on the parent frame, rather than assuming the parent frame has a doh.robot global variable.

util/doh/robot.js still needs to be upgraded to not use document.write(), and to stop referencing the dojo global object. It's trying to walk the fence, branching it's behavior based on whether or not dojo is loaded, but that doesn't work well with AMD.

Refs #14929 !strict.

comment:7 Changed 8 years ago by bill

Milestone: tbd1.8
Owner: set to bill
Status: openassigned

comment:8 Changed 8 years ago by bill

Resolution: fixed
Status: assignedclosed

In [28336]:

Convert robot*.js files to baseless AMD, and remove document.write() usage.

To get util/doh/robot.js to play nice with AMD (no document.write(), and no reference to dojo global), I introduced a dependency to dojo which theoretically wasn't needed before. Perhaps it can be removed in the future. If isolation is desired, the dojo-dependent code should be moved to dojo/robot.js, not put if if(){...} blocks.


Also removed the document.write() usage in dojo/robotx.js to create a console div. On IE6/IE7, we are already getting a firebug-lite console, so just using that. (This might be dependent on having isDebug:true in the data-dojo-config of the test file though.)


Theoretically fixes #14929 !strict.

comment:9 Changed 8 years ago by Colin Snover

Do we have updated docs on this? Is there anything special that needs to happen when a user tries to use DOH Robot with the async loader?

comment:10 Changed 8 years ago by bill

dohRobot.rst should definitely be updated to AMD format. The data in the iframe file (loaded by initRobot()) should be accessed via require(), ex:

var registry = kernel.global.require("dijit/registry");

(That's assuming that registry is already loaded in the iframe.)

The old way of calling dijit.byId() or dojo.global.dijit.byId() will probably still work, but the above syntax is future-proof.

I thought I made a new variable for accessing the iframe's global, like robot.iframe, but I guess I didn't check that change in. robot.iframe.require() seems clearer than kernel.global.require() to me.

Note also that I didn't implement the plugin you suggested in comment:1. Maybe that would be an improvement although it doesn't seem to get us much beyond the kernel.global.require() syntax. (In your plugin suggestion I'm not sure why we need toAbsMid()?)

Last edited 8 years ago by bill (previous) (diff)

comment:11 Changed 8 years ago by Colin Snover

Keywords: needsdocs added

Adding keyword to get docs updated.

comment:12 Changed 8 years ago by bill

In [28448]:

fix path to doh/main, refs #14929 !strict

comment:13 Changed 8 years ago by bill

In [28495]:

Use "dojo/main" rather than "dojo" to pull in the dojo rollup module. Ideally though this file should be modified to use granular dependencies.

Refs #14929 !strict.

comment:14 Changed 8 years ago by bill

In [28497]:

Use "dojo/main" rather than "dojo" to pull in the dojo rollup module. Ideally though this file should be modified to use granular dependencies.

Refs #14929 !strict.

comment:15 Changed 8 years ago by bill

In [28498]:

Woops, for some reason this module needs to explicitly list its module id. Perhaps because of the mapping from "doh/runner" to util/doh/runner?

Refs #14929 !strict.

comment:16 Changed 8 years ago by bill

In [28569]:

make window and document from iframe (loaded by initRobot()) available from robot module export, refs #14929 !strict.

comment:18 Changed 8 years ago by bill

In [28578]:

convert DOH's unit test file to AMD, refs #14929 !strict.

comment:19 Changed 8 years ago by bill

In [28583]:

Convert dijit/tests/robot files to AMD, refs #14929 !strict

comment:20 Changed 7 years ago by Kitson Kelly

In [29095]:

Fix CheckedMultiSelect? to set HTML form value and refactor test case for AMD, fixes #10544 refs #14929 !strict

comment:21 Changed 7 years ago by bill

Blocking: 14279 removed

(In #14279) Fixed #14929 a while ago.

comment:12 Changed 7 years ago by bill

In [29332]:

Convert dijit/tests/layout/robot files to AMD, refs #14929 !strict

comment:13 Changed 7 years ago by bill

In [29360]:

can't create global variable called "focus" on IE, refs #14929 !strict

comment:14 Changed 7 years ago by bill

In [31411]:

Remove dojo/ready dependency from top of file. It's only used from waitForLoad() and it's loaded locally there. Refs #14929.

comment:15 Changed 5 years ago by Bill Keese <bill@…>

In f3ff8db4e41ee59dea2d25bc3539ee6e867ea98f/util:

Error: Processor CommitTicketReference failed
Unsupported version control system "git": Can't find an appropriate component, maybe the corresponding plugin was not enabled? 

comment:16 Changed 5 years ago by Bill Keese <bill@…>

In 1995dc8a70f1aee19a215bdeef9156eed9ae0ed5/util:

Error: Processor CommitTicketReference failed
Unsupported version control system "git": Can't find an appropriate component, maybe the corresponding plugin was not enabled? 

comment:17 Changed 5 years ago by bill

Milestone: 1.81.7.6
Note: See TracTickets for help on using tickets.