Dart Testing with Lots of Events

‹prev | My Chain | next›

The ICE Code Editor boasts a minimal set of controls:



The subject of tonight's exploration is the Update button—more specifically the checkbox in the Update button which governs auto-update. If checked, code changes are reflected automatically in the preview layer. If unchecked, then the only way to force an update is to click the Update button. That all works fine and is well tested, thanks to Dart's lovely unittest library.

What could work a little better is checking the auto-update checkbox. A programmer clicking that checkbox wants auto-update re-enabled—and that works. But it is also almost certainly true that she wants any code changes since auto-update was disabled to be reflected immediately. And that does not work.

I think I have a fairly good idea how to implement that. More perplexing is how to test that. Thanks to Dart's iframe security restrictions, I cannot (easily) query the contents of the preview iframe to verify that the update occurred correctly. Instead I have to rely on secondary indicators like the onPreviewChanged event stream. This is how existing tests verify the update feature:

    test("updates the preview layer", (){
      helpers.createProject("My Project");
      document.activeElement.
        dispatchEvent(new TextEvent('textInput', data: '<h1>Hello</h1>'));

      editor.onPreviewChange.listen(expectAsync1((_)=> true));

      helpers.click('button', text: " Update");
    });

Here, I am using unittest's expectAsync1(), which will poll until a callback is invoked with a single argument, to test my expectation. As long as the onPreviewChange event occurs, this test passes.

For my code tonight, I need to first uncheck the checkbox (by clicking it), then sending a text event with new content, and finally re-checking the auto-update checkbox and verifying that a change event occurs. Something like this ought to do the trick:

    test("checking the checkbox updates the preview layer", (){
      var button = helpers.queryWithContent("button","Update");
      var checkbox = button.query("input[type=checkbox]");
      checkbox.click();

      document.activeElement.
        dispatchEvent(new TextEvent('textInput', data: '<h1>Hello</h1>'));

      editor.onPreviewChange.listen(expectAsync1((_)=> true));

      checkbox.click();
    });

That ought to work, except it passes. That is, even though I know that the feature is not working, my test does not capture this.

After messing around a bit, I find that… this really ought to work. I confirm that the checkbox is unchecked and rechecked. I confirm that the event does not propagate (clicking the Update button to manually update the preview). I confirm that the editor is, indeed, disabled from auto-updating—at least for a little while.

And that little while turns out to be the key. I am dispatching a text event and then immediately unchecking the auto-update box. This immediate unchecking winds update re-enabling auto-update before the text event propagates through the editor. So that, by the time the editor is aware that an update has occurred, it is already in auto-update mode.

To fix, I need a little delay:

    test("checking the checkbox updates the preview layer", (){
      var button = helpers.queryWithContent("button","Update");
      var checkbox = button.query("input[type=checkbox]");
      checkbox.click();

      document.activeElement.
        dispatchEvent(new TextEvent('textInput', data: '<h1>Hello</h1>'));

      editor.onPreviewChange.listen(expectAsync1((_)=> true));

      var wait = new Duration(milliseconds: 100);
      new Timer(wait, (){
        checkbox.click();
      });
    });

That does the trick. I now have a failing test that accurately captures my buggy behavior. When I fix it, I know that I will have a test that can catch regressions.

And as expected, fixing the bug turns out to be fairly easy. I make use of the iterable nature of stream by adding a where() clause to the checkbox's onClick stream. I only want to listen to streams that contain events in which the checkbox is checked:

    return _update_button = new Element.html('''
        <button>
           <input checked type=checkbox/>
           Update
         </button>'''
      )
      // other listeners here...
      ..query("input").onChange.
          where((e) => e.target.checked).
          listen((e)=> ice.updatePreview());

With that, I have another feature complete and am comforted in the knowledge that I have a strong test describing it. That is a good win.


Day #798

The Very Best Format for Dart Optional Constructor Parameters

‹prev | My Chain | next›

I dearly love optional parameters in Dart methods, functions and especially constructors. That said…

A long list of constructor arguments tends to raise my code-too-wide hackles. Consider the constructor for the Editor class in the ICE Code Editor. It has one required argument, a DOM element to hold the editor, and four optional, named parameters:

class Editor {
  Editor(this._el, {this.edit_only:false, this.autoupdate:true, this.title, this.enable_javascript_mode:true}) {
    this._startAce();
    this._applyStyles();
  }
  // ...
}

I like to keep my lines of code under 60 characters wide. This is partially superstition and partially legitimate suspicion. The suspicion is that wide code is hiding complexity, which is a natural enemy of maintainability.

It is hard to get too worked up about constructor arguments, but they do have a significant effect on behavior. The above constructor accepts an edit_only parameter, which defaults to false. It accepts autoupdate, which defaults to true. It accepts title, which is null by default. Finally it accepts enable_javascript_mode, which defaults to true.

OK, so maybe it is easy to get worked up about it when I list those out. Each of those have behavior implications and burying defaults in a long line of unrelated defaults seems like a good way to make the code and intent less readable.

I am not a fan of placing one parameter per line as it then becomes difficult to distinguish between constructor arguments and constructor body:

class Editor {
  Editor(this._el, {
    this.edit_only:false,
    this.autoupdate:true,
    this.title,
    this.enable_javascript_mode:true
  }) {
    this._startAce();
    this._applyStyles();
  }
  // ...
}

In my day dreams, it might be nice to use Dart's redirecting constructors to set optional parameters and then redirect to a “real” constructor:

class Editor {
  Editor.config(this._el, {
    this.edit_only:false,
    this.autoupdate:true,
    this.title,
    this.enable_javascript_mode:true
  }) : this();

  Editor() {
    this._startAce();
    this._applyStyles();
  }
  // ...
}

But that will not work because redirecting constructors cannot use field initializers like that.

So I am left to decide how best to format my largish constructor for optimum readability. It should be obvious that the list of parameters and constructor body are logically separate. It should be obvious that optional parameters are of less importance than required parameters. Perhaps something like this:

class Editor {
  Editor(
    this._el, {
      this.edit_only:false,
      this.autoupdate:true,
      this.title,
      this.enable_javascript_mode:true
    }) 
  { this._start(); }
  // ...
}

I think I can live with that. The constructor name, Editor, has the same indent level as the constructor body (which I squash into a single line). The required parameter, which is assigned to the _el private variable via field initialization, is clearly part of the constructor parameters. By indenting it, it makes the body stand out as logically different that parameters. Finally, by indenting the optional, named parameters, it further aids to distinguish between constructor parameters and body while also suggesting that the optional parameters are less important than the required parameter.

I think I can live with that. I may even like it. Except…

Now that I have the parameters unrolled like that, I finally ask myself if they are needed at all. In fact, the title parameter is never used in the Editor, so I eliminate that right away. The edit_only and autoupdate parameters are not used in any of the code or tests written so far, so neither needs to be in the parameter list (though their default values still need to be set). This leaves me with:

class Editor {
  Editor(this._el, {this.enable_javascript_mode:true}) {
    this._startAce();
    this._applyStyles();
    this._edit_only = false;
    this.autoupdate = true;
  }
  // ...
}

Actually, there are better ways to set default values for instance variables than setting them in constructors. Simply declaring them with default values will work fine:

class Editor {
  bool autoupdate = true;
  bool _edit_only = false;

  Editor(this._el, {this.enable_javascript_mode:true}) {
    this._startAce();
    this._applyStyles();
  }
  // ...
}

That is a lot better. The only thing that really stand out now is the enable_javascript_mode instance variable. In addition to visually standing out, it also bothers me that the only reason this instance variable exists is to support testing (it disables a web worker, which will not work with a file:// URL like test pages). In the setup of many of my tests in ICE, I use this to disable JavaScript mode:

    test("defaults to auto-update the preview", () {
      var it = new Editor('#ice-${currentTestCase.id}', enable_javascript_mode: false);
      expect(it.autoupdate, equals(true));
      expect(it.editorReady, completes);
    });

Now that I think about it—now that re-organizing my code forced me to think about it—that does not need to be an instance variable. Instead, I can make it a static variable:

class Editor {
  bool autoupdate = true;
  bool _edit_only = false;

  static bool disableJavaScriptWorker = false;

  Editor(this._el) {
    this._startAce();
    this._applyStyles();
  }
  // ...
}

Then, I can remove all of those enable_javascript_mode constructor parameters and instead set that static variable once:

library ice_test;

main(){
  Editor.disableJavaScriptWorker = true;
  // ...
}

So in the end, I think that I have a decent format for a large number of constructor arguments. But I think that I have an even better constructor without any optional parameters at all—ironically because I worked so hard to organize the same parameters in the first place. Regardless, the code is in better, more readable, maintainable shape, which is what is really important.


Day #797

Still Can't Dynamically Create Keyboard Events in Dart (But Getting Closer)

‹prev | My Chain | next›

For all the work that I have doing recently with it, I should really write a book about Dart testing. It really is a blast and is so nice to have a solid client-side testing library that can also be easily run under continuous integration. The problem, of course, it that I know too much about it now—I would break my code for writing books and based on the last season of Dexter, breaking a code is clearly a bad thing.

Anyhow, tonight's exploration of Dart testing came from a comment from Don Olmstead on an earlier post about what a pain it is to test (or simulate) keyboard events in Dart. As an aside, if the measure of knowing a technology is understanding its weaknesses, I am certainly getting there. Don suggested that I investigate KeyName.ENTER rather than keyCode .

The KeyName values are string representations of non-string things like the Enter and Escape keys, which are really what I want to test (I used TextEvent to test “real” input). The documentation for KeyName mentions that these values are the things that would be returned from KeyEvent.getKeyboardIdentifier, but there does not seem to be a getKeyBoardIdentifier (or any variation of that name) in KeyEvent or the underlying KeyboardEvent. For now, I latch onto the only string values in KeyboardEvent, which are keyIdentifier in the constructor and the $dom_keyIdentifier getter.

So my testing helpers for hitting the Enter and Escape keys become:

hitEnter()=> type(KeyName.ENTER);
hitEscape()=> type(KeyName.ESC);

type(String key) {
  document.activeElement.dispatchEvent(
    new KeyboardEvent(
      'keyup',
      keyIdentifier: key
    )
  );
}

I now use the KeyName values for Enter and Escape, which are strings, and supply them to the keyIdentifier optional parameter in the KeyboardEvent constructor.

In my application code, based on my earlier unsuccessful attempts to legitimately simulate keyboard events in Dart, I have two helper methods that determine if key presses are Escape and Enter keys:

_isEscapeKey(e) =>
  e.keyCode == 27 || e.$dom_keyIdentifier.codeUnits.first == 27;

_isEnterKey(e) =>
  e.keyCode == 13 || e.$dom_keyIdentifier.codeUnits.first == 13;

The first part the boolean OR statements are the standard keyCode checks. The second part of the booleans were my hack for testing. My hope is that I can actually get rid of the keyCode portion of the boolean and compare the $dom_keyIdentifier with the appropriate KeyName value:

_isEscapeKey(e) =>
  e.$dom_keyIdentifier == KeyName.ESC;

_isEnterKey(e) =>
  e.$dom_keyIdentifier == KeyName.ENTER;

And that actually seems to work. With that change, all 125+ tests in the ICE Code Editor are again passing.

That is not quite the end of the story, however. This only means that I have been able to satisfactorily simulate keyboard events in my tests. What about in real life?

Unfortunately, real life betrays me. When I try this out in Dartium, the Enter key works, but the Escape key does not. Further investigation reveals that the codeUnits (the underlying bytes of the string) of the escape key event, [85, 43, 48, 48, 49, 66], do not match the KeyName code units, [69, 115, 99]. The number of bytes is not even correct—let alone the bytes matching up. Interestingly, the Enter key also has five bytes and, since the event and the KeyName seem to agree, it would seem the KeyName value for Escape is simply wrong.

So, my nice, compact _isEnterKey(e) application method becomes:

_isEscapeKey(e) {
  return e.$dom_keyIdentifier == KeyName.ESC ||
    e.$dom_keyIdentifier == new String.fromCharCodes([85, 43, 48, 48, 49, 66]);
}

With that, I have all tests passing and it works in real life. But I am still not quite done.

Since Dart lacks a keyIdentifier getter for KeyboardEvent objects, I am stuck reaching under the covers with the $dom_keyIdentifier getter. The problem is that not all browsers may support this. So I have to compile my example down into JavaScript to test in FireFox and Internet Explorer.

And, unfortunately, this fails to work in either. I get an error about trying to read get$codeunits from an undefined or null reference because the keyboard event in both FireFox and IE do not support this property. Bugger.

So, in the end, I am forced to settle for a slightly better KeyName-based implementation. Instead of mucking about with codeUnits:

_isEnterKey(e) =>
-  e.keyCode == 13 || e.$dom_keyIdentifier.codeUnits.first == 13;

I can now be explicit about the key in question:

_isEnterKey(e) =>
  e.keyCode == 13 || e.$dom_keyIdentifier == KeyName.ENTER;

As frustrating as this is, it does amount to progress. One has to assume that Dart will support keyIdentifier at some point—in either KeyboardEvent or KeyEvent, normalizing all of this stuff for developers. I do believe that I will surf through bug.dartlang.org bit tonight to see if any of this is in the works and possibly add an issue or two.


Day #796

Manually Cleaning Up Dart Stream Subscriptions

‹prev | My Chain | next›

I had another wildly successful #pairwithme session last night with Santiago Arias. “Wildly successful” in my mind is completing a feature in one session that I had expected to require multiple #pairwithme sessions. Alas, this means that I have to queue up the next meaty project for #pairwithme, but that is a good problem to have™.

The feature was the ability to filter projects in the ICE Code Editor:



As usual, we made extensive use of the unit test libraries in Dart to drive this feature and it all came off swimmingly. Except…

We now have approximately 496 of the following exceptions being noted in the test suite:

Exception: Bad state: No elements                dart:collection/list.dart:20
Object&ListMixin.first                           dart:collection/list.dart:20
_isEscapeKey                                     package:ice_code_editor/full.dart:259
Full._attachKeyboardHandlers.<anonymous closure> package:ice_code_editor/full.dart:165

OK, “approximately” is not the right word. “Exactly” is the right word. We have 496 of those exceptions being logged. All 126 of the tests all still pass, but they now produce those errors.

Actually, it is not 126 test that produce those errors—it is only 4 of the new tests that produce the errors. I know what is causing the errors. The stacktrace makes it pretty plain that the errors are being caused by my solution to generating keyboard events in Dart. The hacky solution that I use is to check the key identifier when the keyCode property is not available:

_isEscapeKey(e) =>
  e.keyCode == 27 || e.$dom_keyIdentifier.codeUnits.first == 27;

The reason that this popped up last night was that we used a different, TextEvent-based keyup strategy for generating input. So the obvious fix is to accommodate text events in my silly hack (or to switch from text events in our new tests).

I can address that later. What concerns me now are those errors. They are the result of keyboard listeners from old instances of the ICE editor sticking around after tests. I have been cleaning up after tests by removing the DOM element and allowing garbage collection to do the rest:

    tearDown(() {
      document.query('#ice').remove();
      editor.store..clear()..freeze();
    });

Unfortunately, it seems garbage collection is not enough in the case of Dart stream subscriptions. Instead, I have squirrel away a copy of the stream subscription so that a new, custom-built remove() method can manually remove the stream subscription:

class Full {
  // ...
  void remove() {
    _keyUpSubscription.cancel();
    el.remove();
  }
  // ...
}

With that, I can remove every instance of the DOM element removal with a call to the editor's remove() method that both removes the DOM element and the stream subscription:

    tearDown(() {
      editor.remove();
      editor.store..clear()..freeze();
    });

And that seems to do the trick.

I am not thrilled to have to do this, especially since it only seems to be of use in tests. It would be nice if stream subscriptions would be removed once the objects that created them go out of scope. Then again, the obvious case of a subscription factory would be foiled by such a strategy, so I suppose I can understand why it works this way. It makes for small pain in tests, but I can live with it.


Day #795

Dart Don't Help Much with CSS

‹prev | My Chain | next›

I am sticking with a theme after last night's fun with z-index in the ICE Code Editor. That theme is CSS, which is not always pleasant, but definitely necessary. What I am curious about tonight probably has very little to do directly with Dart, though I am curious to see if Dart can help in any way or at the very least that it does not get too much in the way of debugging.

The problem is with the main menu styling. In Chrome (on the left), there is an appropriate gap beneath the buttons and the main meny. In FireFox (on the right) there is no gap:


After a bit of element inspection, the problem becomes fairly apparent: the buttons are different sizes in the two browsers, making it seem like the menu is incorrectly placed:


The main menu button in Chrome (on the left) is 9 pixels high. The main menu button in FireFox (on the right) is 15 pixels high. Not coincidentally, the difference is about the same as the gap between buttons and menu in Chrome. So the source of the problem has been identified, but not the cause.

Further inspection reveals that the line-height appears to be the root cause. In Chrome, the calculated line height is “normal” (which seems to be the same as the upcased font-size). In FireFox, it calculates the value to be 15px. Quick Google searching reveals this to be a common problem with a typically weird solution: setting the body to a unit-less 1.2 value for line-height. In my case, that does not work—there is a more specific CSS selector that sets the line-height back to normal. So I have to settle for setting the CSS selector of button:

button {
  font-family: sans-serif;
  font-size: 10px;
  line-height: 12px;
  /* ... */
}

Happily, working with FireFox and Chrome (actually Dartium in this case) causes me no troubles. Since this is just CSS work, I can reload the page as much as I want locally to see changes.

That ends up fixing the problem, but… sigh. CSS. I realize that there is no hope of this working, but it would be so nice to set a CSS font property on an element in Dart and have Dart normalize the rest of the font behavior across browsers:

  get _hideCodeButton {
    return _hide_code_button = new Element.html('<button>Hide Code</button>')
      ..style.fontSize = '10px'
      ..onClick.listen((e)=> hideCode());
  }

Just for kicks, I do try that and, no, it does not work. The font-size is set, but nothing is done to normalize line-height. I appreciate working with styles in Dart because I can use awesome method cascades as I do above, I also love Dart's naming consistency with methods and properties always being camel-cased (with the first character lowercase). That said, it is a pain to have to recompile my Dart into JavaScript to test the changes in FireFox.

Dart fixes a lot of the web's ills, but sadly, it seems that we are still stuck with CSS.


Day #794

Z-Index is a Pain in Any Language

‹prev | My Chain | next›

I think that the Dart version of the ICE Code Editor has more or less reached the point that it could replace the one used in 3D Game Programming for Kids. This is, of course, the exact point that a project starts thinking about awesome feature creep, which we affectionately call 0.1.0.

Of the features and bugs on that list, the one that has bothered me the most is the lack of search and replace. This never even worked in the old JavaScript version, but it was only enough to annoy, not enough to push pass the myriad of other problems and address. Since the underlying ACE code editor supports search and replace (it even works fine on the project landing page), I must be doing something very dumb.

The dumb thing turns out to be that I never included the ext-searchbox.js code in either project. With it installed alongside the other ACE code, the ICE Code Editor now also boasts search and replace:



Or it would boast this support if the ICE Code Editor button controls were not getting in the way of the ACE search and replace box. This is more of a problem than it first appears. I cannot simply fiddle with the CSS z-index of the buttons and the search box. The buttons are in a <div> that is separate from the ACE container (again, the ACE code editor being part of the ICE Code Editor is ugly, but ICE was chosen before I was even aware of the existence of ACE, let alone opted to use it).

The ACE container has a z-index of 20. The search dialog has a z-index of 99 within that container. The problem is that the buttons have a z-index of 1000 in the same context as the ACE container. Any changes that I make to the z-index of the buttons only positions them relative to the ACE container, not anything within it like the search dialog. So if I give the buttons a z-index of 21, they still cover the search dialog with its z-index of 99 (withing the ACE container, z-index of 20).

So I am left with this problem: how do I make the search dialog cover the buttons?

I see no other option but to add the button toolbar directly to the ACE code editor <div> tag:

  _attachCodeToolbar() {
    var toolbar = new Element.html('<div class=ice-toolbar>');
    toolbar.style
      // styles
      ..zIndex = '89'; // below ACE's searchbox

    toolbar.children
      ..add(_updateButton)
      ..add(_hideCodeButton)
      ..add(_mainMenuButton);

    editor_el.children.add(toolbar);
  }

The problem with this approach is mostly sunk cost. I have spent a lot of time getting the show and hide code buttons working—especially with focus and delayed updates. I also have problems with the preview layer. If a “Show Code” toolbar is added to the preview layer, then I have to work very hard to add this toolbar back to the preview layer every time that it gets redrawn (which can be quite often). Last, but not least, I have several event handlers that ignore events inside of the ACE <div>, but now many of the menu events have been moved down inside there and I have my work cut out for me.

In the end, I opt for a hybrid approach to my problem. There really is no way to avoid adding the ICE toolbar buttons to the ACE <div>. So I simply deal with the event fallout, mostly by fixing tests that break. But I keep a toolbar above everything else—the “Show Code” toolbar, which only displays when the preview is displaying. That way, I can keep my sunk cost investment (at least for the time being) because it works and rely on the existing code to do the right thing with regards to which buttons should be visible at any given time.

It is not an ideal solution, but then again, I am not sure anything better is possible given my constraints. Bleh. Maybe something to follow up on another day.



Day #793

Doing Bad Things with Web Components

‹prev | My Chain | next›

Tonight, I continue to explore web components in the web-ui packages for Dart. I was able to get a proof of concept web component up and running last night, using the ICE Code Editor Dart code that I have been so heavily involved with recently.

As web components go, it was fairly useless—there was no data binding, composition, extension. Heck, in the end, I did not even use the <template>, making my web component little more than a cheap, albeit pretty, way to instantiate a full-screen IDE-lite version of ICE. Tonight, I would like to explore supplying data to the web component constructor.

Based on what I know about web components, which admittedly is not much at this point, my sense is that what I want is not a valid web component use-case. But that never stopped me before!

(and really, abusing any kind of technology is the best way to understand it)

What I want is to supply content to be consumed, but not immediately inserted in the web page DOM. I want to supply code for the ICE Code Editor to edit, not to be displayed right away. In other words, something like the following should create an instance of the code editor with the following already loaded into the editor:

  <ice-code-editor>
    <body></body>
    <script>
      console.log("Don't want to see this on page load");
      document.addEventListener('keydown', function(e) {
        console.log(e.keyCode);
      });
    </script>
  </ice-code-editor>

This should prove a challenge because (a) depending on how the <body> tags are interpreted, problems could ensue and (b) I do not want any of that JavaScript actually executed.

So the question becomes, can I slurp that into my web component before it is interpreted by the DOM and/or executed? The answer to the first challenge is that I am not even going to get a chance—compiling my web component chokes on those extra <body> tags:

➜  public git:(web-ui) ✗ dart --package-root=packages/ packages/web_ui/dwc.dart --out web_ui/ web_ui.html
warning web_ui.html:9:5: Unexpected start tag (body).
    <body></body>
    ^^^^^^
warning web_ui.html:9:11: Unexpected end tag (ice-code-editor). Missing end tag (body).
    <body></body>
          ^^^^^^^
warning web_ui.html:10:5: Unexpected start tag token (script) in the after body phase.
    <script>
    ^^^^^^^^

I may still have options there, but I want to focus on answering the second challenge first. Can I prevent the JavaScript from being executed? To find out, I temporarily remove the <body> tags from the <ice-code-editor> tag, leaving just the JavaScript that I want to consume, but not evaluate:

  <ice-code-editor>
    <script>
      console.log("Don't want to see this on page load");
      document.addEventListener('keydown', function(e) {
        console.log(e.keyCode);
      });
    </script>
  </ice-code-editor>

Now, in the web component class, I print the contents of the element as soon as possible. In the case of Dart's WebComponent, that seems to be the create() callback method:

  <element name="ice-code-editor" extends="div">
    <template>
      <!-- Placeholder for template. Live elements added to BODY. -->
    </template>
    <script type="application/dart">
      import 'package:ice_code_editor/ice.dart' as ICE;
      import 'package:web_ui/web_ui.dart';

      class IceCodeEditor extends WebComponent {
        Full ide_lite;
        IceCodeEditor() {
          ide_lite = new ICE.Full();
        }
        created() {
          print(innerHtml);
        }
      }
    </script>
  </element>

If I have any hope of removing the <script> tags before they are evaluated the web component will have to print before the <script> code does. Unfortunately, the <script> tag code is evaluated before the web component code. When I recompile the web component and reload, I see the following output in the JavaScript console:

Don't want to see this on page load web_ui.html:10

    <script>
      console.log("Don't want to see this on page load");
      document.addEventListener('keydown', function(e) {
        console.log(e.keyCode);
      });
    </script>

Ah well, that is not too surprising. It would have been nice to be able to insert raw HTML (including <body> and <script> tags) inside my web component element, but there are other approaches. If there is a silver lining to this experiment, I have a way to read that content with the innerHtml property of WebComponent, which works as early as the create() callback (it does not work in the constructor).

Armed with that bit of knowledge, I believe that including the content in an HTML comment might work:

  <ice-code-editor>
<!--
<body></body>
<script>
  console.log("Don't want to see this on page load");
  document.addEventListener('keydown', function(e) {
    console.log(e.keyCode);
  });
</script>
-->
  </ice-code-editor>

To see this in action, I switch from the full-screen version of ICE to just the core ICE (the editor + preview). I do not need the constructor anymore—all of the action takes place in the created() and inserted() callbacks. Once the web component is created, I set the ID and some styles and grab the content of <ice-code-editor>. Then, when the web component is inserted into the DOM, I create an instance of the ICE editor:

<element name="ice-code-editor" extends="div">
    <template>
      <!-- Placeholder for template. Live elements added to BODY. -->
    </template>
    <script type="application/dart">
      import 'package:ice_code_editor/ice.dart' as ICE;
      import 'package:web_ui/web_ui.dart';

      class IceCodeEditor extends WebComponent {
        ICE.Editor ice;
        String content;

        created() {
          id = 'ice';
          style
           ..width = '600px'
           ..height = '400px';

          content = innerHtml.
            replaceFirst(new RegExp(r'^\s*<!--\s*', multiLine: true), '').
            replaceFirst(new RegExp(r'\s*-->\s*$', multiLine: true), '');
        }

        inserted() {
          ice = new ICE.Editor('#ice');
          ice.content= content;
        }
      }
    </script>
  </element>

And that actually seems to do the trick. When I first load up the page, the code from the comment is in the editor:



And I can edit the code and see the preview change:



So it all seems to works. For good measure, I even try loading a semi-sophisticated Three.js code example and that too works:



To be clear, I still believe that I am heartily abusing web components here. I am making no use whatsoever of data binding or real templates. Even so, this is pretty cool. I still might prefer to be able to get this working without the HTML comments, but even with them, this seems a fairly promising approach.


Day #792

My First Web UI Component

‹prev | My Chain | next›

With my ICE Code Editor house more or less in order, I would like to take a day or two to explore web-ui in Dart. I have only glanced at a few of the article on it, which is enough make me curious, but not enough to really understand it.

I am interested in all that web-ui has to offer—the model-driven-views are of definite interest to me since they would seems a logical extension of the MVC application that serves as the basis for much of the narrative in Dart for Hipsters. But today, I am more curious about the component aspect of web-ui. Specifically, can I use them to build a custom HTML tag for ICE?

I will start with the Full Screen / IDE-lite version of ICE. It would be very cool indeed to have a custom x-ice-code-editor tag. Per the tool article, I need to start by installing the web-ui package from Dart Pub. This is Dart, so it's trivial. I add a web-ui entry in my pubspec.yaml:

name: ice_code_editor
version: 0.0.1
description: Code Editor + Preview
dependencies:
  web_ui: any
  js: ">=0.0.22 <0.0.23"
  crypto: ">=0.5.20 <0.5.21"
  unittest: ">=0.5.20 <0.5.21"

Then do a pub install:

➜  ice-code-editor git:(web-ui) ✗ pub install
Resolving dependencies...........
Downloading web_ui 0.4.12+2 from hosted...
Downloading logging 0.5.20 from hosted...
Downloading pathos 0.5.20 from hosted...
Downloading source_maps 0.5.20 from hosted...
Downloading csslib 0.4.7+6 from hosted...
Downloading args 0.5.20 from hosted...
Downloading analyzer_experimental 0.5.20 from hosted...
Dependencies installed!

I start my web-ui version of ICE in a web_ui.html file:

<html>
  <head>
    <script src="packages/browser/dart.js"></script>
    <script src="packages/browser/interop.js"></script>
  </head>
  <body>
  <x-ice-code-editor></x-ice-code-editor>
  <element name="x-ice-code-editor" extends="div">
    <template>
      {{ el }}
    </template>
    <script type="application/dart">
      import 'package:ice_code_editor/ice.dart' as ICE;
      import 'package:web_ui/web_ui.dart';

      class IceCodeEditor extends WebComponent {
        Full ide_lite;
        IceCodeEditor() {
          ide_lite = new ICE.Full();
        }
        get el => ide_lite.el;
      }
    </script>
  </element>
  </body>
</html>

I leave the dart.js and interop.js files to kick the dart engine and the js-interop stuff that ICE needs. The rest, I mostly copy from tutorials. I have a containing <element> tag that contains my component definition and sets the tag name to the desired <x-ice-code-editor>. I then add a template that inserts the element that is generated by the full-screen IDE class. I have the feeling that won't work as most values in the examples seem to be strings or other primitives. Still it is worth a shot. Then I create the element's constructor. The documentation suggests that the constructor can default to the camel-cased version of the tag, which I try.

I compile only to find:

➜  public git:(web-ui) ✗ dart --package-root=packages/ packages/web_ui/dwc.dart --out web_ui/ web_ui.html 
warning web_ui.html:1:1: Unexpected start tag (html). Expected DOCTYPE.
<html>
^^^^^^
warning web_ui.html:7:3: Implied constructor name for x-tags has changed to "XIceCodeEditor". You should rename your class or add a constructor="IceCodeEditor" attribute to the element declaration. Also custom tags are not required to start with "x-" if their name has at least one dash.
  <element name="x-ice-code-editor" extends="div">
  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Looks like I need a doctype declaration in there—that's easy enough. It also seems that the default constructor has changed to include a leading “X”—that also seems easy enough. I fix those:

<!DOCTYPE html>
<html>
  <head>
    <script src="packages/browser/dart.js"></script>
    <script src="packages/browser/interop.js"></script>
  </head>
  <body>
  <x-ice-code-editor></x-ice-code-editor>
  <element name="x-ice-code-editor" extends="div">
    <template>
      {{ el }}
    </template>
    <script type="application/dart">
      import 'package:ice_code_editor/ice.dart' as ICE;
      import 'package:web_ui/web_ui.dart';

      class XIceCodeEditor extends WebComponent {
        Full ide_lite;
        XIceCodeEditor() {
          ide_lite = new ICE.Full();
        }
        get el => ide_lite.el;
      }
    </script>
  </element>
  </body>
</html>

And with that, I have a working component. It doesn't do much just yet, but it is a component.

I will remove the {{ el }} from the template, leaving it completely blank. As feared, that is only for inserting string-like values and is auto-converted to a string “div” and inserted at the top of the editor:



So in the end, my web component is little more than the simple Dart that normally defines ICE. It is still of some value for a more HTML-y way of instantiating an editor. Still, the main value lies in further developing the component for smaller components that can be inserted into a web page. That's something for another day.


Day #791

Async and User Input Testing in Dart

‹prev | My Chain | next›

Newsflash: sometimes staring at code for hours is counter-productive.

I am fighting a focus bug in the ICE Code Editor. The problem is that I can reproduce the bug “In Real Life,” but cannot do so in a test. In real life, I make a code update in the editor, hide the code with the “Hide Code” button and wait for the preview frame to be updated. What should happen is that the preview frame has focus so that keyboard controls in games work. In real life, the containing body element has focus.

No matter what I tried last night (and a little the night before), I could not get my test to fail. Despite my best efforts, my tests always see the iframe focused.

At this point, I have a lot of print() statements scattered throughout the code in an attempt to see what things are being called and in what order. With fresh eyes and fresh brain, I finally compared the output of the failing real-life scenario:

[hideCode] focus undefined:1
[Editor#focus()] preview focus undefined:1
[udpatePreview] focus nop undefined:1
THREE.CanvasRenderer 52 Three.js:11928
[preview_frame] document.close() 

And the test scenario:

[content=] gonna focus undefined:1
[content=] done focus undefined:1
[udpatePreview] focus nop undefined:1
[preview_frame] document.close() preview_frame.html:24
1 TEXTAREA undefined:1
[content=] gonna focus undefined:1
[content=] done focus undefined:1
[hideCode] focus undefined:1
[Editor#focus()] preview focus undefined:1
[udpatePreview] focus nop undefined:1
[preview_frame] document.close() preview_frame.html:24
2 IFRAME 

Darn. It.

The test version is going through the content setter in the Editor class to performs its update whereas as the “In Real Life” version relies on text events to update the content. Well, that certainly explains the differences between the real-life bug and not being able to reproduce it in tests—especially since the content setter plays with keyboard focus (to ensure focus after opening a project).

The question then becomes how can I test this bug if I cannot do the usual (in this project) content setting in order to force preview updates? How can I trigger updates in the code editor like real text events? Especially since it is not possible to trigger keyboard events in Dart?

It may not be possible to trigger keyboard events in Dart, but what about TextEvents? There is an easy way to find out—I replace the editor.content setter with a TextEvent with the same data:

    group("hiding code after update", (){
      setUp((){
        editor.ice.focus();

        // editor.content = '<h1>Force Update</h1>';
        document.
          query('#ice').
          dispatchEvent(
            new TextEvent('textInput', data: '<h1>Force Update</h1>')
          );

        helpers.click('button', text: 'Hide Code')
        // forced delay until preview is updated here...
      });
      test("preview has focus", (){ /* ... */ });
    });

With that, I finally have my failing test:

ERROR: Focus hiding code after update preview has focus
  Focus hiding code after update preview has focus: Test setup failed: Expected: 'IFRAME'
    Actual: 'BODY'   Which: is different.
  Expected: IFRAME
    Actual: BODY

Yay!

Wow, do I love me a failing test. Because now my course of action is clear: make it pass.

Well, maybe not 100% clear because there is the obvious caveat that I need to make it pass without breaking anything else. And in this case, this is an extremely important caveat because the most obvious way to make this test pass would be to have the updatePreview() method tell the editor to focus itself immediately after it updates the preview. This would be a very bad thing to do since I just removed it to fix a separate bug. Having just slogged through async code, my brain is in no condition to play dueling test failures.

This is where a little know feature of Dart's unittest comes in very handy. If you change a single test definition from test() to solo_test(), then only that test is run. That is a huge convenience when you have scores of tests. In this case, I would like a quick way to verify that two tests pass—that I am able to fix the test in question while still not breaking the previous test.

It turns out that you can mark more than one test as a solo_test() and each will run. That might set off oxymoron alarms, but I don't care—it is absolutely brilliant for what I need in this case. With the two tests marked solo_test(), I have:

unittest-suite-wait-for-done
ERROR: Focus hiding code after update preview has focus
  Focus hiding code after update preview has focus: Test setup failed: Expected: 'IFRAME'
    Actual: 'BODY'   Which: is different.
  Expected: IFRAME

PASS: New Project Dialog after preview is rendered input field retains focus if nothing else happens

1 PASSED, 0 FAILED, 1 ERRORS 

If I restore a focus() call in updatePreview(), then I fix one, but break the other:

unittest-suite-wait-for-done
PASS: Focus hiding code after update preview has focus
FAIL: New Project Dialog after preview is rendered input field retains focus if nothing else happens
  Expected: InputElement:<input>
    Actual: TextAreaElement:<textarea>

1 PASSED, 1 FAILED, 0 ERRORS

The actual fix is rather anticlimatic after all of that effort. But the two tests in question make it painfully clear what I need to do. If the context decides when it is appropriate to hide the preview frame, then the calling context needs to be responsible. So, in the UI's hideCode() method that is called when the “Hide Code” button is clicked, I listen for a preview change event and then tell ICE to focus itself:

  void hideCode() {
    ice.hideCode();
    // hide code control UI elements here...

    ice.onPreviewChange.listen((e) {
      ice.focus();
    });
  }

With that, I have both my tests passing:

unittest-suite-wait-for-done
PASS: Focus hiding code after update preview has focus
PASS: New Project Dialog after preview is rendered input field retains focus if nothing else happens

All 2 tests passed.
unittest-suite-success

After moving the two solo_test() functions back to plain old test() functions, I have 116 passing tests and no failures.

Tonight was a big win for me. I have struggled with this focus feature for a while now. My original implementation missed two important edge cases that turned out to be difficult to understand. Difficult to see with all the async flying about, but very worthwhile to be able to test to prevent future regressions. The biggest win of all may be the TextEvent find. It may not work with 100% of my test cases that mimic user input, but it is a significant improvement for simulating user input over simply setting value attributes.


Day #790

Testing Async Hell: Dart Style

‹prev | My Chain | next›

Man, testing is hard. Actually testing is pretty easy—especially in Dart.

But man is testing async stuff is hard. Dart provides plenty of little tricks to make it easier. Async testing has first-class support in Dart, but just because it is there does not mean that it is easy to setup test cases well.

Last night, Santiago Arias (my #pairwithme partner) and I were able to isolate one problem in the ICE Code Editor. Last night's problem was that something was stealing focus from the new project input field after clicking on it. The baffling thing was that we had a test already in place that was still passing:

    test("new project input field has focus", (){
      helpers.click('button', text: '☰');
      helpers.click('li', text: 'New');

      expect(
        query('.ice-dialog input[type=text]'),
        equals(document.activeElement)
      );
    });

It was a pretty straight-forward test which had served nicely to drive the original implementation. Click the main menu button (the ☰ symbol), then select the “New” menu item. The expectation is that the text field in the resultant dialog has focus. And even though this was not the case in the actual application, this test passed.

Santiago and I eventually reproduced the failure in tests by waiting for the preview to have been updated, and then waiting a single clock tick after clicking the “New” menu item:

    group("after preview is rendered", (){
      setUp((){
        // Return future so that setup is only considered complete when preview
        // had changed
        var preview_ready = new Completer();
        editor.onPreviewChange.listen((e){
          preview_ready.complete();
        });
        return preview_ready.future;
      });

      test("input field retains focus if nothing else happens", (){
        helpers.click('button', text: '☰');
        helpers.click('li', text: 'New');

        Timer.run(expectAsync0((){
          print(document.activeElement.tagName);
          expect(
            document.activeElement,
            equals(query('.ice-dialog input[type=text]'))
          );
        }));
      });
    });

With that, the test would fail with:

FAIL: New Project Dialog after preview is rendered input field retains focus if nothing else happens
  Expected: InputElement:<input>
    Actual: TextAreaElement:<textarea>

This is exactly what the application bug was doing: after clicking the “New” menu item, the textarea in the code editor was active—not the input field in the dialog.

We then found that we could fix this breaking test (and the bug) by disabling the focus() call in the updatePreview() method of the editor:

updatePreview() {
      // ...
      // focus();

      // Trigger an onPreviewChange event:
      _previewChangeController.add(true);
  }

What is odd about this is that focus() call is happening before the onPreviewChange event that kicks off the test.

Anyhow, that is all well and good, but I added that now-commented-out focus() call for a reason. It is responsible for giving focus to the preview element if the code is updated and then hidden with the hide code button. The use case here is that the programmer makes a change and wants to see the code right away. Since this code could very well contain keyboard listeners, focus is vital. Hence the test:

    group("hiding code after update", (){
      setUp((){
        editor.ice.focus();
        editor.content = '<h1>Force Update</h1>';
        helpers.click('button', text: 'Hide Code');

        var preview_ready = new Completer();
        editor.onPreviewChange.listen((e){
          preview_ready.complete();
        });
        return preview_ready.future;
      });

      test("preview has focus", (){
        var el = document.query('iframe');

        expect(document.activeElement, el);
      });
    });

The problem is that I am unable to make this test fail, no matter what delays I add, this test continues to pass:

    group("hiding code after update", (){
      setUp((){
        editor.content = '<h1>Force Update</h1>';
        helpers.click('button', text: 'Hide Code');

        var preview_ready = new Completer();
        editor.onPreviewChange.listen((e){
          preview_ready.complete();
        });
        return preview_ready.future;
      });

      test("preview has focus", (){
        var wait = new Duration(milliseconds: 200);
        new Timer(wait, expectAsync0((){
          new Timer(wait, expectAsync0((){
            expect(document.activeElement.tagName, 'IFRAME');
          }));
        }));
      });

Bother.

Somehow, in the live application, the body of the main page is retaining focus (or it usurping focus at some point after the preview element tried to grab its rightful focus). But try as I might, I have not figured out the magical combination of async setup in my tests that can reproduce this. This is going to bug me all night, but is probably one of those situations in which sleep will reveal something obvious

Regardless, if I cannot reproduce this problem, then I cannot fix it—at least in a meaningful way that will stay fixed.


Day #789

Testing Async Menus in Dart

‹prev | My Chain | next›

I have been using the pre-beta version of the ICE Code Editor for all updates that I make as the first edition of 3D Game Programming for Kids nears. For the most part, it has worked brilliantly which obviously falls under the “win” category.

In the last couple of day, however, I have been annoyed to find that focus seems to be broken. When a dialog, like the new project dialog, is opened, its text field should have keyboard focus. Instead, the code editor retains focus:



What is worrisome here is that I have a test for this:

    test("new project input field has focus", (){
      helpers.click('button', text: '☰');
      helpers.click('li', text: 'New');

      expect(
        query('.ice-dialog input[type=text]'),
        equals(document.activeElement)
      );
    });

And the test still seems to be passing despite visual evidence to the contrary:

PASS: New Project Dialog new project input field has focus

I know what the problem is. I have a vague idea how to fix it. But I am concerned about testing.

The problem is that, when the “New” menu item is clicked, two things happen. First the new project dialog is shown. Second, the main menu is hidden. The problem is that, a few days ago, I added a feature that gave focus to the code editor whenever a menu or dialog was hidden:

_hideDialog() {
  queryAll('.ice-menu').forEach((e)=> e.remove());
  queryAll('.ice-dialog').forEach((e)=> e.remove());
  query('#ice').dispatchEvent(new UIEvent('focus'));
}

The first step here is to add another test. I think the current test still has value, so instead I add a new test that waits for a tiny bit before checking focus:

    test("input field retains focus if nothing else happens", (){
      helpers.click('button', text: '☰');
      helpers.click('li', text: 'New');

      var wait = new Duration(milliseconds: 100);
      new Timer(wait, expectAsync0((){
        expect(
          query('.ice-dialog input[type=text]'),
          equals(document.activeElement)
        );
      }));
    });

This is the same test except for the 100 millisecond wait. I use an expectAsync0() check to ensure that the test will not prematurely pass without that the callback being called. The callback simple checks the same expectation that the current test does. This time it fails:

FAIL: New Project Dialog input field retains focus if nothing else happens
  Expected: TextAreaElement:<textarea>
    Actual: InputElement:<input>

In the end, I decide to solve this in the MenuAction class:

class MenuItem {
  MenuAction action;
  MenuItem(this.action);
  Element get el {
    return new Element.html('<li>${action.name}</li>')
    ..onClick.listen((e)=> _hideMenu(focus:false))
      ..onClick.listen((e)=> action.open())
      ..onClick.listen((e)=> maybeFocus());
  }
}

Where the _maybeFocus() function checks for open dialogs:

_maybeFocus() {
  if (document.activeElement.tagName == 'INPUT') return;
  query('#ice').dispatchEvent(new UIEvent('focus'));
}

That seems to do the trick when I try it manually, but the test still fails. And if the test still fails, I think the feature is not quite baked.

Tonight's #pairwithme pair, Santiago Arias, helps me track down the continued failure to the update-preview, which also fires a focus event. We remove that, but again have a false positive—the test that drove it continues to pass even without the feature. Bah. Too many events floating around and the tests that cover them are proving insufficient.

Oh well, there is progress tonight. The focus feature seems to be working again and tonight's new test look solid as it was failing and we were able to make it pass. As for the current false positive, there is always tomorrow.


Day #788

Dart Breaking Change (But I Already Knew That)

‹prev | My Chain | next›

I received an email notification this afternoon that the ICE Code Editor build had failed. Since I had not committed anything today, this meant one of two things: there was a new version of Dart or I have to hunt down one of my project collaborators. Happily my project collaborators are safe.

It is pretty awesome how drone.io runs builds when there are new versions of Dart. Back in the day you were pretty much guaranteed that the build would break every time Dart was updated, but this happens less and less frequently now that the core language has stabilized. So what monumental shift occurred in the language today?

It seems that Dart is now a little more fussy about return values. I really like the hash rocket function return syntactic sugar in Dart. I like it so much that I will use it even when not returning a value:

  bool _frozen;
  /// Prevent further syncs to localStorage
  void freeze()=> _frozen = true;

It seems that Dart will no longer let me get away with that as the dartanalyzer now tells me:

[warning] The return type 'bool' is not a 'void', as defined by the method 'freeze' (ice-code-editor/lib/store.dart, line 139, col 19)

Bother.

As woes go, this is does not amount to much as I can simply use the curly brace function syntax:

  bool _frozen;
  /// Prevent further syncs to localStorage
  void freeze() { _frozen = true; }

Problem solved.

Since there is a new version of Dart out, I take this as an opportunity to verify that everything works fine—even the package dependencies. We recently uploaded ICE to the Dart Pub package repository. The pub tool strongly recommended that we lock the dependencies in the package's pubspec.yaml:

name: ice_code_editor
version: 0.0.1
description: Code Editor + Preview
authors:
- Chris Strom <chris@eeecomputes.com>
- Santiago Arias <santiago.ariassar@gmail.com>
- Srdjan Pejic
homepage: https://github.com/eee-c/ice-code-editor
dependencies:
  unittest: ">=0.5.13 <0.5.14"
  js: ">=0.0.22 <0.0.23"
  crypto: ">=0.5.13 <0.5.14"

To grab the latest versions of these dependencies, I remove all version constraints for dependencies:

name: ice_code_editor
# ...
dependencies:
  unittest: any
  js: any
  crypto: any

Then I pub update to grab the latest versions of these dependencies:

➜  ice-code-editor git:(master) pub update
Resolving dependencies........
Downloading browser 0.5.20 from hosted...
Downloading crypto 0.5.20 from hosted...
Downloading meta 0.5.20 from hosted...
Downloading unittest 0.5.20 from hosted...
Dependencies updated!

With that, I am ready to run my test suite:

➜  ice-code-editor git:(master) ✗ ./test/test_runner.sh 
Analyzing lib/ice.dart...
No issues found.
[16440:16440:0619/205123:528866804170:ERROR:zygote_host_impl_linux.cc(144)] Running without the SUID sandbox! See https://code.google.com/p/chromium/wiki/LinuxSUIDSandboxDevelopment for more information on developing with the sandbox on.

I should investigate that error someday, but today I am a little more concerned that my test suite never returns. It just hangs there.

So I try the main test file in the browser and find:

Exception: The null object does not have a method 'callSync'.

NoSuchMethodError : method not found: 'callSync'
Receiver: null
Arguments: [GrowableObjectArray len:0] dart:core-patch/object_patch.dart:19
Object.noSuchMethod dart:core-patch/object_patch.dart:19
_enterScope package:js/js.dart:749
_enterScopeIfNeeded package:js/js.dart:728
context package:js/js.dart:717
Editor._startJsAce package:ice_code_editor/editor.dart:218
Editor._startAce.<anonymous closure>

Aha! That rings a bell. Or two. Or 10.

The strong presence of the JavaScript package in this stack trace reminds my of an announcement on the dart mailing list about a change to Dart's JavaScript code. Not only did I see that announcement on the mailing list, but one of my #pairwithme pairs specifically told me about it as well. And yet I forgot anyway.

Good thing there is continuous integration to catch these things.

The announcement was to inform everyone in the community that the browser package's dart.js was undergoing significant change. Specifically, the maintainers removed a lot of code that was in there solely to support js-interop. Since most Dart code does not need to inter-operate with JavaScript, there is no sense in making pure Dart code pay js-interop code evaluation penalties.

For packages that do need js-interop, like ICE, there a simple solution. I need to add a second <script> tag to the HTML pages that hold ICE. This includes the testing context page:

  <script type="application/dart" src="ice_test.dart"></script>
  <script type='text/javascript'>
    // Custom code to notify the test runner when the build is complete...
  </script>
  <script src="packages/browser/dart.js"></script>
  <script src="packages/browser/interop.js"></script>

I make that change everywhere else in the project (example pages, other test contexts, etc.) and that solves my problem. I again have a test suite with 115 passing tests and no complaints from dartanalyzer.

But what to do about the pubspec.yaml? I do not think that I have a choice but to re-introduce the version constraints. If I try to pub lish to Dart Pub, the pub command will whine noisily that I should have version constraints:

...
* Your dependency on "crypto" should have a version constraint. For example:
  
  dependencies:
    crypto: ">=0.5.20 <0.5.21"
...

I am unsure that I understand why I am not including my pubspec.lock file in source code control or the pub package when I end up doing the same work (only less pretty) in my pubspec.yaml:

name: ice_code_editor
# ...
dependencies:
  unittest: ">=0.5.20 <0.5.21"
  js: ">=0.0.22 <0.0.23"
  crypto: ">=0.5.20 <0.5.21"

I do not see that I have a choice at the moment (other than to put up with pub nagging me). The documentation says that I should not include pubspec.lock for a library like ICE, so I add the new version constraints as above and call it a day.

In the end, I really could have saved myself some work had I acted immediately on the ample warning that I had about this change. That said, I am grateful to have drone.io around (not to mention a solid test suite) to catch these kinds of things.


Day #787

Don't Use a Regular Expression When a String Will Do

‹prev | My Chain | next›

It is deadline-day for 3D Game Programming for Kids, so today's post may be briefer than normal. But the gods of the chain who have been so very good to me require their offering and an offering they shall have.

Not coincidentally, the ICE Code Editor, which is used exclusively in the book is nearing a milestone deadline as well: the it-has-to-work-for-the-book deadline. Fortunately, most of the major issues have been cleared up. In fact, there are only two issues still outstanding: a project sharing features that one of my collaborators has nearly finished and a minor bug. Saaay… minor bug sounds perfect tonight!

In the ICE Code Editor, we work hard to ensure that none of the actions overwrites an existing projects. This is a new feature of the Dart version of ICE. I believe that it was possible to overwrite existing projects in the JavaScript version by creating a new project, renaming a project, making a copy, opening a shared project or looking funny at a project. Maybe not the last one.

The feature that prevents overwriting projects in the Dart version is not so much thanks to Dart as it is thanks to the ease of Dart's unit testing library. As with any tool, it is only as good as the person wielding it and I seem to have overlooked something.

Existing tests verify that copied projects get the copy number placed in parentheses:

    test("copied project includes copy number in parentheses", (){
      helpers.createProject('Project #1');

      helpers.click('button', text: '☰');
      helpers.click('li', text: 'Make a Copy');

      expect(
        query('.ice-dialog input[type=text]').value,
        equals("Project #1 (1)")
      );
     });

The problem facing me tonight is that projects ending with parentheses are not creating copies correctly. Or, in unit-test-ese:

    test("project name ending in parens", (){
      helpers.createProject('projectNamedForFunction()');

      helpers.click('button', text: '☰');
      helpers.click('li', text: 'Make a Copy');

      expect(
        query('.ice-dialog input[type=text]').value,
        equals("projectNamedForFunction() (1)")
      );
     });

Thankfully, that fails:

Exception: Bad state: No elements dart:core-patch/growable_array.dart:152
List.last dart:core-patch/growable_array.dart:152
Store.nextProjectNamed package:ice_code_editor/store.dart:119
CopyDialog._copiedProjectName package:ice_code_editor/full/copy_dialog.dart:30
CopyDialog.open package:ice_code_editor/full/copy_dialog.dart:12
MenuItem.el.<anonymous closure>

If that had not failed, then I would have been forced to determine what was different between my test case and the live application before I could get down to fixing the bug. As it is, I have another bit of good news: the bug appears to be on line 119 of the store.dart code file. In other words, I only have to solve this bug in one place for it to be solved anywhere that is trying to use this copy-number functionality.

I eventually track this down to the code that finds the list of all projects with the same base name:

    // ...
    RegExp copy_number_re = new RegExp(r"\s+\((\d+)\)$");
    var title = original_title.replaceFirst(copy_number_re, "");

    var same_base = values.where((p) {
      return new RegExp("^" + title + r"(?:\s+\(\d+\))?$").hasMatch(p['filename']);
    });
    // ...

The problem is that the title in there is projectNamedForFunction()The parentheses in there are interpreted as regular expression syntax instead of literal strings that should match. I could go through and replace parentheses with their escaped counterparts (e.g. '\\('), but then what about project names with square brackets or curly braces?

And then it hits me. Why am I using a regular expression to match literal strings when a simple literal string comparison will do? So instead, I determine the list of projects with the same base name by asking for all projects that start with the base:

    // ...
    RegExp copy_number_re = new RegExp(r"\s+\((\d+)\)$");
    var title = original_title.replaceFirst(copy_number_re, "");

    var same_base = values.where((p) {
      return p['filename'].startsWith(title);
    });
    // ...

And that does the trick:

PASS: Copy Dialog project name ending in parens

I have no idea why I would have used the regular expression in the first place, but my test suite confirms that the simpler string works. With that bug squashed, I can safely return to the last few fixes in 3D Game Programming for Kids. Wish me luck…


Day #786

Triggering Not-So-Custom UI Events in Dart

‹prev | My Chain | next›

I still have not decided if I want to keep last night's focus tests. I wrote a bunch of tests for the ICE Code Editor, but only ended up verifying that most everything worked already. Admittedly this was more of a noticeable problem in the old JavaScript version of ICE, but I had expected more trouble than I found.

I do not think it is so much Dart that made things better as being more consistent in the approach that I took towards buttons, menus and dialogs. Of course, Dart's static typing encourages consistency, but then again so does a strong #pairwithme partner and I have been blessed with many.

At any rate, I did leave off last night with a single failing focus test:

ERROR: Focus- editor has focus after closing a dialog
  Focus- editor has focus after closing a dialog: Test setup failed: Expected: TextAreaElement:<textarea>
       But: was BodyElement:<body>.

Unlike most of the other tests from yesterday, this was a “good” one in that it reproduced buggy behavior that I can see in the application.

My initial instinct for fixing the bug was to add an ice.focus() call in the hideDialog method:

_hideDialog() {
  queryAll('.ice-menu').forEach((e)=> e.remove());
  queryAll('.ice-dialog').forEach((e)=> e.remove());
  ice.focus();
}

If that was all that was necessary to fix it, I would have been done with it last night. The trouble is that the _hideDialog() method is not so much a method on the full-screen version of ICE as it is a helper function. In other words, it does not have access to an instance of ICE nor do the calling contexts always have access.

So to make this pass, I think that I have to have _hideDialog() trigger an event on the main DOM element that holds the editor. The constructor for the full-screen version of the editor creates an #ice <div>:

Full({enable_javascript_mode: true}) {
    el = new Element.html('<div id=ice>');
    document.body.nodes.add(el);

    ice = new Editor('#ice', enable_javascript_mode: enable_javascript_mode);
    store = new Store();
    // ...
  }

So sending a focus event to the #ice element seems the best course of action:

_hideDialog() {
  queryAll('.ice-menu').forEach((e)=> e.remove());
  queryAll('.ice-dialog').forEach((e)=> e.remove());
  query('#ice').focus();
}

A listener on the onFocus event stream should then do the trick:

      el.onFocus.listen((e)=> ice.focus());

It ought to work (at least in my mind), but it does not. The on-focus event is never fired. I could rail against the focus() method failing to generate a focus event, but that will get me no closer to solving my problem.

Instead of telling the element to focus, perhaps I can send a custom event? Or better yet, I try sending a focus event since that is really what I am trying to do:

_hideDialog() {
  queryAll('.ice-menu').forEach((e)=> e.remove());
  queryAll('.ice-dialog').forEach((e)=> e.remove());
  query('#ice').dispatchEvent(new UIEvent('focus'));
}

That is an uglyish constructor signature, but it does the trick. The event is received by the <div id=ice> element, which in turns tells the underlying ICE editor that it has focus. Yesterday's work, which turns out to not be a complete waste after all, ensures that the proper part of ICE—the code editor or the preview layer—gets focus. The bottom line is that I have a passing test:

PASS: Focus- editor has focus after closing a dialog
PASS: Focus- hiding code after update preview has focus

I do believe that closes another issues on the ICE Code Editor tracker, which is a fine stopping point for tonight.


Day #785

Driving Focus Tests

‹prev | My Chain | next›

I have enjoyed refactoring my ICE Code Editor tests over the last few days. I have an even better understanding of how to test in Dart and how to make those tests as readable and robust as possible. I am sorely tempted to continue mucking about with the test suite itself, but the most pressing issue that I need to answer tonight is an actual feature. More specifically, I need to figure out how to test when different pieces of ICE are visible.

One of the few remaining issues in the so-called 3D Game Programming for Kids milestone is giving ICE focus after dialogs and menus have closed. Since the milestone gets its name because it is blocking a release for the 3D Game Programming for Kids book, it seems worth making it a priority.

I need to start by adding a feature to the core Editor class, which is responsible for synchronizing the ACE code editor with a preview layer for the visualizations. Currently, there is no way to focus anything in the core editor, so I need to drive that feature.

I start with the setup for the new focus group in the editor_test.dart file:

  group("focus", (){
    var editor;

    setUp((){
      document.body.nodes.
        add(new Element.html('<div id=ice-${currentTestCase.id}>'));

      editor = new Editor(
        '#ice-${currentTestCase.id}',
        enable_javascript_mode: false
      );

      return editor.editorReady;
    });
    // Test will go here...
  });

Since this is a core editor test, I have to manually create a <div> to hold the editor. In the full-screen class, this is done automatically, but this is a lower level test, hence the greater manual effort. I add the editor to that element and return a future as I learned last night.

With that, I am ready to test. The first test simply verifies default state—that the code editor is visible:

    test('code is visibile by default', (){
      var el = document.
        query('#ice-${currentTestCase.id}').
        query('.ice-code-editor-editor');

      expect(el.style.visibility, isNot('hidden'));
    });

After some debate, we opted to hide the code editor with CSS visibility rather than via z-index hackery of setting the display to none. I am still not sold on this, but it does make this test easy. And, not unexpectedly, this test passes. In fact, all of the tests for the low-level editor pass. The focus seems to naturally go to the correct location. Well, that and it seems that I had inadvertently copied some focus code from the JavaScript version when I was first getting started on the Dart version:

  showCode() {
    _editor_el.style.visibility = 'visible';
    query('.ace_print-margin').style.visibility = 'visible';

    _ace.renderer.onResize();
    _ace.focus();
  }

(again ACE is the actual code editor piece of ICE, which is incorporated via #pairwithme js-interop)

Once I am done with the core editor, I work my way out to the Full screen IDE-lite class, Full. That actually works as well except in the case in which the code is edited, then hidden before the preview layer has a chance to update itself. Immediately after the code is hidden, the preview has focus, but the updated iframe is enough to lose focus.

I drive the test as usual:

  solo_group("Focus", (){
    var editor;

    setUp((){
      editor = new Full(enable_javascript_mode: false)
        ..store.storage_key = "ice-test-${currentTestCase.id}";

      var preview_ready = new Completer();
      editor.onPreviewChange.listen((e){
        if (preview_ready.isCompleted) return;
        preview_ready.complete();
      });
      return preview_ready.future;
    });
    // tests here
  });

But, instead of writing a test, I add another group with more async setup (I really love that newly discovered feature):

    group("hiding code after update", (){
      setUp((){
        editor.ice.focus();
        editor.content = '<h1>Force Update</h1>';
        helpers.click('button', text: 'Hide Code');

        var preview_ready = new Completer();
        editor.onPreviewChange.listen((e){
          preview_ready.complete();
        });
        return preview_ready.future;
      });

      test("preview has focus", (){
        var el = document.query('iframe');

        expect(document.activeElement, el);
      });
    });

Making that pass is a simple matter of adding a discrete focus() call.

In the end it is a lot of work to verify that most everything is working as desired. Still, it is worth the trouble to have the assurance of a stronger test suite.

I end the night by driving the feature that started this adventure

    test("editor has focus after closing a dialog", (){
      helpers.click('button', text: '☰');
      helpers.click('li', text: 'Make a Copy');
      helpers.hitEscape();

      var el = document.
        query('.ice-code-editor-editor').
        query('textarea.ace_text-input');

      expect(document.activeElement, el);
    });

That gets met the expected error:

ERROR: Focus editor has focus after closing a dialog
  Focus editor has focus after closing a dialog: Test setup failed: Expected: TextAreaElement:<textarea>
       But: was BodyElement:<body>.

That may end up being a little trickier than expected since the menu options lack direct access to ICE to manipulate focus. I think it best to leave that for tomorrow. I always appreciate being able to start work with a failing test. It makes it so easy to jump right in.


Day #784

Async Test Setup in Dart

‹prev | My Chain | next›

At this point, I think it's safe to say that I am huge Dart testing fan. I have hundreds of tests covering Dart for Hipsters (many of which are broken at the moment). I have another 100+ covering ICE Code Editor (which is being written to support 3D Game Programming for Kids, which in turn is blocking me from updating Dart for Hipsters). Bottom line: I love Dart tests.

I have gotten quite adept at writing them—regular, asynchronous, client-side, server-side. It's all good. But there is still lots that I do not know. Some stuff that I do not know is stuff I do not even know that I do not know—the stuff that will bite me when my current abstractions break down as I try new things. One thing that I know that I don't know is asynchronous test setup.

(I really enjoyed writing the above paragraph and I think it almost makes sense)

The unittest documentation for setUp(), concludes with:

The setupTest function can be asynchronous; in this case it must return a Future.

I do not really understand what it means for a setupTest to be asynchronous. I know from bitter experience that the tests within a group() sharing a setUp are asynchronous, but how can a setUp() itself be asynchronous and how does it help?

It so happens that many of the tests in the ICE Code Editor have a Future floating around. We use the editorReady future to indicate when all of the associated JavaScript has finished loading. To enable this to work in tests, we wrap the actual test in a function that will not be invoked until the expected asynchronous call from the future is made.

In one of the download tests, it looks something like:

    test("it downloads the source as a file", (){
      _test(_) {
        // actual test expectations go here
      }
      editor.editorReady.then(expectAsync1(_test));
    });

The expectAsync1 is built into Dart's unittest. It prevents the test from passing until it is called (with one argument). Instead of using it to test an asynchronous operation, I am using it to poll for the JavaScript libraries to finish loading before running an unrelated test. I do this a lot in ICE—in well over 50% of the tests I would guess.

But it now occurs to me that the editorReady is a Future. What if I move it up into the setUp() test to be returned?

    setUp((){
      editor = new Full(/* ... */);
      return editor.editorReady;
    });

Does that mean that I can get rid of my abuse of expectAsync? Indeed it does. I remove the expectAsyncs from both of the tests in the Download feature:

  group("Download", () {
    setUp((){
      editor = new Full(/* ... */);
      return editor.editorReady;
    });

    test("it downloads the source as a file", (){
      // Just test code now
    });

    test("closes the main menu", () {
      // Just test code now
    });
  });

And the tests still pass:

unittest-suite-wait-for-done
PASS: Download it downloads the source as a file
PASS: Download closes the main menu
All 2 tests passed.
unittest-suite-success 

If I comment out the return of the editor.editorReady, then both tests fail because pieces of the underlying editor are not ready (are null):

unittest-suite-wait-for-done

FAIL: Download it downloads the source as a file
  Caught The null object does not have a getter 'value'.
  
  NoSuchMethodError : method not found: 'value'
  Receiver: null
  Arguments: []
  ...
  
FAIL: Download closes the main menu
  Caught NoSuchMethodError : method not found: 'ace'
  Receiver: Instance of 'Proxy'
  Arguments: []
  ...
  
0 PASSED, 2 FAILED, 0 ERRORS
Exception: Exception: Some tests failed.

Holy cow. Without that expectAsync code, the intent of my tests are 100% clearer. I am amused by my own arrogance in thinking that I knew Dart testing. Clearly there is still much to learn. I find that a very exciting prospect.

But before finding any new stuff, I have roughly 50 tests that need to be cleansed of unnecessary expectAsync calls.


Day #783