From aef8ffa616979aa5aeb0d789f2a23695262ee9c3 Mon Sep 17 00:00:00 2001 From: Alex Khomchenko Date: Sat, 3 Feb 2018 20:04:17 +0100 Subject: [PATCH 1/2] added more verbose error description for upload size validator --- lib/src/package.dart | 4 +-- lib/src/validator/size.dart | 18 ++++++++++-- test/validator/size_test.dart | 54 ++++++++++++++++++++++++++++++++--- 3 files changed, 68 insertions(+), 8 deletions(-) diff --git a/lib/src/package.dart b/lib/src/package.dart index e059f90db..07379a750 100644 --- a/lib/src/package.dart +++ b/lib/src/package.dart @@ -109,7 +109,7 @@ class Package { } /// Returns whether or not this package is in a Git repo. - bool get _inGitRepo { + bool get inGitRepo { if (_inGitRepoCache != null) return _inGitRepoCache; if (dir == null || !git.isInstalled) { @@ -242,7 +242,7 @@ class Package { // path package, since re-parsing a path is very expensive relative to // string operations. Iterable files; - if (useGitIgnore && _inGitRepo) { + if (useGitIgnore && inGitRepo) { // List all files that aren't gitignored, including those not checked in // to Git. Use [beneath] as the working dir rather than passing it as a // parameter so that we list a submodule using its own git logic. diff --git a/lib/src/validator/size.dart b/lib/src/validator/size.dart index 7242efb3e..da0c6bbdf 100644 --- a/lib/src/validator/size.dart +++ b/lib/src/validator/size.dart @@ -6,6 +6,7 @@ import 'dart:async'; import 'dart:math' as math; import '../entrypoint.dart'; +import '../io.dart' as io; import '../validator.dart'; /// The maximum size of the package to upload (100 MB). @@ -21,8 +22,21 @@ class SizeValidator extends Validator { return packageSize.then((size) { if (size <= _MAX_SIZE) return; var sizeInMb = (size / math.pow(2, 20)).toStringAsPrecision(4); - errors.add("Your package is $sizeInMb MB. Hosted packages must be " - "smaller than 100 MB."); + // Current implementation of Package.listFiles skips hidden files + var ignoreExists = io.fileExists(entrypoint.root.path('.gitignore')); + + var error = new StringBuffer("Your package is $sizeInMb MB. Hosted " + "packages must be smaller than 100 MB."); + + if (ignoreExists && !entrypoint.root.inGitRepo) { + error.write(" Your .gitignore has no effect since your project " + "does not appear to be in version control."); + } else if (!ignoreExists && entrypoint.root.inGitRepo) { + error.write(" Consider adding a .gitignore to avoid including " + "temporary files."); + } + + errors.add(error.toString()); }); } } diff --git a/test/validator/size_test.dart b/test/validator/size_test.dart index 00d32e98a..f1d63d6e1 100644 --- a/test/validator/size_test.dart +++ b/test/validator/size_test.dart @@ -17,15 +17,61 @@ ValidatorCreator size(int size) { return (entrypoint) => new SizeValidator(entrypoint, new Future.value(size)); } +expectSizeValidationError(ValidatorCreator fn, Matcher matcher) { + expect(validatePackage(fn), completion(pairOf(contains(matcher), anything))); +} + main() { - setUp(d.validPackage.create); + test('considers a package valid if it is <= 100 MB', () async { + await d.validPackage.create(); - test('considers a package valid if it is <= 100 MB', () { expectNoValidationError(size(100)); expectNoValidationError(size(100 * math.pow(2, 20))); }); - test('considers a package invalid if it is more than 100 MB', () { - expectValidationError(size(100 * math.pow(2, 20) + 1)); + group('considers a package invalid if it is more than 100 MB', () { + test('package is not under source control and no .gitignore exists', + () async { + await d.validPackage.create(); + + expectSizeValidationError( + size(100 * math.pow(2, 20) + 1), + equals("Your package is 100.0 MB. Hosted packages must " + "be smaller than 100 MB.")); + }); + + test('package is not under source control and .gitignore exists', () async { + await d.validPackage.create(); + await d.dir(appPath, [d.file('.gitignore', 'ignored')]).create(); + + expectSizeValidationError( + size(100 * math.pow(2, 20) + 1), + allOf( + contains("Hosted packages must be smaller than 100 MB."), + contains("Your .gitignore has no effect since your project " + "does not appear to be in version control."))); + }); + + test('package is under source control and no .gitignore exists', () async { + await d.validPackage.create(); + await d.git(appPath).create(); + + expectSizeValidationError( + size(100 * math.pow(2, 20) + 1), + allOf( + contains("Hosted packages must be smaller than 100 MB."), + contains("Consider adding a .gitignore to avoid including " + "temporary files."))); + }); + + test('package is under source control and .gitignore exists', () async { + await d.validPackage.create(); + await d.git(appPath, [d.file('.gitignore', 'ignored')]).create(); + + expectSizeValidationError( + size(100 * math.pow(2, 20) + 1), + equals("Your package is 100.0 MB. Hosted packages must " + "be smaller than 100 MB.")); + }); }); } From dd53e6ecedef28b98b80f7e2f846c5dce625dd30 Mon Sep 17 00:00:00 2001 From: Natalie Weizenbaum Date: Tue, 6 Feb 2018 14:28:55 -0800 Subject: [PATCH 2/2] style changes --- lib/src/validator/size.dart | 4 ++-- test/validator/size_test.dart | 27 +++++++++++---------------- 2 files changed, 13 insertions(+), 18 deletions(-) diff --git a/lib/src/validator/size.dart b/lib/src/validator/size.dart index da0c6bbdf..dcc30f5c3 100644 --- a/lib/src/validator/size.dart +++ b/lib/src/validator/size.dart @@ -6,7 +6,7 @@ import 'dart:async'; import 'dart:math' as math; import '../entrypoint.dart'; -import '../io.dart' as io; +import '../io.dart'; import '../validator.dart'; /// The maximum size of the package to upload (100 MB). @@ -23,7 +23,7 @@ class SizeValidator extends Validator { if (size <= _MAX_SIZE) return; var sizeInMb = (size / math.pow(2, 20)).toStringAsPrecision(4); // Current implementation of Package.listFiles skips hidden files - var ignoreExists = io.fileExists(entrypoint.root.path('.gitignore')); + var ignoreExists = fileExists(entrypoint.root.path('.gitignore')); var error = new StringBuffer("Your package is $sizeInMb MB. Hosted " "packages must be smaller than 100 MB."); diff --git a/test/validator/size_test.dart b/test/validator/size_test.dart index f1d63d6e1..3802cd1f6 100644 --- a/test/validator/size_test.dart +++ b/test/validator/size_test.dart @@ -17,8 +17,9 @@ ValidatorCreator size(int size) { return (entrypoint) => new SizeValidator(entrypoint, new Future.value(size)); } -expectSizeValidationError(ValidatorCreator fn, Matcher matcher) { - expect(validatePackage(fn), completion(pairOf(contains(matcher), anything))); +expectSizeValidationError(Matcher matcher) { + expect(validatePackage(size(100 * math.pow(2, 20) + 1)), + completion(pairOf(contains(matcher), anything))); } main() { @@ -35,7 +36,6 @@ main() { await d.validPackage.create(); expectSizeValidationError( - size(100 * math.pow(2, 20) + 1), equals("Your package is 100.0 MB. Hosted packages must " "be smaller than 100 MB.")); }); @@ -44,24 +44,20 @@ main() { await d.validPackage.create(); await d.dir(appPath, [d.file('.gitignore', 'ignored')]).create(); - expectSizeValidationError( - size(100 * math.pow(2, 20) + 1), - allOf( - contains("Hosted packages must be smaller than 100 MB."), - contains("Your .gitignore has no effect since your project " - "does not appear to be in version control."))); + expectSizeValidationError(allOf( + contains("Hosted packages must be smaller than 100 MB."), + contains("Your .gitignore has no effect since your project " + "does not appear to be in version control."))); }); test('package is under source control and no .gitignore exists', () async { await d.validPackage.create(); await d.git(appPath).create(); - expectSizeValidationError( - size(100 * math.pow(2, 20) + 1), - allOf( - contains("Hosted packages must be smaller than 100 MB."), - contains("Consider adding a .gitignore to avoid including " - "temporary files."))); + expectSizeValidationError(allOf( + contains("Hosted packages must be smaller than 100 MB."), + contains("Consider adding a .gitignore to avoid including " + "temporary files."))); }); test('package is under source control and .gitignore exists', () async { @@ -69,7 +65,6 @@ main() { await d.git(appPath, [d.file('.gitignore', 'ignored')]).create(); expectSizeValidationError( - size(100 * math.pow(2, 20) + 1), equals("Your package is 100.0 MB. Hosted packages must " "be smaller than 100 MB.")); });