Opened 7 years ago

Closed 7 years ago

Last modified 6 years ago

#16825 closed defect (patchwelcome)

doh and dojoConfig race condition

Reported by: jandockx Owned by:
Priority: undecided Milestone: tbd
Component: TestFramework Version: 1.8.3
Keywords: Cc:
Blocked By: Blocking:

Description

Our project, and I assume many others, need some kind of config, for different enviroments etc. The only sensible way to do this seems to use dojoConfig and has. In both cases, we need to be able to set some things before dojo kicks off (really, changing config on the fly almost guarantees race conditions).

Running the tests in doh often requires running the main code with in a test enviroment (e.g., using mock helpers). A test run is an "environment". Thus, we need to be able to configure-for-test. The only sensible way to do this, again, seems to use dojoConfig and has.

There is, as far as I can see, only 1 hook in doh to do this: using the boot parameter provided in util/doh/_parseURLargs.js. We use, e.g., the following URL to start the tests:

<meta http-equiv="refresh" content="0;url=../../util/doh/runner.html?breakOnError=false&sandbox=true&async=true&boot=../../MYPROJECT/test/_dohConfig.js,../../dojo/dojo.js&testModule=MYPROJECT/test/module" />

Line 223 through 232 in _parseURLargs.js takes the boot parameter arguments, and loads then using script tag injection:

	// now script inject any boots
	for(var e, i = 0; i < boot.length; i++) {
		if(boot[i]){
			e = document.createElement("script");
			e.type = "text/javascript";
			e.src = boot[i];
			e.charset = "utf-8";
			document.getElementsByTagName("head")[0].appendChild(e);
		}
	}

Our custum _dohConfig,js file looks like this:

// Executed as boot, before doh or dojo is loaded.
// See util/doh/_parseURLargs

// Just extend and overwrite dojoConfig, which is at this time
// known as window.require

// Finally, dojo will be loaded, and then we are of.


(function(){

  function runningFromLocalhost() {
    return (window.location.hostname === "localhost" || window.location.hostname === "127.0.0.1"
      || window.location.hostname === "ecachim.local");
  }

  function remoteUseServer() {
    return !runningFromLocalhost();
  }

  window.require.has["running-doh"] = true;
  window.requir

Change History (6)

comment:1 Changed 7 years ago by bill

Did this get cutoff in the middle? You didn't say what the race condition is. Is the script loaded via boot sometimes running after dojo.js starts running?

A document.write() instead of script injection would probably solve that problem. Alternately, have a way to inline the dojoConfig parameters (including static has flags) into the URL, rather than specifying a file.

comment:2 Changed 7 years ago by jandockx

The race condition is indeed that dojo.js kicks of before the config file is done. This becomes clear with some logs and debugging. By setting a breakpoint in dojo.js and _dohConfig, I can get it working after some tries.

The race condition is very clear in Chrome, and rare in FireFox? (on Mac at least).

In the mean time, I can tell that this also is the case without breakOnError=false&sandbox=true&. In that case, I have to add

if (! window.require.has) {
  window.require.has = {}
}

before assigning has-statements, but this has nothing to do with the race condition.

Indeed, the best way to resolve this would be to make it possible to set dojoConfig from the URL. On the other hand, putting such JS info in a parameter doesn't seem trivial (URL escaping seems difficult).

Another alternative would be to make it possible in some way to specify only 1 file (the _dohConfig), where we would call dojo.js. Some kind of HTML fragment, instead of a script file. Maybe that is what you have in mind with document.write() also?

comment:3 Changed 7 years ago by jandockx

And now I see what you mean. The initial report was indeed cutoff in the middle. The bottom text of my initial report is gone.

Indeed, this loading via script tags gives a race condition. In FF, we get it rarely. In WebKit?, it is almost always there. dojo.js _dohConfig.js is done.

comment:4 Changed 7 years ago by bill

Yeah, webkit is so fast that always the browser where we find out about the race conditions.

comment:5 Changed 7 years ago by bill

Resolution: patchwelcome
Status: newclosed

OK, well I think a syntax like this might be good:

runner.html?test=foo/bar&has-touch=true&has-ios=6

If you want to make up a patch for that I'd be happy to look at it. I don't really want to fix the script injection; document.write() is fragile.

comment:6 Changed 6 years ago by jandockx

To be complete, I have a workaround since the original report:

As boot I use our own js file (not dojo - that isn't loaded yet), and instead of adding dojo.js as boot, we load dojo by hand, like doh runner would, at the end of our config file:

runner.html?async=true&boot=../../MYPROJECT/test/_dohConfig.js
// Executed as boot, before doh or dojo is loaded.
// See util/doh/_parseURLargs

// Just extend and overwrite dojoConfig, which is at this time
// known as window.require

// Finally, dojo will be loaded, and then we are of.


(function(){

  var useCors = true;

  if (! window.require.has) {
    window.require.has = {};
  }
  window.require.has["running-doh"] = true;
  window.require.has["dojo-debug-messages"] = true;
  window.require.isDebug = true;

  var userServerBaseUrlChoice = ...;
  window.require.serverBaseUrl = userServerBaseUrlChoice;

  window.require.has["allRemoteTests"] = ...;

  window.require.has["ppwcode/vernacular/persistence/polymorphAmdRevive-debug"] = true;

  window.require["ppwcode-contracts-doh-initialization-done"] = true;
  console.log("loaded _dohConfig.js");

  console.log("starting load of dojo.js");
  var e = document.createElement("script");
  e.type = "text/javascript";
  e.src = "../../dojo/dojo.js";
  e.charset = "utf-8";
  document.getElementsByTagName("head")[0].appendChild(e);
  console.log("injection of dojo.js done");
})();

Note: See TracTickets for help on using tickets.