Make servePackages's contents argument saner.

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

Index: sdk/lib/_internal/pub/test/test_pub.dart
diff --git a/sdk/lib/_internal/pub/test/test_pub.dart b/sdk/lib/_internal/pub/test/test_pub.dart
index ff5d9f9db1aebe6e56d5e2059722ce075e36ac57..73200c7f71f10ffdd666efad4e467ca9afa2b862 100644
--- a/sdk/lib/_internal/pub/test/test_pub.dart
+++ b/sdk/lib/_internal/pub/test/test_pub.dart
@@ -238,11 +238,12 @@ d.DirectoryDescriptor _servedApiPackageDir;
 /// this test.
 d.DirectoryDescriptor _servedPackageDir;
 
-/// A map from package names to parsed pubspec maps for those packages.
+/// A map from package names to parsed pubspec maps and contents for those
+/// packages.
 ///
 /// This represents the packages currently being served by [servePackages], and
 /// is `null` if [servePackages] has not yet been called for this test.
-Map<String, List<Map>> _servedPackages;
+Map<String, List<Pair<Map, Iterable<d.Descriptor>>>> _servedPackages;
 
 /// Creates an HTTP server that replicates the structure of pub.dartlang.org.
 ///
@@ -253,15 +254,17 @@ Map<String, List<Map>> _servedPackages;
 /// set of packages that are being served. Previous packages will continue to be
 /// served. Otherwise, the previous packages will no longer be served.
 ///
-/// If [contents] is given, its contents are added to every served
-/// package.
+/// If [contents] is given, it should be a map from package names and versions
+/// (e.g. "foo-1.2.3") to their contents.

[Bob Nystrom, 2014/08/20 23:50:23]

Having the mention the package names and versions twice is verbose and
error-prone. A map of lists of pairs of maps to iterables is kind of hairy too.

I think it's time we made a little utility class here. What do you think of
something like:

servePackages([
  servedPackage("foo", "1.2.3", contents: [
    descriptor...
  ]),
  servedPackage("bar", "1.2.4")
]);

ServedPackage servedPackage(String name, String version, {Iterable<d.Descriptor>
contents}) { ... }

class ServedPackage {
  final String name;
  final Version version;
  final Iterable<d.Descriptor> contents
}

?

[nweiz, 2014/08/21 01:21:23]

Done.

 ///
 /// If [serveBarback] is true, the repo versions of barback and its dependencies
 /// will be served as well.
 void servePackages(List<Map> pubspecs, {bool replace: false,
-    Iterable<d.Descriptor> contents, bool serveBarback: false}) {
+    Map<String, Iterable<d.Descriptor>> contents, bool serveBarback: false}) {
+  if (contents == null) contents = {};
+
   if (_servedPackages == null || _servedPackageDir == null) {
-    _servedPackages = <String, List<Map>>{};
+    _servedPackages = <String, List<Pair<Map, Iterable<d.Descriptor>>>>{};
     _servedApiPackageDir = d.dir('packages', []);
     _servedPackageDir = d.dir('packages', []);
     serve([
@@ -284,7 +287,11 @@ void servePackages(List<Map> pubspecs, {bool replace: false,
         var name = pubspec['name'];
         var version = pubspec['version'];
         var versions = _servedPackages.putIfAbsent(name, () => []);
-        versions.add(pubspec);
+        var packageContents = contents['$name-$version'];
+        if (packageContents == null) {
+          packageContents = [d.libDir(name, '$name $version')];
+        }
+        versions.add(new Pair(pubspec, packageContents));
       }
 
       var repoPackages = new Set();
@@ -300,7 +307,12 @@ void servePackages(List<Map> pubspecs, {bool replace: false,
           // while testing.
           pubspec.remove('environment');
 
-          _servedPackages[name] = [pubspec];
+          _servedPackages[name] = [
+            new Pair(pubspec, [
+              new d.DirectoryDescriptor.fromFilesystem('lib',
+                  path.join(repoRoot, 'pkg', name, 'lib'))
+            ])
+          ];
           if (pubspec.containsKey('dependencies')) {
             pubspec['dependencies'].keys.forEach(_addPackage);
           }
@@ -316,10 +328,12 @@ void servePackages(List<Map> pubspecs, {bool replace: false,
           d.file('$name', JSON.encode({
             'name': name,
             'uploaders': ['nweiz@google.com'],
-            'versions': _servedPackages[name].map(packageVersionApiMap).toList()
+            'versions': _servedPackages[name]
+                .map((pair) => packageVersionApiMap(pair.first)).toList()
           })),
           d.dir(name, [
-            d.dir('versions', _servedPackages[name].map((pubspec) {
+            d.dir('versions', _servedPackages[name].map((pair) {
+              var pubspec = pair.first;
               return d.file(pubspec['version'], JSON.encode(
                   packageVersionApiMap(pubspec, full: true)));
             }))
@@ -327,7 +341,9 @@ void servePackages(List<Map> pubspecs, {bool replace: false,
         ]);
 
         _servedPackageDir.contents.add(d.dir(name, [
-          d.dir('versions', _servedPackages[name].map((pubspec) {
+          d.dir('versions', _servedPackages[name].map((pair) {
+            var pubspec = pair.first;
+            var packageContents = pair.last;
             var version = pubspec['version'];
 
             if (repoPackages.contains(name)) {
@@ -338,16 +354,9 @@ void servePackages(List<Map> pubspecs, {bool replace: false,
               ]);
             }
 
-            var archiveContents = [
-                d.file('pubspec.yaml', JSON.encode(pubspec)),
-                d.libDir(name, '$name $version')
-            ];
-
-            if (contents != null) {
-              archiveContents.addAll(contents);
-            }
-
-            return d.tar('$version.tar.gz', archiveContents);
+            return d.tar('$version.tar.gz', [
+                d.file('pubspec.yaml', JSON.encode(pubspec))
+            ]..addAll(packageContents));
           }))
         ]));
       }