Refactor about:sync's events framework

chrome/browser/ui/webui/sync_internals_browsertest.js

Index: chrome/browser/ui/webui/sync_internals_browsertest.js
diff --git a/chrome/browser/ui/webui/sync_internals_browsertest.js b/chrome/browser/ui/webui/sync_internals_browsertest.js
index 733bac59692a9f403e9d0809775371cac4b0725f..fc24ef39bb68e76a99df85a0c4ab4fea154b5c78 100644
--- a/chrome/browser/ui/webui/sync_internals_browsertest.js
+++ b/chrome/browser/ui/webui/sync_internals_browsertest.js
@@ -18,6 +18,12 @@ SyncInternalsWebUITest.prototype = {
    */
   browsePreload: 'chrome://sync-internals',
 
+  /**
+   * Disable accessibility testing for this page.
+   * @override
+   */
+  runAccessibilityChecks: false,
+
   /** @override */
   preLoad: function() {
     this.makeAndRegisterMockHandler([
@@ -241,3 +247,49 @@ TEST_F('SyncInternalsWebUITest', 'NodeBrowserRefreshOnTabSelect', function() {
   $('sync-browser-tab').selected = true;
   expectEquals($('node-browser-refresh-time').textContent, 'TestCanary');
 });
+
+// Tests that the events log page correctly receives and displays an event.
+TEST_F('SyncInternalsWebUITest', 'EventLogTest', function() {
+  // Dispatch an event.
+  var connectionEvent = new Event('onConnectionStatusChange');
+  connectionEvent.details = { 'status': 'CONNECTION_OK' }

[Dan Beam, 2014/02/11 23:21:48]

nit: unfortunately (because it differs from C++), JS prefers:

  connectionEvent.details = {status: 'CONNECTION_OK'};

[rlarocque, 2014/02/12 00:39:55]

Done.

+  chrome.sync.events.dispatchEvent(connectionEvent);
+
+  // Verify that it is displayed in the events log.
+  var syncEventsTable = $('sync-events');
+  var firstRow = syncEventsTable.children[0];
+
+  // Makes some assumptions about column ordering.  We'll need re-think this if
+  // it turns out to be a maintenance burden.
+  assertLE(4, firstRow.children.length);

[Dan Beam, 2014/02/12 00:46:19]

shouldn't this be assertGE(), btw?

[rlarocque, 2014/02/12 01:00:46]

Let's just go with assertEquals().  If someone adds an extra column they'd
probably need to update this test anyway.

+  var submoduleName = firstRow.children[1].textContent;
+  var eventName = firstRow.children[2].textContent;
+  var detailsText = firstRow.children[3].textContent;
+
+  assertNotEquals(undefined, submoduleName, firstRow);

[Dan Beam, 2014/02/11 23:21:48]

nit: don't think you need to explicitly say undefined here, i.e.:

  assertNotEquals(submoduleName, firstRow);

and so on

[rlarocque, 2014/02/12 00:39:55]

Wouldn't that end up comparing submoduleName != firstRow?

[Dan Beam, 2014/02/12 00:46:19]

ah, I see.  these checks will never fail AFAIK as Element.textContent will
return '' in all cases if there's no text.  if you want to check something
perhaps something like this:

  assertGE(submoduleName.length, 0);

[rlarocque, 2014/02/12 01:00:46]

I think the idea here was to make sure that the DOM elements I'm looking for
actually exist before looking inside them.  It's a pattern I use often in C++
tests.  In this case, it's both unnecessary and wrongly implemented.

Maybe the test will be better off without these checks.  We'll just let it fail
on lines 273-278.

+  assertNotEquals(undefined, eventName, firstRow);
+  assertNotEquals(undefined, detailsText, firstRow);
+
+  expectTrue(submoduleName.indexOf('manager') != -1,
+             'submoduleName=' + submoduleName);

[Dan Beam, 2014/02/11 23:21:48]

nit:

expectGE(submoduleName.indexOf('manager'), 0);

[rlarocque, 2014/02/12 00:39:55]

Done.

+  expectTrue(eventName.indexOf('onConnectionStatusChange') != -1,
+             'eventName=' + eventName);
+  expectTrue(detailsText.indexOf('CONNECTION_OK') != -1,
+             'detailsText=' + detailsText);
+});
+
+TEST_F('SyncInternalsWebUITest', 'DumpSyncEventsToText', function() {
+  // Dispatch an event.
+  var connectionEvent = new Event('onConnectionStatusChange');
+  connectionEvent.details = { 'status': 'CONNECTION_OK' }
+  chrome.sync.events.dispatchEvent(connectionEvent);
+
+  // Click the dump-to-text button.
+  $('dump-to-text').click();
+
+  // Verify our event is among the results.
+  var eventDumpText = $('data-dump').textContent;
+
+  expectTrue(eventDumpText.indexOf('onConnectionStatusChange') != -1);
+  expectTrue(eventDumpText.indexOf('CONNECTION_OK') != -1);
+});