VariationsService now supports wildcard in min/max version

base/version_unittest.cc

Index: base/version_unittest.cc
===================================================================
--- base/version_unittest.cc	(revision 142948)
+++ base/version_unittest.cc	(working copy)
@@ -89,3 +89,42 @@
     EXPECT_EQ(lhs->IsOlderThan(cases[i].rhs), (cases[i].expected == -1));
   }
 }
+
+TEST_F(VersionTest, CompareToWildcard) {
+  static const struct version_compare {
+    const char* lhs;
+    const char* rhs;
+    int expected;
+  } cases[] = {
+    {"1.0", "1.*", 0},
+    {"1.0", "0.*", 1},
+    {"1.0", "2.*", -1},
+    {"1.2.3", "1.2.3.*", 0},
+    {"10.0", "1.0.*", 1},
+    {"1.0", "3.0.*", -1},
+  };
+  for (size_t i = 0; i < ARRAYSIZE_UNSAFE(cases); ++i) {
+    Version version(cases[i].lhs);
+    EXPECT_EQ(version.CompareToWildcard(cases[i].rhs), cases[i].expected) <<

[Alexei Svitkine (slow), 2012/06/19 14:53:17]

Nit: Suggest extracting the result of version.CompareToWildcard(cases[i].rhs) to
a local var so that the EXPECT_EQ line can take up just one line.

[Mathieu, 2012/06/19 17:38:55]

Done.

+          cases[i].lhs << " <-> " << cases[i].rhs;
+  }
+}
+
+TEST_F(VersionTest, IsValidWildcard) {
+  static const struct version_compare {
+    const char* version;
+    bool expected;
+  } cases[] = {
+    {"1.0", true},

[Alexei Svitkine (slow), 2012/06/19 14:53:17]

Please add a test for empty string.

[Mathieu, 2012/06/19 17:38:55]

Done.

+    {"1.2.3.4.5.6", true},
+    {"1.2.3.*", true},
+    {"20.*", true},
+    {"+2.*", false},
+    {"*", false},
+    {"*.2", false},

[Alexei Svitkine (slow), 2012/06/19 14:53:17]

How about a case like: "1.2.3*"?

This should be invalid, because it's ambiguous (i.e. 1.2.35 vs. 1.2.3).

Please also expand the comment for Version::IsValidWildcard() to be very clear
about this. 

[Mathieu, 2012/06/19 17:38:55]

Done. Good point.

+  };
+  for (size_t i = 0; i < ARRAYSIZE_UNSAFE(cases); ++i) {
+    EXPECT_EQ(Version::IsValidWildcard(cases[i].version), cases[i].expected) <<

[Alexei Svitkine (slow), 2012/06/19 14:53:17]

Nit: Suggest extracting the result of Version::IsValidWildcard(cases[i].version)
to a local var so that the EXPECT_EQ line can take up just one line.

[Mathieu, 2012/06/19 17:38:55]

Done. Can't really make it fit...

+          cases[i].version << " failed -> " << cases[i].expected;
+  }
+}