Added tests for docs.dart

Added tests for docs.dart

Committed: https://code.google.com/p/dart/source/detail?r=20630

[Bob Nystrom, 2013-03-28 14:18:40]

https://codereview.chromium.org/13000005/diff/6001/tools/dom/docs/lib/docs.dart
File tools/dom/docs/lib/docs.dart (right):

https://codereview.chromium.org/13000005/diff/6001/tools/dom/docs/lib/docs.da...
tools/dom/docs/lib/docs.dart:66: Future<bool> _exportJsonToFile(Map
convertedJson, Path jsonPath) {
This method does both sync and async IO, which means it could both throw a
runtime exception or complete to one. We prefer having functions that return
futures to report all runtime errors through the future. The simple fix is to
wrap the sync IO in a future:

return Future.of(() {
  final jsonFile...
  ...
  return outputSream.done...
});

https://codereview.chromium.org/13000005/diff/6001/tools/dom/docs/test/docs_t...
File tools/dom/docs/test/docs_test.dart (right):

https://codereview.chromium.org/13000005/diff/6001/tools/dom/docs/test/docs_t...
tools/dom/docs/test/docs_test.dart:14: final Path test_json_path =
scriptDir.append('test.json').canonicalize();
Camel-case "testJsonPath" and others. This isn't python. :)

You can lose the type annotation here too.

https://codereview.chromium.org/13000005/diff/6001/tools/dom/docs/test/docs_t...
tools/dom/docs/test/docs_test.dart:30: }
You can make this a one-line if if you want.

https://codereview.chromium.org/13000005/diff/6001/tools/dom/docs/test/docs_t...
tools/dom/docs/test/docs_test.dart:35: .then((bool anyErrors) {
We still don't have an ideal style for code like this, but I would likely do:

expect(convert(lib_path, test_json_path)
    .then((bool anyErrors) {
  expect(anyErrors, isFalse);

The continued lines should be indented +4 (which means more than the later
function body).

https://codereview.chromium.org/13000005/diff/6001/tools/dom/docs/test/docs_t...
tools/dom/docs/test/docs_test.dart:38: testJson = new
File.fromPath(test_json_path);
Are these needed?

https://codereview.chromium.org/13000005/diff/6001/tools/dom/docs/test/docs_t...
tools/dom/docs/test/docs_test.dart:44: expect(testJson.readAsLinesSync(),
equals(oldJson.readAsLinesSync()));
Nit, but I would probably do readAsStringSync().

https://codereview.chromium.org/13000005/diff/6001/tools/dom/docs/test/docs_t...
tools/dom/docs/test/docs_test.dart:54: return false;
You don't use the completion value, so just remove this.

[Andrei Mouravski, 2013-03-28 17:01:14]

PTAL.

https://codereview.chromium.org/13000005/diff/6001/tools/dom/docs/lib/docs.dart
File tools/dom/docs/lib/docs.dart (right):

https://codereview.chromium.org/13000005/diff/6001/tools/dom/docs/lib/docs.da...
tools/dom/docs/lib/docs.dart:66: Future<bool> _exportJsonToFile(Map
convertedJson, Path jsonPath) {
On 2013/03/28 14:18:40, Bob Nystrom wrote:
> This method does both sync and async IO, which means it could both throw a
> runtime exception or complete to one. We prefer having functions that return
> futures to report all runtime errors through the future. The simple fix is to
> wrap the sync IO in a future:
> 
> return Future.of(() {
>   final jsonFile...
>   ...
>   return outputSream.done...
> });

Done.

https://codereview.chromium.org/13000005/diff/6001/tools/dom/docs/test/docs_t...
File tools/dom/docs/test/docs_test.dart (right):

https://codereview.chromium.org/13000005/diff/6001/tools/dom/docs/test/docs_t...
tools/dom/docs/test/docs_test.dart:14: final Path test_json_path =
scriptDir.append('test.json').canonicalize();
On 2013/03/28 14:18:40, Bob Nystrom wrote:
> Camel-case "testJsonPath" and others. This isn't python. :)
> 
> You can lose the type annotation here too.

Done.

https://codereview.chromium.org/13000005/diff/6001/tools/dom/docs/test/docs_t...
tools/dom/docs/test/docs_test.dart:30: }
On 2013/03/28 14:18:40, Bob Nystrom wrote:
> You can make this a one-line if if you want.

Done.

https://codereview.chromium.org/13000005/diff/6001/tools/dom/docs/test/docs_t...
tools/dom/docs/test/docs_test.dart:35: .then((bool anyErrors) {
Will defer because I'm not that concerned about this case, but I did file
www.dartbug.com/9486

On 2013/03/28 14:18:40, Bob Nystrom wrote:
> We still don't have an ideal style for code like this, but I would likely do:
> 
> expect(convert(lib_path, test_json_path)
>     .then((bool anyErrors) {
>   expect(anyErrors, isFalse);
> 
> The continued lines should be indented +4 (which means more than the later
> function body).

https://codereview.chromium.org/13000005/diff/6001/tools/dom/docs/test/docs_t...
tools/dom/docs/test/docs_test.dart:38: testJson = new
File.fromPath(test_json_path);
On 2013/03/28 14:18:40, Bob Nystrom wrote:
> Are these needed?

Done.

https://codereview.chromium.org/13000005/diff/6001/tools/dom/docs/test/docs_t...
tools/dom/docs/test/docs_test.dart:44: expect(testJson.readAsLinesSync(),
equals(oldJson.readAsLinesSync()));
On 2013/03/28 14:18:40, Bob Nystrom wrote:
> Nit, but I would probably do readAsStringSync().

Done.

https://codereview.chromium.org/13000005/diff/6001/tools/dom/docs/test/docs_t...
tools/dom/docs/test/docs_test.dart:54: return false;
On 2013/03/28 14:18:40, Bob Nystrom wrote:
> You don't use the completion value, so just remove this.

Done.

[Bob Nystrom, 2013-03-28 18:17:15]

LGTM.

[Andrei Mouravski, 2013-03-28 18:25:29]

Committed patchset #5 manually as r20630 (presubmit successful).