Port dart:io Path tests to package:path.

pkg/path/test/relative_test.dart

Index: pkg/path/test/relative_test.dart
diff --git a/pkg/path/test/relative_test.dart b/pkg/path/test/relative_test.dart
new file mode 100644
index 0000000000000000000000000000000000000000..d6582e9505c5f2f76e3ce44d6e57d15d61e1fd59
--- /dev/null
+++ b/pkg/path/test/relative_test.dart
@@ -0,0 +1,87 @@
+// 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 "relative" on all styles of path.Builder, on all platforms.
+
+import "package:unittest/unittest.dart";
+import "package:path/path.dart" as path;
+
+void main() {
+  test("test relative", testRelativeTo);
+}
+
+

[Bob Nystrom, 2013/07/18 20:45:08]

Nit: just a single blank line between functions.

+void testRelativeTo() {

[Bob Nystrom, 2013/07/18 20:45:08]

Instead of a named function, how about just inlining this as a lambda in the
call to test() above?

[Bill Hesse, 2013/07/22 11:35:22]

Done.

+  relativeTest(new path.Builder(style: path.Style.posix, root: '.'), '');
+  relativeTest(new path.Builder(style: path.Style.posix, root: '/'), '');
+  relativeTest(new path.Builder(style: path.Style.windows, root: 'd:/'), 'c:');
+  relativeTest(new path.Builder(style: path.Style.windows, root: '.'), 'c:');
+  relativeTest(new path.Builder(style: path.Style.url, root: 'file:///'),
+               'http://myserver');
+  relativeTest(new path.Builder(style: path.Style.url, root: '.'),
+              'http://myserver');
+  relativeTest(new path.Builder(style: path.Style.url, root: 'file:///'),
+               '');

[Bob Nystrom, 2013/07/18 20:45:08]

Nit: this can go on the previous line.

+  relativeTest(new path.Builder(style: path.Style.url, root: '.'),
+              '');

[Bob Nystrom, 2013/07/18 20:45:08]

This too.

+}
+
+
+void relativeTest(path.Builder builder, String absolutePrefix) {
+  var m = absolutePrefix;

[Bob Nystrom, 2013/07/18 20:45:08]

How about "prefix"? That should still keep the test strings short but isn't
meaningless like "m".

[Bill Hesse, 2013/07/22 11:35:22]

Done.

+  var isRelative = (builder.root == '.');
+  // Cases where the arguments are absolute paths.
+  void test(result, p, from) {

[Bob Nystrom, 2013/07/18 20:45:08]

Nit: ditch the "void".

+    expect(builder.normalize(result), builder.relative(p, from: from));
+  }
+
+  test('c/d', '$m/a/b/c/d', '$m/a/b');
+  test('c/d', '$m/a/b/c/d', '$m/a/b/');
+  test('.', '$m/a', '$m/a');
+  // Trailing slashes in the inputs have no effect.
+  test('../../z/x/y', '$m/a/b/z/x/y', '$m/a/b/c/d/');
+  test('../../z/x/y', '$m/a/b/z/x/y', '$m/a/b/c/d');
+  test('../../z/x/y', '$m/a/b/z/x/y/', '$m/a/b/c/d');
+  test('../../../z/x/y', '$m/z/x/y', '$m/a/b/c');
+  test('../../../z/x/y', '$m/z/x/y', '$m/a/b/c/');
+  // Cases where the arguments are relative paths.

[Bob Nystrom, 2013/07/18 20:45:08]

Use a ":" here or a "." on the comment below.

[Bill Hesse, 2013/07/22 11:35:22]

Done.

+  test('c/d', 'a/b/c/d', 'a/b');
+  test('.', 'a/b/c', 'a/b/c');
+  test('.', 'a/d/../b/c', 'a/b/c/');
+  test('.', '', '');
+  test('.', '.', '');
+  test('.', '', '.');
+  test('.', '.', '.');
+  test('.', '..', '..');
+  if (isRelative) test('..', '..', '.');
+  test('a', 'a', '');
+  test('a', 'a', '.');
+  test('..', '.', 'a');
+  test('.', 'a/b/f/../c', 'a/e/../b/c');
+  test('d', 'a/b/f/../c/d', 'a/e/../b/c');
+  test('..', 'a/b/f/../c', 'a/e/../b/c/e/');
+  test('../..', '', 'a/b/');
+  if (isRelative) test('../../..', '..', 'a/b/');
+  test('../b/c/d', 'b/c/d/', 'a/');
+  test('../a/b/c', 'x/y/a//b/./f/../c', 'x//y/z');
+
+  // Case where base is a substring of relative:
+  test('a/b', '$m/x/y//a/b', '$m/x/y/');
+  test('a/b', 'x/y//a/b', 'x/y/');
+  test('../ya/b', '$m/x/ya/b', '$m/x/y');
+  test('../ya/b', 'x/ya/b', 'x/y');
+  test('../b', 'x/y/../b', 'x/y/.');
+  test('a/b/c', 'x/y/a//b/./f/../c', 'x/y');
+  test('.', '$m/x/y//', '$m/x/y/');
+  test('.', '$m/x/y/', '$m/x/y');
+
+  // Should always throw - no relative path can be constructed.
+  if (isRelative) {
+    expect(() => builder.relative('.', from: '..'), throws);
+    expect(() => builder.relative('a/b', from: '../../d'), throws);
+    expect(() => builder.relative('a/b', from: '$m/a/b'), throws);

[Bob Nystrom, 2013/07/18 20:45:08]

I'm still not sure about these. My intuition is that, if the cwd is
"/path/to/dir", then I would expect:

builder.relative('.', from: '..') -> "dir"
builder.relative('a/b', from: '../../d') -> "../to/dir/a/b"
builder.relative('a/b', from: '$m/a/b') -> "../../path/to/dir/a/b"

Is there something I'm missing?

[Bill Hesse, 2013/07/22 11:35:22]

This is for the case where cwd is '.'.  When the user wants to work with paths
relative to an unknown base, and get results that are correct no matter what the
actual base is.  The isRelative guard runs the tests only when there is no
absolute cwd to resolve to.

[Bob Nystrom, 2013/07/22 20:53:11]

Ah, that makes more sense now. If there are cases where Builder.relative can
throw, we should document that clearly.

Also, change "throws" to "throwsArgumentError" here.

[Bill Hesse, 2013/07/23 16:07:00]

Done.

Re the documentation, I just noticed, in looking at .relative's doc
comments, that it says "If [from] is passed, [path] is made relative to that
instead."  This seems to imply that [root] is not used in that case, but the
implementation does prepend both with [root] if they are relative.  Should this
be changed?  The information about throwing ArgumentError can be added at the
same time.

[nweiz, 2013/07/23 19:31:06]

I think it's implicit that [path] may be relative to [from] using information
known about [root], but it wouldn't hurt to make that more explicit in the
documentation.

+    // An absolute path relative from a relative path returns the absolute path.
+    test('$m/a/b', '$m/a/b', 'c/d');
+  }
+}