Make servePackages's contents argument saner.

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

 // Copyright (c) 2013, 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.
 
 /// Test infrastructure for testing pub.
 ///
 /// Unlike typical unit tests, most pub tests are integration tests that stage
 /// some stuff on the file system, run pub, and then validate the results. This
 /// library provides an API to build tests like that.
 library test_pub;
@@ skipping 220 matching lines @@
 d.DirectoryDescriptor _servedApiPackageDir;
 
 /// The [d.DirectoryDescriptor] describing the server layout of `/packages` on
 /// the test server.
 ///
 /// This contains the tarballs for packages that are being served via
 /// [servePackages]. It's `null` if [servePackages] has not yet been called for
 /// 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.
 ///
 /// [pubspecs] is a list of unserialized pubspecs representing the packages to
 /// serve.
 ///
 /// If [replace] is false, subsequent calls to [servePackages] will add to the
 /// 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([
       d.dir('api', [_servedApiPackageDir]),
       _servedPackageDir
     ]);
 
     currentSchedule.onComplete.schedule(() {
       _servedPackages = null;
       _servedApiPackageDir = null;
       _servedPackageDir = null;
     }, 'cleaning up served packages');
   }
 
   schedule(() {
     return awaitObject(pubspecs).then((resolvedPubspecs) {
       if (replace) _servedPackages.clear();
 
       for (var pubspec in resolvedPubspecs) {
         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();
       if (serveBarback) {
         _addPackage(name) {
           if (_servedPackages.containsKey(name)) return;
           repoPackages.add(name);
 
           var pubspec = new Map.from(loadYaml(
               readTextFile(path.join(repoRoot, 'pkg', name, 'pubspec.yaml'))));
 
           // Remove any SDK constraints since we don't have a valid SDK version
           // 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);
           }
         }
 
         _addPackage('barback');
       }
 
       _servedApiPackageDir.contents.clear();
       _servedPackageDir.contents.clear();
       for (var name in _servedPackages.keys) {
         _servedApiPackageDir.contents.addAll([
           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)));
             }))
           ])
         ]);
 
         _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)) {
               return d.tar('$version.tar.gz', [
                 d.file('pubspec.yaml', JSON.encode(pubspec)),
                 new d.DirectoryDescriptor.fromFilesystem('lib',
                     path.join(repoRoot, 'pkg', name, 'lib'))
               ]);
             }
 
-            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));
           }))
         ]));
       }
     });
   }, 'initializing the package server');
 }
 
 /// Converts [value] into a YAML string.
 String yaml(value) => JSON.encode(value);
 
@@ skipping 712 matching lines @@
         _lastMatcher.matches(item.last, matchState);
   }
 
   Description describe(Description description) {
     return description.addAll("(", ", ", ")", [_firstMatcher, _lastMatcher]);
   }
 }
 
 /// A [StreamMatcher] that matches multiple lines of output.
 StreamMatcher emitsLines(String output) => inOrder(output.split("\n"));