From e5aead03ccde236aaf2e0944ac6c4ca29d749b2f Mon Sep 17 00:00:00 2001 From: Sarah Zakarias Date: Tue, 12 Sep 2017 12:53:03 +0200 Subject: [PATCH] Comments to PR #12032 (#12045) --- packages/flutter_tools/lib/src/asset.dart | 29 +++--- .../test/asset_bundle_package_test.dart | 94 ++++++++++--------- 2 files changed, 69 insertions(+), 54 deletions(-) diff --git a/packages/flutter_tools/lib/src/asset.dart b/packages/flutter_tools/lib/src/asset.dart index 62e9a8b02c0..7e23de4200e 100644 --- a/packages/flutter_tools/lib/src/asset.dart +++ b/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; } diff --git a/packages/flutter_tools/test/asset_bundle_package_test.dart b/packages/flutter_tools/test/asset_bundle_package_test.dart index f9a3b397fdc..ddebb1e3fde 100644 --- a/packages/flutter_tools/test/asset_bundle_package_test.dart +++ b/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: { - 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 assets = ['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 ['test_package'], expectedAssetManifest, ); - }, overrides: { - 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 assetEntries = ['packages/test_package/a/foo']; @@ -156,11 +178,9 @@ $assetsSection ['test_package'], expectedAssetManifest, ); - }, overrides: { - 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 ['test_package'], expectedManifest, ); - }, overrides: { - 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 ['test_package'], expectedManifest, ); - }, overrides: { - 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 ['test_package'], expectedAssetManifest, ); - }, overrides: { - 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 assetEntries = [ @@ -272,11 +286,9 @@ $assetsSection ['test_package'], expectedAssetManifest, ); - }, overrides: { - 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 ['test_package', 'test_package2'], expectedAssetManifest, ); - }, overrides: { - 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 assetEntries = [ @@ -351,11 +361,9 @@ $assetsSection ['test_package', 'test_package2'], expectedAssetManifest, ); - }, overrides: { - 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: ['packages/test_package2/a/foo'] + assets: ['packages/test_package2/a/foo'], ); writePubspecFile( 'p2/p/pubspec.yaml', @@ -384,8 +392,10 @@ $assetsSection ['test_package2'], expectedAssetManifest, ); - }, overrides: { - FileSystem: () => new MemoryFileSystem(), - }); + }, overrides: contextOverrides); }); } + +Map get contextOverrides => { + FileSystem: () => new MemoryFileSystem() +};