[IndexedDB] Adding transaction and value support to observers

third_party/WebKit/LayoutTests/storage/indexeddb/resources/generic-idb-operations.js

 if (this.importScripts) {
     importScripts('../../../resources/testharness.js');
 }
 
 function compareChanges(actual, expected) {

[pwnall, 2017/01/10 01:41:11]

would it be fair to say this is assertChangesEqual?

[dmurph, 2017/01/10 20:40:02]

Done.

   assert_equals(actual.database.name, expected.dbName, 'The change record database should be the same as the database being acted on');
   var stores = Object.keys(expected.records);
   assert_equals(actual.records.size, stores.length, 'Incorrect number of objectStores recorded by observer');
 
+  var storesStr = Array.from(actual.records.keys()).join(", ");

[pwnall, 2017/01/10 01:41:11]

storeNamesString?

[dmurph, 2017/01/10 20:40:02]

Done.

   for (var i in stores) {
     var key = stores[i];
-    assert_true(actual.records.has(key));
+    assert_true(actual.records.has(key), "Store '" + key + "' not found in changes. Stores: " + storesStr);
     var actual_obsv = actual.records.get(key);

[pwnall, 2017/01/10 01:41:11]

camelCase instead of snake_case?

[dmurph, 2017/01/10 20:40:05]

Done.

     var expected_obsv = expected.records[key];
     assert_equals(actual_obsv.length, expected_obsv.length, 'Number of observations recorded for objectStore '+ key + ' should match observed operations');
     for (var j in expected_obsv)
       compareObservations(actual_obsv[j], expected_obsv[j]);
   }
 }
 
 function compareObservations(actual, expected) {
   assert_equals(actual.type, expected.type);
   assert_not_equals(actual.type, 'kDelete');

[pwnall, 2017/01/10 01:41:11]

Why is this here?

[dmurph, 2017/01/10 20:40:03]

Done.

   if (actual.type == 'clear') {

[pwnall, 2017/01/10 01:41:11]

I'd prefer === because it makes things slightly easier to reason about. In this
case it's pretty clear that no type coercion will happen, but it's still nice
not to have to think that through.

[dmurph, 2017/01/10 20:40:05]

Done.

     assert_equals(actual.key, undefined, 'clear operation has no key');
     assert_equals(actual.value, null, 'clear operation has no value');
     return;
   }
   if (actual.type == 'delete') {
     assert_equals(actual.key.lower, expected.key.lower, 'Observed operation key lower bound should match operation performed');
     assert_equals(actual.key.upper, expected.key.upper, 'Observed operation key upper bound should match operation performed');
     assert_equals(actual.key.lower_open, expected.key.lower_open, 'Observed operation key lower open should match operation performed');
     assert_equals(actual.key.upper_open, expected.key.upper_open, 'Observed operation key upper open should match operation performed');
     // TODO(dmurph): Value needs to be updated, once returned correctly. Issue crbug.com/609934.
     assert_equals(actual.value, null, 'Delete operation has no value');
     return;
   }
   assert_equals(actual.key.lower, expected.key, 'Observed operation key lower bound should match operation performed');

[pwnall, 2017/01/10 01:41:11]

There doesn't seem to be any checking for add vs put. Would it make sense to
have that here? I imagine that, at the very least, you could check that the
change type is 'add' or 'put'.

[dmurph, 2017/01/10 20:40:05]

That's basically done above, in the assert_equals(actual.type, expected.type);

That would fail if the operation is not what we expect it to be. Here we're just
special casing what we compare, as it changes based on the type.

   assert_equals(actual.key.upper, expected.key, 'Observed operation key upper bound should match operation performed');
-  // TODO(dmurph): Value needs to be updated, once returned correctly. Issue crbug.com/609934.
-  assert_equals(actual.value, null, 'Put/Add operation value does not match');
+  if (expected.value != undefined) {
+    assert_equals(actual.value, expected.value, 'Put/Add operation value does not match');
+  } else {
+    // TODO(dmurph): Value needs to be updated, once returned correctly. Issue crbug.com/609934.
+    assert_equals(actual.value, null, 'Put/Add operation has unexpected value');
+  }
 }
 
 function countCallbacks(actual, expected) {
   assert_equals(actual, expected, 'Number of callbacks fired for observer should match number of transactions it observed')
 }