Restructure pkg/unittest and pkg/webdriver to follow the pub conventions.

Restructure pkg/unittest and pkg/webdriver to follow the pub conventions.
This means all imports of unittest in our test code had to change to include 'lib' in the path.
While doing that change I changed the library/imports to the new syntax in the affected files.
Committed: https://code.google.com/p/dart/source/detail?r=14443

[gram, 2012-10-30 23:27:01]

Bill, I added you because I touched tools/testing/dart. Please take a look at
that.

Siggy, if this is too much, please farm some out. Most of the files are just
library/import syntax changes, but pay attention to the stuff in pkg.

Thanks!

[Siggi Cherem (dart-lang), 2012-10-31 20:54:02]

https://codereview.chromium.org/11301046/diff/1/pkg/unittest/lib/basematcher....
File pkg/unittest/lib/basematcher.dart (right):

https://codereview.chromium.org/11301046/diff/1/pkg/unittest/lib/basematcher....
pkg/unittest/lib/basematcher.dart:17: part of matcher;
move part above the comment (the comment should be associated with the 'class
MatchState' below)

https://codereview.chromium.org/11301046/diff/1/pkg/unittest/lib/expect.dart
File pkg/unittest/lib/expect.dart (right):

https://codereview.chromium.org/11301046/diff/1/pkg/unittest/lib/expect.dart#...
pkg/unittest/lib/expect.dart:11: * such exceptions. The [asyncGuard] property is
a function that
asyncGuard => _wrapAsync?

https://codereview.chromium.org/11301046/diff/1/pkg/unittest/lib/expect.dart#...
pkg/unittest/lib/expect.dart:15: */
nits:
- move this comment to the publicly visible property so that it gets included in
this library's documentation.
- remove empty line after this block

https://codereview.chromium.org/11301046/diff/1/pkg/unittest/lib/expect.dart#...
pkg/unittest/lib/expect.dart:19: set wrapAsync(Function f) => _wrapAsync = f;
seems silly to have a private property that can be publicly set and not written,
maybe simply combine the two and make this a public field?

https://codereview.chromium.org/11301046/diff/1/pkg/unittest/lib/html_print.dart
File pkg/unittest/lib/html_print.dart (right):

https://codereview.chromium.org/11301046/diff/1/pkg/unittest/lib/html_print.d...
pkg/unittest/lib/html_print.dart:7: /** This file is sourced by both
dom_config.dart and html_config.dart. */
move this comment above the 'part' declaration. consider combining this into
html_config now that dom_config no longer exists.

https://codereview.chromium.org/11301046/diff/1/pkg/unittest/lib/matcher.dart
File pkg/unittest/lib/matcher.dart (right):

https://codereview.chromium.org/11301046/diff/1/pkg/unittest/lib/matcher.dart...
pkg/unittest/lib/matcher.dart:6: // without Dartdoc choking, this should be
moved to a separate package.
I'm not sure I understand this TODO - we want matcher to still be distributed
with unittest, so it should all be part of a single package

https://codereview.chromium.org/11301046/diff/1/pkg/unittest/lib/matcher.dart...
pkg/unittest/lib/matcher.dart:8: library matcher;
we should add some top-level dartdocs to this library.

https://codereview.chromium.org/11301046/diff/1/pkg/unittest/lib/matcher.dart...
pkg/unittest/lib/matcher.dart:10: part 'basematcher.dart';
I think we should create a src/ subdirectory and move all the parts there:

library matcher;

part 'src/core_matchers.dart';

...

We want the lib directory to just have what users will be importing from.

https://codereview.chromium.org/11301046/diff/1/pkg/unittest/lib/matcher.dart...
pkg/unittest/lib/matcher.dart:21: 
remove empty line

https://codereview.chromium.org/11301046/diff/1/pkg/unittest/lib/mock.dart
File pkg/unittest/lib/mock.dart (right):

https://codereview.chromium.org/11301046/diff/1/pkg/unittest/lib/mock.dart#ne...
pkg/unittest/lib/mock.dart:6: // without Dartdoc choking, this should be moved
to a separate package.
same question as before

https://codereview.chromium.org/11301046/diff/1/pkg/unittest/test/test_utils....
File pkg/unittest/test/test_utils.dart (right):

https://codereview.chromium.org/11301046/diff/1/pkg/unittest/test/test_utils....
pkg/unittest/test/test_utils.dart:5: // This is included in more than one
library so we don't have a 'part of'.
we should fix this, because it will break in the future when partof is
mandatory.
Possible solutions:
- use the same name on all libraries that includes it
- upgrade this into a library and import it, rather than using 'part'

https://codereview.chromium.org/11301046/diff/1/pkg/webdriver/pubspec.yaml
File pkg/webdriver/pubspec.yaml (right):

https://codereview.chromium.org/11301046/diff/1/pkg/webdriver/pubspec.yaml#ne...
pkg/webdriver/pubspec.yaml:2: description: >
remove > ?

add author, homepage, version, dependencies?

https://codereview.chromium.org/11301046/diff/1/tests/isolate/isolate_negativ...
File tests/isolate/isolate_negative_test.dart (right):

https://codereview.chromium.org/11301046/diff/1/tests/isolate/isolate_negativ...
tests/isolate/isolate_negative_test.dart:21: expect("Expected fail", isFail);  
// <=-------- Should fail here.
isFail? isFalse? 

or replace with unittest.fail()?

speaking of which, should unittest.fail move to expect.dart?

https://codereview.chromium.org/11301046/diff/1/tests/isolate/timer_test.dart
File tests/isolate/timer_test.dart (right):

https://codereview.chromium.org/11301046/diff/1/tests/isolate/timer_test.dart...
tests/isolate/timer_test.dart:8: import 'dart:isolate';
'dart:' imports should go first.

https://codereview.chromium.org/11301046/diff/1/tools/testing/dart/test_suite...
File tools/testing/dart/test_suite.dart (right):

https://codereview.chromium.org/11301046/diff/1/tools/testing/dart/test_suite...
tools/testing/dart/test_suite.dart:985: const
RegExp(r"^#(source|import|resource)\(", multiLine: true);
it is important to fix also the sourceOrImportRegExp and the domImportRegex,
otherwise we will end up wrapping the wrong code sometimes. 

domImportRegExp can be simplified now that dom and html.dart are gone
('dart:html' will be enough)

[ahe, 2012-11-01 17:23:53]

It would be awesome if you could split this CL in two: one for the bulk "update
tests to use new syntax" and one for  "handle library syntax and related test
framework work".

[gram, 2012-11-01 21:23:18]

Removing Bill as the test suite changes are now in a different CL.

https://codereview.chromium.org/11301046/diff/1/pkg/unittest/lib/basematcher....
File pkg/unittest/lib/basematcher.dart (right):

https://codereview.chromium.org/11301046/diff/1/pkg/unittest/lib/basematcher....
pkg/unittest/lib/basematcher.dart:17: part of matcher;
On 2012/10/31 20:54:02, Siggi Cherem (dart-lang) wrote:
> move part above the comment (the comment should be associated with the 'class
> MatchState' below)

Done.

https://codereview.chromium.org/11301046/diff/1/pkg/unittest/lib/expect.dart
File pkg/unittest/lib/expect.dart (right):

https://codereview.chromium.org/11301046/diff/1/pkg/unittest/lib/expect.dart#...
pkg/unittest/lib/expect.dart:11: * such exceptions. The [asyncGuard] property is
a function that
On 2012/10/31 20:54:02, Siggi Cherem (dart-lang) wrote:
> asyncGuard => _wrapAsync?

Done.

https://codereview.chromium.org/11301046/diff/1/pkg/unittest/lib/expect.dart#...
pkg/unittest/lib/expect.dart:15: */
On 2012/10/31 20:54:02, Siggi Cherem (dart-lang) wrote:
> nits:
> - move this comment to the publicly visible property so that it gets included
in
> this library's documentation.
> - remove empty line after this block

Done.

https://codereview.chromium.org/11301046/diff/1/pkg/unittest/lib/expect.dart#...
pkg/unittest/lib/expect.dart:19: set wrapAsync(Function f) => _wrapAsync = f;
On 2012/10/31 20:54:02, Siggi Cherem (dart-lang) wrote:
> seems silly to have a private property that can be publicly set and not
written,
> maybe simply combine the two and make this a public field?

Done.

https://codereview.chromium.org/11301046/diff/1/pkg/unittest/lib/html_print.dart
File pkg/unittest/lib/html_print.dart (right):

https://codereview.chromium.org/11301046/diff/1/pkg/unittest/lib/html_print.d...
pkg/unittest/lib/html_print.dart:7: /** This file is sourced by both
dom_config.dart and html_config.dart. */
On 2012/10/31 20:54:02, Siggi Cherem (dart-lang) wrote:
> move this comment above the 'part' declaration. consider combining this into
> html_config now that dom_config no longer exists.

Done.

https://codereview.chromium.org/11301046/diff/1/pkg/unittest/lib/matcher.dart
File pkg/unittest/lib/matcher.dart (right):

https://codereview.chromium.org/11301046/diff/1/pkg/unittest/lib/matcher.dart...
pkg/unittest/lib/matcher.dart:6: // without Dartdoc choking, this should be
moved to a separate package.
On 2012/10/31 20:54:02, Siggi Cherem (dart-lang) wrote:
> I'm not sure I understand this TODO - we want matcher to still be distributed
> with unittest, so it should all be part of a single package

Personally it seems to me it needn't be, and could be a whole separate package,
but we can decide that later; I removed the comment.

https://codereview.chromium.org/11301046/diff/1/pkg/unittest/lib/matcher.dart...
pkg/unittest/lib/matcher.dart:8: library matcher;
On 2012/10/31 20:54:02, Siggi Cherem (dart-lang) wrote:
> we should add some top-level dartdocs to this library.

Done.

https://codereview.chromium.org/11301046/diff/1/pkg/unittest/lib/matcher.dart...
pkg/unittest/lib/matcher.dart:10: part 'basematcher.dart';
On 2012/10/31 20:54:02, Siggi Cherem (dart-lang) wrote:
> I think we should create a src/ subdirectory and move all the parts there:
> 
> library matcher;
> 
> part 'src/core_matchers.dart';
> 
> ...
> 
> We want the lib directory to just have what users will be importing from.

Done.

https://codereview.chromium.org/11301046/diff/1/pkg/unittest/lib/matcher.dart...
pkg/unittest/lib/matcher.dart:21: 
On 2012/10/31 20:54:02, Siggi Cherem (dart-lang) wrote:
> remove empty line

Done.

https://codereview.chromium.org/11301046/diff/1/pkg/unittest/lib/mock.dart
File pkg/unittest/lib/mock.dart (right):

https://codereview.chromium.org/11301046/diff/1/pkg/unittest/lib/mock.dart#ne...
pkg/unittest/lib/mock.dart:6: // without Dartdoc choking, this should be moved
to a separate package.
On 2012/10/31 20:54:02, Siggi Cherem (dart-lang) wrote:
> same question as before

Done.

https://codereview.chromium.org/11301046/diff/1/pkg/unittest/test/test_utils....
File pkg/unittest/test/test_utils.dart (right):

https://codereview.chromium.org/11301046/diff/1/pkg/unittest/test/test_utils....
pkg/unittest/test/test_utils.dart:5: // This is included in more than one
library so we don't have a 'part of'.
On 2012/10/31 20:54:02, Siggi Cherem (dart-lang) wrote:
> we should fix this, because it will break in the future when partof is
> mandatory.
> Possible solutions:
> - use the same name on all libraries that includes it
> - upgrade this into a library and import it, rather than using 'part'
> 

Done.

https://codereview.chromium.org/11301046/diff/1/pkg/webdriver/pubspec.yaml
File pkg/webdriver/pubspec.yaml (right):

https://codereview.chromium.org/11301046/diff/1/pkg/webdriver/pubspec.yaml#ne...
pkg/webdriver/pubspec.yaml:2: description: >
On 2012/10/31 20:54:02, Siggi Cherem (dart-lang) wrote:
> remove > ?
> 
> add author, homepage, version, dependencies?

For now I have just copied the format used by the other yaml files in pkg. They
all have '>' and don't have those other fields.

https://codereview.chromium.org/11301046/diff/1/tests/isolate/isolate_negativ...
File tests/isolate/isolate_negative_test.dart (right):

https://codereview.chromium.org/11301046/diff/1/tests/isolate/isolate_negativ...
tests/isolate/isolate_negative_test.dart:21: expect("Expected fail", isFail);  
// <=-------- Should fail here.
On 2012/10/31 20:54:02, Siggi Cherem (dart-lang) wrote:
> isFail? isFalse? 
> 
> or replace with unittest.fail()?
> 
> speaking of which, should unittest.fail move to expect.dart?

Yes, this should be isFalse; fixed.

expect does not depend on unittest, so no, unittest.fail should not be part of
it.

https://codereview.chromium.org/11301046/diff/1/tests/isolate/timer_test.dart
File tests/isolate/timer_test.dart (right):

https://codereview.chromium.org/11301046/diff/1/tests/isolate/timer_test.dart...
tests/isolate/timer_test.dart:8: import 'dart:isolate';
On 2012/10/31 20:54:02, Siggi Cherem (dart-lang) wrote:
> 'dart:' imports should go first.

Done.

https://codereview.chromium.org/11301046/diff/1/tools/testing/dart/test_suite...
File tools/testing/dart/test_suite.dart (right):

https://codereview.chromium.org/11301046/diff/1/tools/testing/dart/test_suite...
tools/testing/dart/test_suite.dart:985: const
RegExp(r"^#(source|import|resource)\(", multiLine: true);
On 2012/10/31 20:54:02, Siggi Cherem (dart-lang) wrote:
> it is important to fix also the sourceOrImportRegExp and the domImportRegex,
> otherwise we will end up wrapping the wrong code sometimes. 


Done that. I added a regexp too for part of so we can distinguish between part
and part of.

> 
> domImportRegExp can be simplified now that dom and html.dart are gone
> ('dart:html' will be enough)

[Siggi Cherem (dart-lang), 2012-11-01 21:40:54]

lgtm

https://codereview.chromium.org/11301046/diff/1/tests/isolate/isolate_negativ...
File tests/isolate/isolate_negative_test.dart (right):

https://codereview.chromium.org/11301046/diff/1/tests/isolate/isolate_negativ...
tests/isolate/isolate_negative_test.dart:21: expect("Expected fail", isFail);  
// <=-------- Should fail here.
On 2012/11/01 21:23:19, gram wrote:
> On 2012/10/31 20:54:02, Siggi Cherem (dart-lang) wrote:
> > isFail? isFalse? 
> > 
> > or replace with unittest.fail()?
> > 
> > speaking of which, should unittest.fail move to expect.dart?
> 
> Yes, this should be isFalse; fixed.
> 
> expect does not depend on unittest, so no, unittest.fail should not be part of
> it.

what I meant to say is that [unittest.fail] is a generic fail function similar
to expect(false, true). So it seems like if it belongs in the same place as
expect. Unittest imports it and rexports it so it will have access to it
anyways.