Clean up version solver tests a bit.

sdk/lib/_internal/pub/test/version_solver_test.dart

 // Copyright (c) 2012, 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.
 
 library pub_upgrade_test;
 
 import 'dart:async';
 
 import 'package:unittest/unittest.dart';
 
@@ skipping 871 matching lines @@
   testFn(description, () {
     var cache = new SystemCache('.');
     source1 = new MockSource('mock1');
     source2 = new MockSource('mock2');
     cache.register(source1);
     cache.register(source2);
     cache.sources.setDefault(source1.name);
 
     // Build the test package graph.
     var root;
-    packages.forEach((nameVersion, dependencies) {
-      var parsed = parseSource(nameVersion, (isDev, nameVersion, source) {
-        var parts = nameVersion.split(' ');
-        var name = parts[0];
-        var version = parts[1];
-
-        var package = mockPackage(name, version, dependencies);
-        if (name == 'myapp') {
-          // Don't add the root package to the server, so we can verify that Pub
-          // doesn't try to look up information about the local package on the
-          // remote server.
-          root = package;
-        } else {
-          (cache.sources[source] as MockSource).addPackage(name, package);
-        }
-      });
+    packages.forEach((description, dependencies) {
+      var id = parseSpec(description);
+      var package = mockPackage(id, dependencies);
+      if (id.name == 'myapp') {
+        // Don't add the root package to the server, so we can verify that Pub
+        // doesn't try to look up information about the local package on the
+        // remote server.
+        root = package;
+      } else {
+        (cache.sources[id.source] as MockSource).addPackage(
+            id.description, package);
+      }
     });
 
     // Clean up the expectation.
     if (result != null) {
       var newResult = {};
-      result.forEach((name, version) {
-        parseSource(name, (isDev, name, source) {
-          version = new Version.parse(version);
-          newResult[name] = new PackageId(name, source, version, name);
-        });
+      result.forEach((description, version) {
+        var id = parseSpec(description, version);
+        newResult[id.name] = id;
       });
       result = newResult;
     }
 
+    // Parse the lockfile.
     var realLockFile = new LockFile.empty();
     if (lockfile != null) {
       lockfile.forEach((name, version) {
         version = new Version.parse(version);
         realLockFile.packages[name] =
             new PackageId(name, source1.name, version, name);
       });
     }
 
     // Make a version number like the continuous build's version.
@@ skipping 256 matching lines @@
   Future<bool> get(PackageId id, String path) {
     throw new Exception('no');
   }
 
   void addPackage(String description, Package package) {
     _packages.putIfAbsent(description, () => new Map<Version, Package>());
     _packages[description][package.version] = package;
   }
 }
 
-Package mockPackage(String description, String version,
-                    Map dependencyStrings) {
+Package mockPackage(PackageId id, Map dependencyStrings) {
   var sdkConstraint = null;
 
   // Build the pubspec dependencies.
   var dependencies = <PackageDep>[];
   var devDependencies = <PackageDep>[];
 
-  dependencyStrings.forEach((name, constraint) {
-    parseSource(name, (isDev, name, source) {
-      var packageName = name.replaceFirst(new RegExp(r"-[^-]+$"), "");
-      constraint = new VersionConstraint.parse(constraint);
+  dependencyStrings.forEach((description, constraint) {
+    var isDev = description.startsWith("(dev) ");
+    if (isDev) {
+      description = description.substring("(dev) ".length);
+    }
 
-      if (name == 'sdk') {
-        sdkConstraint = constraint;
-        return;
-      }
+    var dep = parseSpec(description).withConstraint(
+        new VersionConstraint.parse(constraint));
 
-      var dep = new PackageDep(packageName, source, constraint, name);
+    if (dep.name == 'sdk') {
+      sdkConstraint = dep.constraint;
+      return;
+    }
 
-      if (isDev) {
-        devDependencies.add(dep);
-      } else {
-        dependencies.add(dep);
-      }
-    });
+    if (isDev) {
+      devDependencies.add(dep);
+    } else {
+      dependencies.add(dep);
+    }
   });
 
-  var name = description.replaceFirst(new RegExp(r"-[^-]+$"), "");
-  var pubspec = new Pubspec(
-      name, new Version.parse(version), dependencies, devDependencies,
-      new PubspecEnvironment(sdkConstraint), []);
+  var pubspec = new Pubspec(id.name, id.version, dependencies,
+      devDependencies, new PubspecEnvironment(sdkConstraint), []);
   return new Package.inMemory(pubspec);
 }
 
-void parseSource(String description,
-    callback(bool isDev, String name, String source)) {
-  var isDev = false;
+/// Creates a new [PackageId] parsed from [text], which looks something like
+/// this:
+///
+///   foo-xyz 1.0.0 from mock
+///
+/// The package name is "foo". A hyphenated suffix like "-xyz" here is part
+/// of the package description, but not its name, so the description here is
+/// "foo-xyz".
+///
+/// This is followed by an optional [Version]. If [version] is provided, then
+/// it is parsed to a [Version], and [text] should *not* also contain a
+/// version string.
+///
+/// The "from mock" optional suffix is the name of a source for the package.
+/// If omitted, it defaults to "mock1".
+PackageId parseSpec(String text, [String version]) {
+  var pattern = new RegExp(r"(([a-z]*)(-[a-z]+)?)( ([^ ]+))?( from (.*))?$");
+  var match = pattern.firstMatch(text);
+  assert(match != null);

[nweiz, 2013/11/14 01:16:14]

Throw an error message that includes [text]. If someone actually does add a spec
with a typo, it'll be really helpful to tell them which spec they did wrong.

[Bob Nystrom, 2013/11/21 18:19:57]

Done.

 
-  if (description.startsWith("(dev) ")) {
-    description = description.substring("(dev) ".length);
-    isDev = true;
+  var description = match[1];
+  var name = match[2];
+
+  var parsedVersion;
+  if (version != null) {
+    // Spec string shouldn't also contain a version.
+    assert(match[5] == null);

[nweiz, 2013/11/14 01:16:14]

See above.

[Bob Nystrom, 2013/11/21 18:19:57]

Done.

+    parsedVersion = new Version.parse(version);
+  } else {
+    if (match[5] != null) {
+      parsedVersion = new Version.parse(match[5]);
+    } else {
+      parsedVersion = Version.none;
+    }
   }
 
-  var name = description;
   var source = "mock1";
-  var match = new RegExp(r"(.*) from (.*)").firstMatch(description);
-  if (match != null) {
-    name = match[1];
-    source = match[2];
+  if (match[7] != null) {
+    source = match[7];
     if (source == "root") source = null;
   }
 
-  callback(isDev, name, source);
+  return new PackageId(name, source, parsedVersion, description);
 }