Fix bug in equalsIgnoreAsciiCase.

test/ignore_ascii_case_test.dart

+// Copyright (c) 2016, the Dart project authors.  Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+/// Tests case-ignoring compare and equality.
+
+import "package:collection/collection.dart";
+import "package:test/test.dart";
+
+main() {
+  testEquality();
+  testCompareUpper();
+  testCompareLower();
+}

[nweiz, 2016/11/10 21:44:44]

Please just include the tests inline in main(). The descriptions you pass to
test() should be clear enough to describe what the tests are doing.

[Lasse Reichstein Nielsen, 2016/11/11 07:56:51]

I originally had more tests in each, but I guess this separation doesn't make
sense any more. Done.

+
+void testEquality() {
+  test("equality", () {

[nweiz, 2016/11/10 21:44:44]

I'd find this test a lot easier to read with some more blank lines separating
logical sections.

[Lasse Reichstein Nielsen, 2016/11/11 07:56:51]

Done.

+    var strings = [];
+    upIf(String s, bool up) {
+      if (up) return s.toUpperCase();
+      return s;
+    }
+    for (int i = 0; i < 16; i++) {
+      var string =
+          upIf("a", i & 1 != 0) +
+          upIf("o", i & 2 != 0) +
+          upIf("p", i & 4 != 0) +
+          upIf("z", i & 8 != 0);
+      strings.add(string);
+    }

[nweiz, 2016/11/10 21:44:44]

Can you add a comment describing what's going on here? Alternately, could you
just explicitly include the strings your generating so someone reading the test
can just see what's being compared?

[Lasse Reichstein Nielsen, 2016/11/11 07:56:51]

Including the literal strings. It's only 16 of them anyway.

+    for (var s1 in strings) {
+      for (var s2 in strings) {
+        expect(equalsIgnoreAsciiCase(s1, s2), true);
+        expect(hashIgnoreAsciiCase(s1), hashIgnoreAsciiCase(s2));

[nweiz, 2016/11/10 21:44:44]

Any failures here are going to be really difficult to figure out. Add a reason
parameter describing what's being compared. Same goes for other expect()s.

[Lasse Reichstein Nielsen, 2016/11/11 07:56:51]

Oooh, there's a "reason" parameter. Good to know :) 

+      }
+    }
+    var lcletters = "@`abcdefghijklmnopqrstuvwxyz[{åÅ";
+    var ucletters = "@`ABCDEFGHIJKLMNOPQRSTUVWXYZ[{åÅ";

[nweiz, 2016/11/10 21:44:44]

"lowerCaseLetters" and "upperCaseLetters".

[Lasse Reichstein Nielsen, 2016/11/11 07:56:51]

Done.

+    expect(equalsIgnoreAsciiCase(lcletters, ucletters), true);
+    for (int i = 0; i < lcletters.length; i++) {
+      for (int j = 0; i < lcletters.length; i++) {
+        expect(equalsIgnoreAsciiCase(lcletters[i], lcletters[j]), i == j);
+        expect(equalsIgnoreAsciiCase(ucletters[i], lcletters[j]), i == j);
+        expect(equalsIgnoreAsciiCase(lcletters[i], ucletters[j]), i == j);
+        expect(equalsIgnoreAsciiCase(ucletters[i], ucletters[j]), i == j);

[nweiz, 2016/11/10 21:44:44]

Any failures here are going to be really difficult to figure out. Add a reason
parameter describing the letters being compared.

[Lasse Reichstein Nielsen, 2016/11/11 07:56:51]

Done.

+      }
+    }
+  });
+}
+
+void testCompareUpper() {

[nweiz, 2016/11/10 21:44:44]

Aren't these tests redundant with the comparison tests in comparators_test.dart?

[Lasse Reichstein Nielsen, 2016/11/11 07:56:51]

True, removed.

+  var orderedStrings = [
+    "@zzz",
+    "aopz@{",
+    "aopzZ{",
+    "aopzz{",  // z < [ due to upper-casing.
+    "Aopz[{",
+    "aoPz[{",
+    "aopz[{",
+    "AOPZ[{@",
+    "AOPZÅ",
+    "AOPZÕ",  // Õ is between Å and å.
+    "AOPZå",
+    "b@@@@",
+    "bAa@@",
+    "baa@@",
+    "ba`@@",
+  ];
+  test("compareAsciiUpperCase", () {
+    for (int i = 0; i < orderedStrings.length; i++) {
+      var s1 = orderedStrings[i];
+      for (int j = 0; j < orderedStrings.length; j++) {
+        var s2 = orderedStrings[j];
+        expect(compareAsciiUpperCase(s1, s2).sign, (i - j).sign);
+      }
+    }
+  });
+}
+
+void testCompareLower() {
+  var orderedStrings = [
+    "@zzz",
+    "aopz@{",
+    "Aopz[{",
+    "aoPz[{",
+    "aopz[{",
+    "AOPZ[{@",
+    "aopzZ{",
+    "aopzz{",  // z < [ due to upper-casing.
+    "AOPZÅ",
+    "AOPZÕ",  // Õ is between Å and å.
+    "AOPZå",
+    "b@@@@",
+    "ba`@@",
+    "bAa@@",
+    "baa@@",
+  ];
+  test("compareAsciiLowerCase", () {
+    for (int i = 0; i < orderedStrings.length; i++) {
+      var s1 = orderedStrings[i];
+      for (int j = 0; j < orderedStrings.length; j++) {
+        var s2 = orderedStrings[j];
+        expect(compareAsciiLowerCase(s1, s2).sign, (i - j).sign);
+      }
+    }
+  });
+}