Comments to PR #12032 (#12045)

packages/flutter_tools/lib/src/asset.dart

@@ -479,7 +479,7 @@ Map<_Asset, List<_Asset>> _parseAssets(
           packageMap,
           assetBase,
           asset,
-          packageName
+          packageName,
         );
         if (!baseAsset.assetFileExists) {
           printError('Error: unable to locate asset entry in pubspec.yaml: "$asset".');
@@ -502,37 +502,42 @@ _Asset _resolveAsset(
 ) {
   if (asset.startsWith('packages/') && !fs.isFileSync(fs.path.join(assetBase, asset))) {
     // The asset is referenced in the pubspec.yaml as
-    // 'packages/PACKAGE_NAME/PATH/TO/ASSET
+    // 'packages/PACKAGE_NAME/PATH/TO/ASSET .
     final _Asset packageAsset = _resolvePackageAsset(asset, packageMap);
     if (packageAsset != null)
       return packageAsset;
   }
 
   final String assetEntry = packageName != null
-    ? 'packages/$packageName/$asset' // Asset from, and declared in $packageName
-    : null; // Asset from the current application
+    ? 'packages/$packageName/$asset' // Asset from, and declared in $packageName.
+    : null; // Asset from the current application.
   return new _Asset(base: assetBase, assetEntry: assetEntry, relativePath: asset);
 }
 
 _Asset _resolvePackageAsset(String asset, PackageMap packageMap) {
   assert(asset.startsWith('packages/'));
-  String packageKey = asset.substring(9);
+  String packageKey = asset.substring('packages/'.length);
   String relativeAsset = asset;
 
   final int index = packageKey.indexOf('/');
   if (index != -1) {
     relativeAsset = packageKey.substring(index + 1);
     packageKey = packageKey.substring(0, index);
-  }
 
-  final Uri uri = packageMap.map[packageKey];
-  if (uri != null && uri.scheme == 'file') {
-    final File file = fs.file(uri);
-    final String base = file.path.substring(0, file.path.length - 1);
-    return new _Asset(base: base, assetEntry: asset, relativePath: relativeAsset);
+
+    final Uri uri = packageMap.map[packageKey];
+    if (uri != null && uri.scheme == 'file') {
+      final File file = fs.file(uri);
+      final String base = file.path.substring(0, file.path.length - 1);
+      return new _Asset(
+        base: base,
+        assetEntry: asset,
+        relativePath: relativeAsset,
+      );
+    }
   }
   printStatus('Error detected in pubspec.yaml:', emphasis: true);
-  printError('Could not resolve $packageKey for asset $asset.\n');
+  printError('Could not resolve package $packageKey for asset $asset.\n');
   return null;
 }
 

packages/flutter_tools/test/asset_bundle_package_test.dart

@@ -93,8 +93,8 @@ $assetsSection
     }
   }
 
-  group('AssetBundle assets from package', () {
-    testUsingContext('One package with no assets', () async {
+  group('AssetBundle assets from packages', () {
+    testUsingContext('No assets are bundled when the package has no assets', () async {
       establishFlutterRoot();
 
       writePubspecFile('pubspec.yaml', 'test');
@@ -104,11 +104,35 @@ $assetsSection
       final AssetBundle bundle = new AssetBundle();
       await bundle.build(manifestPath: 'pubspec.yaml');
       expect(bundle.entries.length, 2); // LICENSE, AssetManifest
-    }, overrides: <Type, Generator>{
-      FileSystem: () => new MemoryFileSystem(),
-    });
+      final String expectedAssetManifest = '{}';
+      expect(
+        UTF8.decode(await bundle.entries['AssetManifest.json'].contentsAsBytes()),
+        expectedAssetManifest,
+      );
+    }, overrides: contextOverrides);
 
-    testUsingContext('One package with one asset', () async {
+    testUsingContext('No assets are bundled when the package has an asset that is not listed', () async {
+      establishFlutterRoot();
+
+      writePubspecFile('pubspec.yaml', 'test');
+      writePackagesFile('test_package:p/p/lib/');
+      writePubspecFile('p/p/pubspec.yaml', 'test_package');
+
+      final List<String> assets = <String>['a/foo'];
+      writeAssets('p/p/', assets);
+
+      final AssetBundle bundle = new AssetBundle();
+      await bundle.build(manifestPath: 'pubspec.yaml');
+      expect(bundle.entries.length, 2); // LICENSE, AssetManifest
+      final String expectedAssetManifest = '{}';
+      expect(
+        UTF8.decode(await bundle.entries['AssetManifest.json'].contentsAsBytes()),
+        expectedAssetManifest,
+      );
+
+    }, overrides: contextOverrides);
+
+    testUsingContext('One asset is bundled when the package has and lists one asset its pubspec', () async {
       establishFlutterRoot();
 
       writePubspecFile('pubspec.yaml', 'test');
@@ -130,11 +154,9 @@ $assetsSection
         <String>['test_package'],
         expectedAssetManifest,
       );
-    }, overrides: <Type, Generator>{
-      FileSystem: () => new MemoryFileSystem(),
-    });
+    }, overrides: contextOverrides);
     
-    testUsingContext('One package with one asset not specified', () async {
+    testUsingContext("One asset is bundled when the package has one asset, listed in the app's pubspec", () async {
       establishFlutterRoot();
 
       final List<String> assetEntries = <String>['packages/test_package/a/foo'];
@@ -156,11 +178,9 @@ $assetsSection
         <String>['test_package'],
         expectedAssetManifest,
       );
-    }, overrides: <Type, Generator>{
-      FileSystem: () => new MemoryFileSystem(),
-    });
+    }, overrides: contextOverrides);
 
-    testUsingContext('One package with asset variants', () async {
+    testUsingContext("One asset and its variant are bundled when the package has an asset and a variant, and lists the asset in its pubspec", () async {
       establishFlutterRoot();
 
       writePubspecFile('pubspec.yaml', 'test');
@@ -182,11 +202,9 @@ $assetsSection
         <String>['test_package'],
         expectedManifest,
       );
-    }, overrides: <Type, Generator>{
-      FileSystem: () => new MemoryFileSystem(),
-    });
+    }, overrides: contextOverrides);
 
-    testUsingContext('One package with asset variants not specified', () async {
+    testUsingContext("One asset and its variant are bundled when the package has an asset and a variant, and the app lists the asset in its pubspec", () async {
       establishFlutterRoot();
 
       writePubspecFile(
@@ -211,11 +229,9 @@ $assetsSection
         <String>['test_package'],
         expectedManifest,
       );
-    }, overrides: <Type, Generator>{
-      FileSystem: () => new MemoryFileSystem(),
-    });
+    }, overrides: contextOverrides);
 
-    testUsingContext('One package with two assets', () async {
+    testUsingContext("Two assets are bundled when the package has and lists two assets in its pubspec", () async {
       establishFlutterRoot();
 
       writePubspecFile('pubspec.yaml', 'test');
@@ -238,11 +254,9 @@ $assetsSection
         <String>['test_package'],
         expectedAssetManifest,
       );
-    }, overrides: <Type, Generator>{
-      FileSystem: () => new MemoryFileSystem(),
-    });
+    }, overrides: contextOverrides);
 
-    testUsingContext('One package with two assets not specified', () async {
+    testUsingContext("Two assets are bundled when the package has two assets, listed in the app's pubspec", () async {
       establishFlutterRoot();
 
       final List<String> assetEntries = <String>[
@@ -272,11 +286,9 @@ $assetsSection
         <String>['test_package'],
         expectedAssetManifest,
       );
-    }, overrides: <Type, Generator>{
-      FileSystem: () => new MemoryFileSystem(),
-    });
+    }, overrides: contextOverrides);
 
-    testUsingContext('Two packages with assets', () async {
+    testUsingContext("Two assets are bundled when two packages each have and list an asset their pubspec", () async {
       establishFlutterRoot();
 
       writePubspecFile(
@@ -310,11 +322,9 @@ $assetsSection
         <String>['test_package', 'test_package2'],
         expectedAssetManifest,
       );
-    }, overrides: <Type, Generator>{
-      FileSystem: () => new MemoryFileSystem(),
-    });
+    }, overrides: contextOverrides);
     
-    testUsingContext('Two packages with assets not specified', () async {
+    testUsingContext("Two assets are bundled when two packages each have an asset, listed in the app's pubspec", () async {
       establishFlutterRoot();
 
       final List<String> assetEntries = <String>[
@@ -351,11 +361,9 @@ $assetsSection
         <String>['test_package', 'test_package2'],
         expectedAssetManifest,
       );
-    }, overrides: <Type, Generator>{
-      FileSystem: () => new MemoryFileSystem(),
-    });
+    }, overrides: contextOverrides);
 
-    testUsingContext('Transitive asset dependency', () async {
+    testUsingContext("One asset is bundled when the app depends on a package, listing in its pubspec an asset from another package", () async {
       establishFlutterRoot();
       writePubspecFile(
         'pubspec.yaml',
@@ -365,7 +373,7 @@ $assetsSection
       writePubspecFile(
         'p/p/pubspec.yaml',
         'test_package',
-        assets: <String>['packages/test_package2/a/foo']
+        assets: <String>['packages/test_package2/a/foo'],
       );
       writePubspecFile(
         'p2/p/pubspec.yaml',
@@ -384,8 +392,10 @@ $assetsSection
         <String>['test_package2'],
         expectedAssetManifest,
       );
-    }, overrides: <Type, Generator>{
-      FileSystem: () => new MemoryFileSystem(),
-    });
+    }, overrides: contextOverrides);
   });
 }
+
+Map<Type, Generator> get contextOverrides => <Type, Generator>{
+  FileSystem: () => new MemoryFileSystem()
+};