Don't run tests against pinned barback versions in third_party.

Don't run tests against pinned barback versions in third_party.

These packages are kept around to ensure that pub continues working
with historic barback versions, but pub's test suite handles loading
and testing against them manually. Other test suites (pkgbuild in
particular) can safely ignore them.

TBR

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

[nweiz, 2014-05-27 23:05:22]

Committed patchset #1 manually as r36718 (presubmit successful).

[kustermann, 2014-05-27 23:28:16]

lgtm with comments.

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

https://codereview.chromium.org/306603002/diff/1/tools/testing/dart/test_suit...
tools/testing/dart/test_suite.dart:421: if (packageName.contains('-')) continue;
I'd prefer if you move this logic into the "isValid(packageName)" function above
and constrain it more, something like:

isValid(packageName) {
  return !['third_party', 'barback-0.13.0'].contains(packageName);
}

You're more familiar with the package name constraints: Is "-" disallowed in the
directory names for pub packages? If so, I'm also fine with "-", but I'd still
move it up to "isValid(...)" and extend the comment saying that directories
containing "-" are not valid pub packages.

[nweiz, 2014-05-27 23:56:21]

Follow-up CL: https://codereview.chromium.org/303693006

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

https://codereview.chromium.org/306603002/diff/1/tools/testing/dart/test_suit...
tools/testing/dart/test_suite.dart:421: if (packageName.contains('-')) continue;
On 2014/05/27 23:28:17, kustermann wrote:
> I'd prefer if you move this logic into the "isValid(packageName)" function
above
> and constrain it more, something like:
> 
> isValid(packageName) {
>   return !['third_party', 'barback-0.13.0'].contains(packageName);
> }

Done.

> You're more familiar with the package name constraints: Is "-" disallowed in
the
> directory names for pub packages? If so, I'm also fine with "-", but I'd still
> move it up to "isValid(...)" and extend the comment saying that directories
> containing "-" are not valid pub packages.

Package names can't contain "-"; they have to be valid Dart identifiers. I think
it's better to check for "-" because we're likely to add more pinned versions of
barback in the future.