Add more tests

Add more tests

Committed: 3572f68bd97b82e6934d776b4e6e98d75dbdb665

[Lasse Reichstein Nielsen, 2015-05-23 13:17:25]

lrn@google.com changed reviewers:
+ pquitslund@google.com, sethladd@google.com

[Lasse Reichstein Nielsen, 2015-05-26 07:14:37]

lrn@google.com changed reviewers:
+ sgjesse@google.com

[Lasse Reichstein Nielsen, 2015-05-26 07:15:00]

https://codereview.chromium.org/1152173005/diff/20001/lib/src/packages_impl.dart
File lib/src/packages_impl.dart (right):

https://codereview.chromium.org/1152173005/diff/20001/lib/src/packages_impl.d...
lib/src/packages_impl.dart:73: new Uri.directory(path.join(_packageDir.path,
packageName, ''));
Tests found bug! Yey!

[Søren Gjesse, 2015-05-26 08:11:53]

https://codereview.chromium.org/1152173005/diff/20001/test/discovery_test.dart
File test/discovery_test.dart (right):

https://codereview.chromium.org/1152173005/diff/20001/test/discovery_test.dar...
test/discovery_test.dart:30: void validatePackagesDir(Packages resolver, Uri
location, {bool enumerate: true}) {
Long line.

https://codereview.chromium.org/1152173005/diff/20001/test/discovery_test.dar...
test/discovery_test.dart:57: main() {
Can't you always make a fileTest and httpTest from the same description and
using common validation?

https://codereview.chromium.org/1152173005/diff/20001/test/discovery_test.dar...
test/discovery_test.dart:87: {"packages": { "foo": {}, "bar": {}, "baz": {}},
"script.dart": "main(){}"},
Long line.

https://codereview.chromium.org/1152173005/diff/20001/test/discovery_test.dar...
test/discovery_test.dart:122: resolver = await
findPackagesFromNonFile(location);
Not related to this CL. but what is a "NonFile" location?

https://codereview.chromium.org/1152173005/diff/20001/test/discovery_test.dar...
test/discovery_test.dart:128: httpTest(".packages not from subdir",
The file version of this test is called '.packages recursive'.

https://codereview.chromium.org/1152173005/diff/20001/test/discovery_test.dar...
test/discovery_test.dart:225: request.response.write(fileOrDir);
How does this work of fileOrDir refers to a subdir - a Map?

[Lasse Reichstein Nielsen, 2015-05-26 09:24:30]

https://codereview.chromium.org/1152173005/diff/20001/test/discovery_test.dart
File test/discovery_test.dart (right):

https://codereview.chromium.org/1152173005/diff/20001/test/discovery_test.dar...
test/discovery_test.dart:30: void validatePackagesDir(Packages resolver, Uri
location, {bool enumerate: true}) {
On 2015/05/26 08:11:53, Søren Gjesse wrote:
> Long line.

Done.

https://codereview.chromium.org/1152173005/diff/20001/test/discovery_test.dar...
test/discovery_test.dart:57: main() {
For the directly accessed .package files and packages dir, yes.
For a recursively found .packages file, no, that only works on file:.
Let's combine the ones that can be combined.

https://codereview.chromium.org/1152173005/diff/20001/test/discovery_test.dar...
test/discovery_test.dart:87: {"packages": { "foo": {}, "bar": {}, "baz": {}},
"script.dart": "main(){}"},
On 2015/05/26 08:11:53, Søren Gjesse wrote:
> Long line.

Done.

https://codereview.chromium.org/1152173005/diff/20001/test/discovery_test.dar...
test/discovery_test.dart:122: resolver = await
findPackagesFromNonFile(location);
It's any URI that doesn't start with "file:".

https://codereview.chromium.org/1152173005/diff/20001/test/discovery_test.dar...
test/discovery_test.dart:128: httpTest(".packages not from subdir",
So this one should be "not recursive". Fixed.

https://codereview.chromium.org/1152173005/diff/20001/test/discovery_test.dar...
test/discovery_test.dart:225: request.response.write(fileOrDir);
Should probably make it an explicit error. It won't happen in this test, I only
ask for files that are there or not there, but it should still be safe -
otherwise may still mask an error in the test.

[Lasse Reichstein Nielsen, 2015-05-26 09:26:58]

Committed patchset #4 (id:60001) manually as
3572f68bd97b82e6934d776b4e6e98d75dbdb665 (presubmit successful).

[Søren Gjesse, 2015-05-26 14:55:30]

lgtm