yellow team mailing list archive

[Benjamin Saller <benjamin.saller@xxxxxxxxxxxxx>]

This looks good, and looks like a real improvement, I made some changes
below that I think help as well.

Not sure why Y.use around the tests is an anti-pattern. With this style
of loading it all up front we could almost get away with use('*') which
would bind all the modules to the Y object but that gives up something
about scoped dependencies in the tests. Happy with the improvement.


https://codereview.appspot.com/7003054/diff/1/test/index.html
File test/index.html (left):

https://codereview.appspot.com/7003054/diff/1/test/index.html#oldcode44
test/index.html:44: <script>
I changed to a pre-load global config object and added use of the
delayUntil option. I think this makes this already nice improvement a
little nicer.

=== modified file 'test/index.html'
--- test/index.html	2012-12-24 03:17:11 +0000
+++ test/index.html	2012-12-24 18:21:50 +0000
@@ -53,29 +53,29 @@


    <script>
-  YUI({'async': false}).use('node', 'event', function(Y) {
-     Y.on('domready', function() {
-
-     var config = GlobalConfig;
-
-     config.async = false;
-     config.consoleEnabled = true;
-
-     for (group in config.groups) {
+  YUI_config = {
+      async: false,
+      consoleEnabled: true,
+      delayUntil: 'domready'
+  };
+
+  YUI().use(['node', 'event'], function(Y) {
+      var config = GlobalConfig;
+
+      for (group in config.groups) {
            var group = config.groups[group];
-         for (m in group.modules) {
-            var resource = group.modules[m];
-            if (!m || !resource.fullpath) {
-              continue
-            }
-            resource.fullpath = resource.fullpath.replace(
-              '/juju-ui/', '../juju-ui/');
-         }
-     }
-     // Run the tests.
-     if (window.mochaPhantomJS) { mochaPhantomJS.run(); }
-     else { mocha.run(); }
-     });
+          for (m in group.modules) {
+              var resource = group.modules[m];
+              if (!m || !resource.fullpath) {
+                  continue
+              }
+              resource.fullpath = resource.fullpath.replace(
+                  '/juju-ui/', '../juju-ui/');
+          }
+      }
+      // Run the tests.
+      if (window.mochaPhantomJS) { mochaPhantomJS.run(); }
+      else { mocha.run(); }
    });
    </script>

https://codereview.appspot.com/7003054/

-- 
https://code.launchpad.net/~hazmat/juju-gui/reliable-test/+merge/141197
Your team Juju GUI Hackers is requested to review the proposed merge of lp:~hazmat/juju-gui/reliable-test into lp:juju-gui.