Dart Test Helpers

‹prev | My Chain | next›

I find myself repeating a lot of the same test actions in my ICE Code Editor test suite. This seems a fine opportunity to explore test helpers in Dart unit tests.

The one in particular that I find myself doing a lot is clicking on element, usually with particular content:

      queryAll('button').
        firstWhere((e)=> e.text=='☰').
        click();

My first instinct is that the helpers should be a separate library that is imported into my test suite. The main reason is that I can import with an “as” prefix to make it patently obvious where the helper methods live. So, in my main ice_test.dart main test file, I add the import statement:

library ice_test;

import 'package:unittest/unittest.dart';
// ...
import 'helpers.dart' as helpers;
import 'package:ice_code_editor/ice.dart';

main(){
  // tests go here...
}

In helpers.dart, I start with a single click() function:

library ice_test_helpers;

import 'dart:html';

void click(String selector, {text}) {
  if (text == null) return query(selector).click();

  queryAll(selector).
    firstWhere((e)=> e.text==text).
    click();
}

The click function requires a string selector that will be used to query for elements to click. If no text is specified—if the optional, named parameter text is null—then I query for the first matching selector and click it. If the text parameter is specified, then I query for all matching selectors, find the first that contains the supplied text and click that.

I continue to use the firstWhere() because it will throw an exception if no matching element is found. I may want to bundle that into a new exception that makes it more obvious what has gone wrong in the test, but I leave it for now.

With that, I can change the test that verifies one of the ways to close a menu:

    test("the menu button closes the projects dialog", (){
      queryAll('button').
        firstWhere((e)=> e.text=='☰').
        click();

      queryAll('li').
        firstWhere((e)=> e.text=='Projects').
        click();

      queryAll('button').
        firstWhere((e)=> e.text=='☰').
        click();

      expect(
        queryAll('div').map((e)=> e.text).toList(),
        isNot(contains(matches('Saved Projects')))
      );
    });

Instead, I can write that as:

    test("the menu button closes the projects dialog", (){
      helpers.click('button', text: '☰');
      helpers.click('li', text: 'Projects');
      helpers.click('button', text: '☰');

      expect(
        queryAll('div').map((e)=> e.text).toList(),
        isNot(contains(matches('Saved Projects')))
      );
    });

Holy clearer intent Batman! It is much easier to see that this test clicks the menu button, then the Projects menu item, then the menu button again.

I might have omitted the “helpers” prefix from my import statement and thus been able to treat click() as a top-level function. I tend to think that the prefix will aid in long-term maintainability of the test suite as there will never be a question as to the source of the helper function.


Day #757

Verifying Persistent Browser Storage with Dart Integration Tests

‹prev | My Chain | next›

One of the crazy things about blogging every day—even the routine stuff of BDDing a new feature—is my ability to run into seemingly daily problems. Last night, I realized that I could not simulate keyboard events in Dart tests. Thanks to Damon Douglas, my #pairwithme partner, I have a solution. It is not an ideal solution, but it will suffice.

So tonight, I try to find yet another daily problem.

I am going to attempt to drive the saving of projects in the ICE Code Editor with tests. In some ways, this is a useless feature because the editor will (eventually) auto-save on every change. Still, it makes people feel more comfortable if it is around. Also, it is a good opportunity for mayhem as this is the first time that I need to use the Store class, which interfaces with localStorage. Problems are sure to abound!

So I start with a test. I create a new full-screen editor instance, set the content, save it with the menu and then start a new instance. The new instance should retain the contents of the previous session by virtue of the Store class that we wrote a couple of weeks ago:

  group("saving projects", (){
    var editor;

    setUp(()=> editor = new Full(enable_javascript_mode: false));
    tearDown(()=> document.query('#ice').remove());

    test("a saved project is loaded when the editor starts", (){
      editor.content = 'asdf';

      queryAll('button').
        firstWhere((e)=> e.text=='☰').
        click();

      queryAll('li').
        firstWhere((e)=> e.text=='Save').
        click();

      document.query('#ice').remove();
      editor = new Full(enable_javascript_mode: false);

      expect(editor.content, equals('asdf'));
    });
  });

I probably need to write some helper functions for clicking buttons and menu items, but I will leave that for tomorrow. I have other problems tonight. Specifically, since there is no save action yet, I get a nice failing test:

FAIL: saving projects a saved project is loaded when the editor starts
  Expected: 'asdf'
       but: was ''.

In the Full class for full-screen editing, I need to initialize an instance of the Store class. The constructor is just the place for this:

class Full {
  Editor _ice;
  Store _store;

  Full({enable_javascript_mode: true}) {
    // ...
    _ice = new Editor('#ice', enable_javascript_mode: enable_javascript_mode);
    _store = new Store();
    // ...
  }
  // ...
}

Next, I need a menu item that will do the saving of the contents:

  Element get _saveMenuItem {
    return new Element.html('<li>Save</li>')
      ..onClick.listen((e)=> _save());
  }

  void _save() {
    _store['asdf'] = {'code': content};
  }

The _saveMenuItem getter returns an <li> element that, when clicked, calls the _save() method, which is responsible for updating the actual store. The name is obviously wrong and something that a subsequent test will have to drive. But it should suffice for now.

With that, the only other thing that I need to do is update the constructor to set the content from the store (if present):

  Full({enable_javascript_mode: true}) {
    // ..
    _ice = new Editor('#ice', enable_javascript_mode: enable_javascript_mode);
    _store = new Store();
    // ...
    editorReady.then((_)=> content = _store.projects.first['code']);
  }

This makes use of the underlying Future that completes when the JavaScript editor (ACE) finishes loading and doing its own initialization. Of course, that takes some time, which causes my test to still fail. In the test, I also have to wait for the future to complete before checking that the content is retained. Dart may not do me any favors when testing keyboard events, but it does make testing asynchronous events a breeze:

  group("saving projects", (){
    var editor;

    setUp(()=> editor = new Full(enable_javascript_mode: false));
    tearDown(() {
      document.query('#ice').remove();
      new Store().clear();
    });

    test("a saved project is loaded when the editor starts", (){
      editor.content = 'asdf';

      queryAll('button').
        firstWhere((e)=> e.text=='☰').
        click();

      queryAll('li').
        firstWhere((e)=> e.text=='Save').
        click();

      document.query('#ice').remove();
      editor = new Full(enable_javascript_mode: false);

      _test(_) {
        expect(editor.content, equals('asdf'));
      };
      editor.editorReady.then(expectAsync1(_test));
    });

The expectAsync1() test function declares to my test that there will be an asynchronous call (and that it will receive one argument) and that the test should not consider itself done until that wrapper is called. Once the expectAsync1() function is called by the editorReady completer, then the private _test() method is called, which checks the editor content.

And it works! If I run the test, I have now added one more passing test to the test suite:

PASS: saving projects a saved project is loaded when the editor starts

All 31 tests passed. 

Best of all, if I remove the restore-from-storage future in the constructor, this test fails, which gives me more confidence in the value of this test.

There is definitely more work ahead of me, but it is pretty exciting to have taken the next step toward a working persistent store in the full-screen version of ICE. It is even more exciting to have it working under a strong test to help guard against regressions.


Day #756

BDD a Dart Menu

‹prev | My Chain | next›

The sharing feature in the ICE Code Editor is not quite done, but I am going to risk starting a new feature tonight. It is a bit of a concern having multiple features under development at the same time, but it could be a good thing for my #pairwithme sessions.

The most interesting next feature is the project menu. What makes it interesting is that it combines three different classes in the project: the core editor, the localStorage, and the full-screen IDE. I start with four empty tests that still start me on the way:

  group("project menu", (){
    skip_test("clicking the project menu item opens the project dialog", (){});
    skip_test("the escape key closes the project dialog", (){});
    skip_test("the menu button closes the projects dialog", (){});
    skip_test("contains a default project on first load", (){});
  });

I will probably leave that last one, which begins to exercise the localStorage component, until tonight's #pairwithme session, but hopefully I can get through the rest.

I start by adding the usual setup and teardown:

  group("project menu", (){
    setUp(()=> new Full(enable_javascript_mode: false));
    tearDown(()=> document.query('#ice').remove());

    skip_test("clicking the project menu item opens the project dialog", (){});
    // ...
  });

The enable_javascript_mode option is a test-only feature. It disables JavaScript syntax highlighting because the underlying ACE code editor uses web workers, which do not work for file:// URLs. Since the tests are run from a URL like file:///home/chris/repos/ice-code-editor/test/index.html, the web workers are guaranteed to fail, adding messy warnings to the test output.

I convert the first test from a skip_test() to the real thing and set my expectations:

    test("clicking the project menu item opens the project dialog", (){
      queryAll('button').
        firstWhere((e)=> e.text=='☰').
        click();

      queryAll('li').
        firstWhere((e)=> e.text=='Projects').
        click();

      expect(
        queryAll('div').map((e)=> e.text).toList(),
        contains(matches('Saved Projects'))
      );
    });

This test says that, after clicking the menu button and the “Projects” menu item, I expect that the project sub-menu will be open. I like using the firstWhere() method when looking for elements to click in tests—it throws exceptions if there is no matching element. That is what happens in this case:

FAIL: project menu clicking the project menu item opens the project dialog
  Caught Bad state: No matching element
  Object&ListMixin.firstWhere                                    dart:collection 561:5
  full_tests.<anonymous closure>.<anonymous closure>             file:///home/chris/repos/ice-code-editor/test/full_test.dart 90:19

This is because I have named the menu item “Open.” I rename it to “Projects” and now I have a failure that the projects dialog is not open:

FAIL: project menu clicking the project menu item opens the project dialog
  Expected: contains match 'Saved Projects'
       but: was <[☰XNewProjectsSaveMake a CopyShareDownloadHelp, ☰, X, , , , , , , , , , , , , X, , , ]>.
  
  full_tests.<anonymous closure>.<anonymous closure>             file:///home/chris/repos/ice-code-editor/test/full_test.dart 93:13

I make that pass by calling a new private method to add the Projects menu item:

    menu.children
      ..add(new Element.html('<li>New</li>'))
      ..add(_projectsMenuItem())
      // ...

The new private method creates the menu item and attaches a click listener:

  _projectsMenuItem() {
    return new Element.html('<li>Projects</li>')
      ..onClick.listen((e) => _openProjectsMenu());
  }

Finally, the menu that opens the project menu and makes the test pass:

  _openProjectsMenu() {
    var menu = new Element.html(
        '''
        <div class=ice-menu>
        <h1>Saved Projects
        </div>
        '''
    );

    el.children.add(menu);

    menu.style
      ..maxHeight = '560px'
      ..overflowY = 'auto'
      ..position = 'absolute'
      ..right = '17px'
      ..top = '60px'
      ..zIndex = '1000';
  }

Both the CSS and the code seem ripe to extraction. I have the feeling that this is not the last menu that I need. Still, I will worry about generalizing the behavior later.

Update: The close-menu-with-escape proves to be extremely difficult. But, thanks to Damon Douglas, tonight's #pairwithme partner, I have a solution.

As best I can tell, there is no way to properly simulate keyboard events in Dart. It is possible to create an instance of KeyboardEvent, but it is not possible to set the charCode of that event—either in the constructor or via a setter. Craziness!

We found that it is possible to specify a key identifier in the constructor. If the character code for the escape key is 27, then I can create an escape keyboard event with:

    test("the escape key closes the project dialog", (){
      // open the project menu

      document.body.dispatchEvent(
        new KeyboardEvent(
          'keyup',
          keyIdentifier: new String.fromCharCode(27)
        )
      );

      expect(
        queryAll('div').map((e)=> e.text).toList(),
        isNot(contains(matches('Saved Projects')))
      );
    });

Unfortunately, this still does not set a charCode on the resulting event. Nevertheless, it does set information that can be passed from the test into the application code. To get this test to pass, we have to check both the charCode and the keyIdentifier values in the application code:

    document.onKeyUp.listen((e) {
      if (e.keyCode == 27) _hideMenu();
      if (e.$dom_keyIdentifier.codeUnits.first == 27) _hideMenu();
    });

It is never a good thing to call one of the dollar-sign methods in Dart. It is also not a good thing to write code just to make a test pass. There is no way that second conditional will get triggered in live code and it does contain the same spirit as the real event conditional on the previous line.

Still, I will be much happier once there is a real way to test keyboard events in Dart. This is a useful first approximation, but nothing beats the real thing.


Day #755

Headless Testing in Dart (Take Two)

‹prev | My Chain | next›

I am pretty jazzed about the progress in both the ICE Code Editor and the test suite that is driving much of its development. I really have the sense that I have the beginnings of a robust codebase. I love Dart. One of the things that I do not have is the ability to test headlessly. This is something that worked way back in the day, but stopped at some point due to a bug in unittest.

Actually, thanks to some feedback on that bug report, it may just be a question of some additional test setup. To test that out, I first upgrade my Dart SDK and unittest. I've been stuck on:

ice-code-editor git:(master) dart --version
Dart VM version: 0.5.0.1_r21823 (Mon Apr 22 14:02:11 2013)

This keeps me back on the 0.5.0 version of unittest:

➜  ice-code-editor git:(master) ls -l packages 
total 16
lrwxrwxrwx 1 chris chris 66 May 16 00:21 browser -> /home/chris/.pub-cache/hosted/pub.dartlang.org/browser-0.5.0+1/lib
lrwxrwxrwx 1 chris chris  6 May 16 00:21 ice_code_editor -> ../lib
lrwxrwxrwx 1 chris chris 60 May 16 00:21 js -> /home/chris/.pub-cache/hosted/pub.dartlang.org/js-0.0.22/lib
lrwxrwxrwx 1 chris chris 63 May 16 00:21 meta -> /home/chris/.pub-cache/hosted/pub.dartlang.org/meta-0.5.0+1/lib
lrwxrwxrwx 1 chris chris 67 May 16 00:21 unittest -> /home/chris/.pub-cache/hosted/pub.dartlang.org/unittest-0.5.0+1/lib

After downloading the latest version of Dart, I have SDK version 0.5.7. A pub install then installs the 0.5.7 version of unittest (among others):

➜  ice-code-editor git:(master) dart --version
Dart VM version: 0.5.7.3_r22659 (Mon May 13 20:57:19 2013) on "linux_x64"
➜  ice-code-editor git:(master) pub update
Resolving dependencies...
Downloading unittest 0.5.7...
Downloading browser 0.5.7...
Downloading meta 0.5.7...
Dependencies updated!

I did not realize that it was possible to pin pub packages like this. I doubt that I will ever need to do something like that, but a quick inspection of unittest's pubspec.yaml shows that the environment property does the trick:

➜  ice-code-editor git:(master) cat ~/.pub-cache/hosted/pub.dartlang.org/unittest-0.5.7/pubspec.yaml 
name: unittest
author: "Dart Team <misc@dartlang.org>"
homepage: http://www.dartlang.org
documentation: http://api.dartlang.org/docs/pkg/unittest
description: >
 A library for writing dart unit tests.
dependencies:
  meta: any


version: 0.5.7
environment:
  sdk: ">=0.5.7"

Anyhow, now that I am on latest, I can see if things have changed with headless Dart testing. Headless testing is done with the DumpRenderTree tool that is bundled with Dart:

➜  ice-code-editor git:(master) DumpRenderTree test/index.html 
CONSOLE MESSAGE: unittest-suite-wait-for-done
Content-Type: text/plain
layer at (0,0) size 800x600
  RenderView at (0,0) size 800x600
layer at (0,0) size 800x600
  RenderBlock {HTML} at (0,0) size 800x600
    RenderBody {BODY} at (8,8) size 784x571
      RenderBlock {H1} at (0,0) size 784x37
        RenderText {#text} at (0,0) size 69x36
          text run at (0,0) width 69: "Test!"
#EOF
#EOF

Since there is no test output, it seems that the bug still exists. So I add the suggested code to the web page that provides the DOM context in which the tests run:

<html>
<head>
  <title>ICE Test Suite</title>
  <script type='text/javascript'>
    var testRunner = window.testRunner || window.layoutTestController;
    if (testRunner) {
      function handleMessage(m) {
        if (m.data == 'done') {
          testRunner.notifyDone();
        }
      }
      testRunner.waitUntilDone();
      window.addEventListener("message", handleMessage, false);
    }
    if (navigator.webkitStartDart) {
      navigator.webkitStartDart();
    }
  </script>
  <script type="application/dart" src="ice_test.dart"></script>
</head>
<body>
<h1>Test!</h1>
</body>
</html>

But, when I run this in DumpRenderTree, the output hangs with:

➜  ice-code-editor git:(master) ✗ DumpRenderTree test/index.html
CONSOLE MESSAGE: unittest-suite-wait-for-done
CONSOLE MESSAGE: line 54: Uncaught ReferenceError: ReceivePortSync is not defined
CONSOLE MESSAGE: Exception: The null object does not have a method 'callSync'.

NoSuchMethodError : method not found: 'callSync'
Receiver: null
Arguments: [GrowableObjectArray len:0]
CONSOLE MESSAGE: Exception: The null object does not have a method 'callSync'.

NoSuchMethodError : method not found: 'callSync'
Receiver: null
Arguments: [GrowableObjectArray len:0]

After some time, this eventually returns with:

FAIL: Timed out waiting for notifyDone to be called
FAIL: Timed out waiting for notifyDone to be called

I have seen the ReceivePortSync error before when testing js-interop. To fix, I cannot manually start the Dart engine with navigator.webkitStartDart()—I need a more complete start of the engine. So I remove the conditional above, replacing it with a <script> source that pulls in Dart's browser/dart.js:

  <script type='text/javascript'>
    var testRunner = window.testRunner || window.layoutTestController;
    if (testRunner) {
      function handleMessage(m) {
        if (m.data == 'done') {
          testRunner.notifyDone();
        }
      }
      testRunner.waitUntilDone();
      window.addEventListener("message", handleMessage, false);
    }
    // if (navigator.webkitStartDart) {
    //   navigator.webkitStartDart();
    // }
  </script>
  <script src="packages/browser/dart.js"></script>
  <script type="application/dart" src="ice_test.dart"></script>

With that, I do see my test output:

➜  ice-code-editor git:(master) ✗ DumpRenderTree test/index.html
CONSOLE MESSAGE: unittest-suite-wait-for-done
CONSOLE MESSAGE: PASS: defaults defaults to auto-update the preview
CONSOLE MESSAGE: PASS: defaults starts an ACE instance
CONSOLE MESSAGE: PASS: defaults defaults to disable edit-only mode
CONSOLE MESSAGE: PASS: content can set the content
...
CONSOLE MESSAGE:
CONSOLE MESSAGE: All 29 tests passed.
CONSOLE MESSAGE: unittest-suite-success

Unfortunately, it still hangs at this point. Eventually, I get notifyDone errors:

FAIL: Timed out waiting for notifyDone to be called
FAIL: Timed out waiting for notifyDone to be called

Taking a closer look at the supplied runner code, I see that, to notify the test runner that it is done, a message with the contents "done" needs to be posted:

      function handleMessage(m) {
        if (m.data == 'done') {
          testRunner.notifyDone();
        }
      }

I am unsure if the unittest library is supposed to post that message. Regardless of whether it is supposed to send the message, it is not. So it seems that I need to add a way to poll for the test cases to be complete (there are no tests-done Futures in unittest).

To poll, I import the dart:async library for access to the Timer class. The pollForDone() function can then be added to my test suite as:

library ice_test;

import 'package:unittest/unittest.dart';
import 'dart:html';
import 'dart:async';
import 'package:ice_code_editor/ice.dart';
// ...
main(){
  // Run tests here...
  pollForDone(testCases);
}

pollForDone(List tests) {
  if (tests.every((t)=> t.isComplete)) {
    window.postMessage('done', window.location.href);
    return;
  }

  var wait = new Duration(milliseconds: 100);
  new Timer(wait, ()=> pollForDone(tests));
}

Given the list of all test cases, this checks if every one of them is complete. If they are, then it posts the 'done' message for the test runner and exits. If some tests are incomplete, then it waits for 100 milliseconds before trying again.

And that does the trick! The test suite still runs in Dartium and now it runs headless as well:

➜  ice-code-editor git:(master) ✗ time DumpRenderTree test/index.html
CONSOLE MESSAGE: unittest-suite-wait-for-done
CONSOLE MESSAGE: PASS: defaults defaults to auto-update the preview
CONSOLE MESSAGE: PASS: defaults starts an ACE instance
CONSOLE MESSAGE: PASS: defaults defaults to disable edit-only mode
CONSOLE MESSAGE: PASS: content can set the content
...
CONSOLE MESSAGE: PASS: sharing menu should close when share dialog activates

CONSOLE MESSAGE:
CONSOLE MESSAGE: All 29 tests passed.
CONSOLE MESSAGE: unittest-suite-success
...
DumpRenderTree test/index.html  0.77s user 0.10s system 100% cpu 0.870 total

The pollForDone() function is a bit of a hassle, but one that I am happy to live with if it gets me headless testing. Still, I will post that solution back onto the bug to see if there is some way that unittest could post the 'done' message itself.


Day #754

Getting Started BDDing Dart HTML Workflows

‹prev | My Chain | next›

Now that I have a beautiful Dart test suite for ICE Code Editor, I would like to try driving a little UI workflow.

For my first test, I write something that is decidedly not behavior driving. I check for the presence of a button:

  group("main toolbar", (){
    setUp(()=> new Full(enable_javascript_mode: false));
    tearDown(()=> document.query('#ice').remove());

    test("it has a menu button", (){
      var buttons = document.queryAll('button');
      expect(buttons.any((e)=> e.text=='☰'), isTrue);
    });
  });

In other words, when the full screen editor first starts up, there should be a menu button available.

I am not going to test for location or z-index or any formatting here. I will visually inspect that they exist. It might be nice if Dart supported something like a visible getter, but perhaps that is something for a library. Anyhow I will get the toolbar in place, then I will write another test to drive some UI behavior.

But first, I need to get the test passing:

FAIL: main toolbar it has a menu button 
  Expected: true
       but: was <false>.

I make that pass by modifying the Full class to attach a toolbar on construction. And, I have the toolbar create the menu button:

class Full {
  Full({enable_javascript_mode: true}) {
    // ...
    _attachToolbar();
  }

  _attachToolbar() {
    var el = new Element.html('<div class=ice-toolbar>');
    el.style
      ..position = 'absolute'
      ..top = '10px'
      ..right = '20px'
      ..zIndex = '999';

    _attachMenuButton(el);

    document.body.nodes.add(el);
  }

  _attachMenuButton(parent) {
    var el = new Element.html('<button>☰');
    parent.children.add(el);
  }
}

That gets my first test passing:

PASS: main toolbar it has a menu button

Now, I am ready to write my first UI workflow test—nothing fancy, just that clicking this button brings up the menu that includes a “Help” item:

    test("clicking the menu button brings up the menu", (){
      var menu_button = queryAll('button').
        firstWhere((e)=> e.text=='☰');

      menu_button.click();
      var menu = queryAll('li').
        firstWhere((e)=> e.text.contains('Help'));

      expect(menu, isNotNull);
    });

And make it pass with:

  toggleMenu() {
    var el = new Element.html('<ul class=ice-menu>');
    document.body.children.add(el);

    el.style
      ..position = 'absolute'
      ..right = '17px'
      ..top = '55px'
      ..zIndex = '999';

    el.children
      ..add(new Element.html('<li>New</li>'))
      ..add(new Element.html('<li>Open</li>'))
      ..add(new Element.html('<li>Save</li>'))
      ..add(new Element.html('<li>Make a Copy</li>'))
      ..add(new Element.html('<li>Share</li>'))
      ..add(new Element.html('<li>Download</li>'))
      ..add(new Element.html('<li>Help</li>'));
  }

That was easy!

My #pairwithme, Srdjan Pejic, and I spend the rest of the evening building out a few of those menu elements—BDDing as much as possible along the way. So far, I have to say that I really like driving UI changes like this. It will be interesting to see if my happiness level remains high as the UI (and complexity) grows.


Day #753

One Main() for Great Good of Dart Tests

‹prev | My Chain | next›

Thanks largely to Damon Douglas, my #pairwithme pair, I have the test suite for the Dart version of the ICE Code Editor in much better shape.

When we started last night all the tests passed, but there was lots of red and other weird error-like output when I ran the test suite:



When we finished, all of that was gone. Well, almost all of it:



It was only one red line, but we gave it a good try to eliminate that one red line. Unfortunately, it was late and everything we did only made things worse. Compounding the problems was that the single red line often grew into a full stack trace dripping with js-interop influence:

Exception: Non unique ID: dart-0
Stack Trace: #0      Proxy._forward (file:///home/chris/repos/ice-code-editor/test/packages/js/js.dart:1043:22)
#1      Proxy.noSuchMethod (file:///home/chris/repos/ice-code-editor/test/packages/js/js.dart:1033:20)
#2      Ace.edit (file:///home/chris/repos/ice-code-editor/test/packages/ice_code_editor/editor.dart:238:65)
#3      Editor._startJsAce (file:///home/chris/repos/ice-code-editor/test/packages/ice_code_editor/editor.dart:180:20)
#4      Editor._startAce.<anonymous closure> (file:///home/chris/repos/ice-code-editor/test/packages/ice_code_editor/editor.dart:171:52)

Last night, we tried to tweak the code to prevent the problem. Fresh eyes today make me realize that I am probably going to have to re-organize my test suite to eliminate this problem.

What leads me to believe that a code re-organization is in the cards is that the various specs—for the core editor, the storage, the full-screen editor—all run fine in isolation. This particular problem only pops up when two different test groups try to run the same js-interop code. I do not think this is a limitation of js-interop or of Dart. I think it is more a case that the organization scheme that I have chosen was, in hindsight, poor.

So far, all of my tests have needed to run in the browser. Since I have multiple classes in ICE, I have been creating a new file to test each of these classes. I think that is OK, but what is not OK—at least not when there is certain kinds of js-interop work involved—is how I included the tests. On the web page that provided the test suite context, I had included the various test files as:

<head>
  <title>ICE Test Suite</title>
  <script type="application/dart" src="editor_test.dart"></script>
  <script type="application/dart" src="store_test.dart"></script>
  <script type="application/dart" src="gzip_test.dart"></script>
  <script type="application/dart" src="full_test.dart"></script>
</head>

Each of those _test.dart files had their own main() entry point. For normal Dart code (and even certain classes of tests), this is perfectly OK. What makes it not OK is that the editor.dart pulled JS into its own isolate:

import 'package:unittest/unittest.dart';
import 'dart:html';
import 'package:ice_code_editor/ice.dart';

main() {
  // tests that create js-interop proxies
}

And then the full_test.dart did the exact same thing:

import 'package:unittest/unittest.dart';
import 'dart:html';
import 'package:ice_code_editor/ice.dart';

main() {
  // tests that create js-interop proxies
}

So both tests have their own main isolate, but share libraries and ultimately try to share js-interop proxies into the same JavaScript code (the ACE code editor in this case). The first time through, everything is OK. The second time through, js-interop tries to setup the first proxy in the new isolate, only to find that it has already started a zeroeth proxy. Even if that succeeds (and I'm not sure how it could), there is still a problem caused by double-loading ACE JavaScript script files that result in the cannot read property 'ace/ace' of undefined. Some of what Damon and I did last night ensured that the ACE JavaScript source would only load once. But obviously that only works in a single isolate.

So it seems clear that I have no choice but to run all of my tests in the same main() isolate. If I want to retain the same separation of testing concerns, this means that I need to move my test files into Dart “parts.”

Starting from the top, I load a single (new) ice_test.dart source file into my testing web page context:

<head>
  <title>ICE Test Suite</title>
  <script type="application/dart" src="ice_test.dart"></script>
  <script src="packages/browser/dart.js"></script>
</head>

In ice_test.dart, I import the packages necessary to run all of my tests: unittest, dart:html and, of course, ice. Then I declare the various parts that make up this testing library:

library ice_test;

import 'package:unittest/unittest.dart';
import 'package:ice_code_editor/ice.dart';
import 'dart:html';

part 'editor_test.dart';
part 'store_test.dart';
part 'gzip_test.dart';
part 'full_test.dart';

main(){
  editor_tests();
  store_tests();
  gzip_tests();
  full_tests();
}

I have to give separate names to each of the functions that run the tests in editor_test.dart, full_test.dart, etc. so that the names do not clash. I start with the convention of using the plural of the file name.

The last thing that I need to do is declare each of the _test.dart files as parts of the ice_test library. The editor_test.dart file then becomes:

part of ice_test;

editor_tests() {
  // test here
}

After doing the same in the other test files, I am ready to run my test suite and... it works!



Yay! No red. All in all, I am pretty happy with that solution. I might prefer not to have to follow the convention of popularizing the the test functions, but if that is the only bother that I have in my test organization, I can live with it.


Day #752

Better DOM Tests in Dart

‹prev | My Chain | next›

Last night I and Santiago Arias (my #pairwithme pair) made a decent start on the full-screen, mini-IDE version of the Dart-flavor ICE Code Editor. We are finding that the test suite is growing messy—primarily, I suspect, due to the manner in which all of the tests run asynchronously (i.e. at once). With so many elements all creating an instance of the ICE Code Editor and tearing down the DOM elements used to build it, tests are stomping on each other. Tonight, I hope to fix that up a bit.

Dart provides facilities to run groups of tests serially, but I do not think that I need make use of that—or at least I think that I can address most of my problems without doing that. To illustrate the point, most of the errors are along the lines of:

TypeError {} ace.js:5464
Could not load worker ace.js:5463
TypeError {} ace.js:5464
Uncaught TypeError: Cannot read property 'ace/ace' of undefined ace.js:114
Could not load worker ace.js:5463
TypeError {} ace.js:5464
Uncaught TypeError: Cannot read property 'ace/ace' of undefined ace.js:114
Could not load worker ace.js:5463
TypeError {} 

My initial thought is that my setup and teardown are being overwritten by async tests all writing to the <div> tag with the same ID:

  group("content", () {
    var el;
    setUp(() {
      el = new Element.html('<div id=ice>'); document.body.nodes.remove(el));

    test("can set the content", () {
      var it = new Editor('#ice');
      // ...
    });
  });

There are more tests that do the same, so I try creating unique IDs using the test-case ID:

group("content", () {
    setUp(() {
      var el = new Element.html('<div id=ice-${currentTestCase.id}>');
      document.body.nodes.add(el);
    });
    tearDown(() {
      var el = document.query('#ice-${currentTestCase.id}');
      document.body.nodes.remove(el);
    });

    test("can set the content", () {
      var it = new Editor('#ice-${currentTestCase.id}');
      // ...
    });
  });

If I am right, then using unique <div> tags ought to clean up the test cases.

It turns out that I am wrong. I still see the same errors. So instead, I work with Damon Douglas, tonight's #pairwithme pair, to get to the bottom of this. The bottom turns out to be the ACE JavaScript editor that is being loaded via js-interop. Loading ACE once is fine. Loading it several times as in test cases: not so fine.

The solution is to move the dynamic creation of the <script> tags for the ACE editor into a one-time only static method in the ICE.Editor class:

  static _attachScripts() {
    if (_isAceJsAttached) return;

    var script_paths = [ /* ... */ ];

    var scripts = script_paths.
      map((path) {
        var script = new ScriptElement()
          ..async = false
          ..src = path;
        document.head.nodes.add(script);
        return script;
      }).
      toList();

    return _scripts = scripts;
  }

If the private method indicating that the ACE JavaScript is already loaded returns true, then there is no need to dynamically create new <script> elements that load ACE again and again.

I really seem to be paying the price for dynamically creating those ACE <script> tags. It was a pain to get working in the first place and it is a pain to be able to test with them in the way. And yet, I cannot bring myself to get rid of them. From the outside world, a developer should rarely need to worry about this. Even if it is a problem, there is a single Future, editorReady, that completes when everything is in place. All of this makes for a web page that contains no more than:

<head>
  <script src="packages/browser/dart.js"></script>
  <script type="application/dart">
    import 'package:ice_code_editor/ice.dart' as ICE;
    main()=> new ICE.Full();
  </script>
</head>

With just that, all of the JavaScript is loaded, along with the Dart that controls it. This still seems a worthy goal. And, since I have my tests in better shape thanks to Damon, I will stick to this path for a little while longer.


Day #751