Improve parsing logic to better align with what is supported by pub (dart-lang/pubspec_parse#9)
* Allow environment constraint values to be null
* Treat dependencies with empty map values like they are empty.
* Use an SDK constraint in tests that is compatible with the supported
SDK
* Update tests to verify behavior of pub client
* Failing test cases should cause pub to report a parse error
* Successful test cases should be parsed successfully by pub,
even if pub get ends up failing for another reasondiff --git a/pkgs/pubspec_parse/CHANGELOG.md b/pkgs/pubspec_parse/CHANGELOG.md
index c0c94c9..720f0e7 100644
--- a/pkgs/pubspec_parse/CHANGELOG.md
+++ b/pkgs/pubspec_parse/CHANGELOG.md
@@ -1,7 +1,7 @@
## 0.1.1
- Fixed name collision with error type in latest `package:json_annotation`.
-- Improved parsing of hosted dependencies.
+- Improved parsing of hosted dependencies and environment constraints.
## 0.1.0
diff --git a/pkgs/pubspec_parse/lib/src/dependency.dart b/pkgs/pubspec_parse/lib/src/dependency.dart
index 1498740..c4f7b26 100644
--- a/pkgs/pubspec_parse/lib/src/dependency.dart
+++ b/pkgs/pubspec_parse/lib/src/dependency.dart
@@ -62,12 +62,7 @@
}
Dependency _fromMap(Map data) {
- if (data.entries.isEmpty) {
-// TODO: provide list of supported keys?
- throw new CheckedFromJsonException(
- data, null, 'Dependency', 'Must provide at least one key.');
- }
-
+ assert(data.entries.isNotEmpty);
if (data.entries.length > 1) {
throw new CheckedFromJsonException(data, data.keys.skip(1).first as String,
'Dependency', 'Expected only one key.');
@@ -179,9 +174,12 @@
data = {'version': data};
}
- if (data is Map &&
- (data.containsKey('version') || data.containsKey('hosted'))) {
- return _$HostedDependencyFromJson(data);
+ if (data is Map) {
+ if (data.isEmpty ||
+ data.containsKey('version') ||
+ data.containsKey('hosted')) {
+ return _$HostedDependencyFromJson(data);
+ }
}
return null;
diff --git a/pkgs/pubspec_parse/lib/src/pubspec.dart b/pkgs/pubspec_parse/lib/src/pubspec.dart
index 7cc1ee1..a5cba62 100644
--- a/pkgs/pubspec_parse/lib/src/pubspec.dart
+++ b/pkgs/pubspec_parse/lib/src/pubspec.dart
@@ -124,8 +124,10 @@
source, 'dart', 'Use "sdk" to for Dart SDK constraints.');
}
- if (value is String) {
- VersionConstraint constraint;
+ VersionConstraint constraint;
+ if (value == null) {
+ constraint = null;
+ } else if (value is String) {
try {
constraint = new VersionConstraint.parse(value);
} on FormatException catch (e) {
@@ -133,8 +135,10 @@
}
return new MapEntry(key, constraint);
+ } else {
+ throw new CheckedFromJsonException(
+ source, key, 'VersionConstraint', '`$value` is not a String.');
}
- throw new CheckedFromJsonException(
- source, key, 'VersionConstraint', '`$value` is not a String.');
+ return new MapEntry(key, constraint);
});
diff --git a/pkgs/pubspec_parse/pubspec.yaml b/pkgs/pubspec_parse/pubspec.yaml
index 8d305ef..0bf1220 100644
--- a/pkgs/pubspec_parse/pubspec.yaml
+++ b/pkgs/pubspec_parse/pubspec.yaml
@@ -20,3 +20,5 @@
path: ^1.5.1
stack_trace: ^1.9.2
test: ^0.12.0
+ test_descriptor: ^1.0.3
+ test_process: ^1.0.2
diff --git a/pkgs/pubspec_parse/test/dependency_test.dart b/pkgs/pubspec_parse/test/dependency_test.dart
index f9fd369..658e132 100644
--- a/pkgs/pubspec_parse/test/dependency_test.dart
+++ b/pkgs/pubspec_parse/test/dependency_test.dart
@@ -29,13 +29,6 @@
^^^''');
});
- test('empty map', () {
- _expectThrows({}, r'''
-line 4, column 10: Must provide at least one key.
- "dep": {}
- ^^''');
- });
-
test('map with too many keys', () {
_expectThrows({'path': 'a', 'git': 'b'}, r'''
line 5, column 12: Expected only one key.
@@ -53,6 +46,12 @@
expect(dep.toString(), 'HostedDependency: any');
});
+ test('empty map', () {
+ var dep = _dependency<HostedDependency>({});
+ expect(dep.hosted, isNull);
+ expect(dep.toString(), 'HostedDependency: any');
+ });
+
test('string version', () {
var dep = _dependency<HostedDependency>('^1.0.0');
expect(dep.version.toString(), '^1.0.0');
diff --git a/pkgs/pubspec_parse/test/parse_test.dart b/pkgs/pubspec_parse/test/parse_test.dart
index 1d7fcd8..00a0de9 100644
--- a/pkgs/pubspec_parse/test/parse_test.dart
+++ b/pkgs/pubspec_parse/test/parse_test.dart
@@ -25,31 +25,42 @@
});
test('all fields set', () {
- var constraint = new Version.parse('1.2.3');
+ var version = new Version.parse('1.2.3');
+ var sdkConstraint = new VersionConstraint.parse('>=2.0.0-dev.54 <2.0.0');
var value = parse({
'name': 'sample',
- 'version': constraint.toString(),
+ 'version': version.toString(),
'author': 'name@example.com',
- 'environment': {'sdk': '1.2.3'},
+ 'environment': {'sdk': sdkConstraint.toString()},
'description': 'description',
'homepage': 'homepage',
'documentation': 'documentation'
});
expect(value.name, 'sample');
- expect(value.version, constraint);
+ expect(value.version, version);
expect(value.description, 'description');
expect(value.homepage, 'homepage');
// ignore: deprecated_member_use
expect(value.author, 'name@example.com');
expect(value.authors, ['name@example.com']);
expect(value.environment, hasLength(1));
- expect(value.environment, containsPair('sdk', constraint));
+ expect(value.environment, containsPair('sdk', sdkConstraint));
expect(value.documentation, 'documentation');
expect(value.dependencies, isEmpty);
expect(value.devDependencies, isEmpty);
expect(value.dependencyOverrides, isEmpty);
});
+ test('environment values can be null', () {
+ var value = parse({
+ 'name': 'sample',
+ 'environment': {'sdk': null}
+ });
+ expect(value.name, 'sample');
+ expect(value.environment, hasLength(1));
+ expect(value.environment, containsPair('sdk', isNull));
+ });
+
group('author, authors', () {
test('one author', () {
var value = parse({'name': 'sample', 'author': 'name@example.com'});
@@ -132,16 +143,6 @@
^^^^^^''');
});
- test('environment values cannot be null', () {
- expectParseThrows({
- 'name': 'sample',
- 'environment': {'sdk': null}
- }, r'''
-line 4, column 10: `null` is not a String.
- "sdk": null
- ^^^^^''');
- });
-
test('environment values cannot be int', () {
expectParseThrows({
'name': 'sample',
diff --git a/pkgs/pubspec_parse/test/pub_utils.dart b/pkgs/pubspec_parse/test/pub_utils.dart
new file mode 100644
index 0000000..984bb49
--- /dev/null
+++ b/pkgs/pubspec_parse/test/pub_utils.dart
@@ -0,0 +1,95 @@
+// Copyright (c) 2018, 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.
+
+import 'dart:async';
+import 'dart:io';
+
+import 'package:path/path.dart' as p;
+import 'package:test/test.dart';
+import 'package:test_descriptor/test_descriptor.dart' as d;
+import 'package:test_process/test_process.dart';
+
+Future<ProcResult> tryPub(String content) async {
+ await d.file('pubspec.yaml', content).create();
+
+ var proc = await TestProcess.start(_pubPath, ['get', '--offline'],
+ workingDirectory: d.sandbox);
+
+ var result = await ProcResult.fromTestProcess(proc);
+
+ printOnFailure([
+ '-----BEGIN pub output-----',
+ result.toString().trim(),
+ '-----END pub output-----',
+ ].join('\n'));
+
+ if (result.exitCode == 0) {
+ var lockContent =
+ new File(p.join(d.sandbox, 'pubspec.lock')).readAsStringSync();
+
+ printOnFailure([
+ '-----BEGIN pubspec.lock-----',
+ lockContent.trim(),
+ '-----END pubspec.lock-----',
+ ].join('\n'));
+ }
+
+ return result;
+}
+
+class ProcResult {
+ final int exitCode;
+ final List<ProcLine> lines;
+
+ bool get cleanParse => exitCode == 0 || exitCode == 66 || exitCode == 69;
+
+ ProcResult(this.exitCode, this.lines);
+
+ static Future<ProcResult> fromTestProcess(TestProcess proc) async {
+ var items = <ProcLine>[];
+
+ var values = await Future.wait([
+ proc.exitCode,
+ proc
+ .stdoutStream()
+ .forEach((line) => items.add(new ProcLine(false, line))),
+ proc
+ .stderrStream()
+ .forEach((line) => items.add(new ProcLine(true, line))),
+ ]);
+
+ return new ProcResult(values[0] as int, items);
+ }
+
+ @override
+ String toString() {
+ var buffer = new StringBuffer('Exit code: $exitCode');
+ for (var line in lines) {
+ buffer.write('\n$line');
+ }
+ return buffer.toString();
+ }
+}
+
+class ProcLine {
+ final bool isError;
+ final String line;
+
+ ProcLine(this.isError, this.line);
+
+ @override
+ String toString() => '${isError ? 'err' : 'out'} $line';
+}
+
+/// The path to the root directory of the SDK.
+final String _sdkDir = (() {
+ // The Dart executable is in "/path/to/sdk/bin/dart", so two levels up is
+ // "/path/to/sdk".
+ var aboveExecutable = p.dirname(p.dirname(Platform.resolvedExecutable));
+ assert(FileSystemEntity.isFileSync(p.join(aboveExecutable, 'version')));
+ return aboveExecutable;
+})();
+
+final String _pubPath =
+ p.join(_sdkDir, 'bin', Platform.isWindows ? 'pub.bat' : 'pub');
diff --git a/pkgs/pubspec_parse/test/test_utils.dart b/pkgs/pubspec_parse/test/test_utils.dart
index 65000af..7180292 100644
--- a/pkgs/pubspec_parse/test/test_utils.dart
+++ b/pkgs/pubspec_parse/test/test_utils.dart
@@ -2,6 +2,7 @@
// 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.
+import 'dart:cli';
import 'dart:convert';
import 'package:json_annotation/json_annotation.dart';
@@ -9,6 +10,8 @@
import 'package:stack_trace/stack_trace.dart';
import 'package:test/test.dart';
+import 'pub_utils.dart';
+
String _encodeJson(Object input) =>
const JsonEncoder.withIndent(' ').convert(input);
@@ -48,11 +51,29 @@
Pubspec parse(Object content, {bool quietOnError: false}) {
quietOnError ??= false;
+
+ var encoded = _encodeJson(content);
+
+ var pubResult = waitFor(tryPub(encoded));
+
try {
- return new Pubspec.parse(_encodeJson(content));
- } on ParsedYamlException catch (e) {
- if (!quietOnError) {
- _printDebugParsedYamlException(e);
+ var value = new Pubspec.parse(encoded);
+
+ addTearDown(() {
+ expect(pubResult.cleanParse, isTrue,
+ reason:
+ 'On success, parsing from the pub client should also succeed.');
+ });
+ return value;
+ } catch (e) {
+ addTearDown(() {
+ expect(pubResult.cleanParse, isFalse,
+ reason: 'On failure, parsing from the pub client should also fail.');
+ });
+ if (e is ParsedYamlException) {
+ if (!quietOnError) {
+ _printDebugParsedYamlException(e);
+ }
}
rethrow;
}
