Skip to content

Add automatic SDK warning comparison between versions to dartdoc #1572

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 20 commits into from
Dec 20, 2017
Merged
Show file tree
Hide file tree
Changes from 18 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
157 changes: 157 additions & 0 deletions lib/src/io_utils.dart
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
/// This is a helper library to make working with io easier.
library dartdoc.io_utils;

import 'dart:async';
import 'dart:convert';
import 'dart:io';

import 'package:path/path.dart' as path;
Expand Down Expand Up @@ -60,3 +62,158 @@ String getFileNameFor(String name) =>
final libraryNameRegexp = new RegExp('[.:]');
final partOfRegexp = new RegExp('part of ');
final newLinePartOfRegexp = new RegExp('\npart of ');

final RegExp quotables = new RegExp(r'[ "\r\n\$]');

class SubprocessLauncher {
final String context;
Map<String, String> _environment;

Map<String, String> get environment => _environment;

String get prefix => context.isNotEmpty ? '$context: ' : '';

// from flutter:dev/tools/dartdoc.dart, modified
static void _printStream(Stream<List<int>> stream, Stdout output,
{String prefix: '', Iterable<String> Function(String line) filter}) {
assert(prefix != null);
if (filter == null) filter = (line) => [line];
stream
.transform(UTF8.decoder)
.transform(const LineSplitter())
.expand(filter)
.listen((String line) {
if (line != null) {
output.write('$prefix$line'.trim());
output.write('\n');
}
});
}

SubprocessLauncher(this.context, [Map<String, String> environment]) {
if (environment == null) this._environment = new Map();
}

/// A wrapper around start/await process.exitCode that will display the
/// output of the executable continuously and fail on non-zero exit codes.
/// It will also parse any valid JSON objects (one per line) it encounters
/// on stdout/stderr, and return them. Returns null if no JSON objects
/// were encountered.
///
/// Makes running programs in grinder similar to set -ex for bash, even on
/// Windows (though some of the bashisms will no longer make sense).
/// TODO(jcollins-g): move this to grinder?
Future<Iterable<Map>> runStreamed(String executable, List<String> arguments,
{String workingDirectory}) async {
List<Map> jsonObjects;

/// Allow us to pretend we didn't pass the JSON flag in to dartdoc by
/// printing what dartdoc would have printed without it, yet storing
/// json objects into [jsonObjects].
Iterable<String> jsonCallback(String line) {
Map result;
try {
result = json.decoder.convert(line);
} catch (FormatException) {}
if (result != null) {
if (jsonObjects == null) {
jsonObjects = new List();
}
jsonObjects.add(result);
if (result.containsKey('message')) {
line = result['message'];
} else if (result.containsKey('data')) {
line = result['data']['text'];
}
}
return line.split('\n');
}

stderr.write('$prefix+ ');
if (workingDirectory != null) stderr.write('(cd "$workingDirectory" && ');
if (environment != null) {
stderr.write(environment.keys.map((String key) {
if (environment[key].contains(quotables)) {
return "$key='${environment[key]}'";
} else {
return "$key=${environment[key]}";
}
}).join(' '));
stderr.write(' ');
}
stderr.write('$executable');
if (arguments.isNotEmpty) {
for (String arg in arguments) {
if (arg.contains(quotables)) {
stderr.write(" '$arg'");
} else {
stderr.write(" $arg");
}
}
}
if (workingDirectory != null) stderr.write(')');
stderr.write('\n');
Process process = await Process.start(executable, arguments,
workingDirectory: workingDirectory, environment: environment);

_printStream(process.stdout, stdout, prefix: prefix, filter: jsonCallback);
_printStream(process.stderr, stderr, prefix: prefix, filter: jsonCallback);
await process.exitCode;

int exitCode = await process.exitCode;
if (exitCode != 0) {
throw new ProcessException(executable, arguments,
"SubprocessLauncher got non-zero exitCode", exitCode);
}
return jsonObjects;
}
}

/// Output formatter for comparing warnings.
String printWarningDelta(String title, String dartdocOriginalBranch,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this public for testing?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's "public" so it can be imported by grinder. It's not public in another sense, that being it is under lib/src.

Map<String, int> original, Map<String, int> current) {
StringBuffer printBuffer = new StringBuffer();
Set<String> quantityChangedOuts = new Set();
Set<String> onlyOriginal = new Set();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you can calculate these using Set.difference()?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I could, but I would have to build more sets and then throw them away to do so. Map.keys is an iterable, not a set. Since I have to traverse by hand once anyway since "quantityChangedOuts" isn't something computable by difference, it seemed reasonable to do so for all three and save the throwaway Sets.

Set<String> onlyCurrent = new Set();
Set<String> allKeys =
new Set.from([]..addAll(original.keys)..addAll(current.keys));

for (String key in allKeys) {
if (original.containsKey(key) && !current.containsKey(key)) {
onlyOriginal.add(key);
} else if (!original.containsKey(key) && current.containsKey(key)) {
onlyCurrent.add(key);
} else if (original.containsKey(key) &&
current.containsKey(key) &&
original[key] != current[key]) {
quantityChangedOuts.add(key);
}
}

if (onlyOriginal.isNotEmpty) {
printBuffer.writeln(
'*** $title : ${onlyOriginal.length} warnings from original ($dartdocOriginalBranch) missing in current:');
onlyOriginal.forEach((warning) => printBuffer.writeln(warning));
}
if (onlyCurrent.isNotEmpty) {
printBuffer.writeln(
'*** $title : ${onlyCurrent.length} new warnings not in original ($dartdocOriginalBranch)');
onlyCurrent.forEach((warning) => printBuffer.writeln(warning));
}
if (quantityChangedOuts.isNotEmpty) {
printBuffer.writeln('*** $title : Identical warning quantity changed');
for (String key in quantityChangedOuts) {
printBuffer.writeln(
"* Appeared ${original[key]} times in original ($dartdocOriginalBranch), now ${current[key]}:");
printBuffer.writeln(key);
}
}
if (onlyOriginal.isEmpty &&
onlyCurrent.isEmpty &&
quantityChangedOuts.isEmpty) {
printBuffer.writeln(
'*** $title : No difference in warning output from original ($dartdocOriginalBranch)${allKeys.isEmpty ? "" : " (${allKeys.length} warnings found)"}');
}
return printBuffer.toString();
}
32 changes: 32 additions & 0 deletions test/io_utils_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -17,4 +17,36 @@ void main() {
expect(getFileNameFor('dartdoc.generator'), 'dartdoc-generator.html');
});
});

group('printWarningDelta', () {
Map<String, int> original;
Map<String, int> current;
setUp(() {
original = new Map.fromIterables(
["originalwarning", "morewarning", "duplicateoriginalwarning"],
[1, 1, 2]);
current = new Map.fromIterables(
["newwarning", "morewarning", "duplicateoriginalwarning"], [1, 1, 1]);
});

test('verify output of printWarningDelta', () {
expect(
printWarningDelta('Diff Title', 'oldbranch', original, current),
equals(
'*** Diff Title : 1 warnings from original (oldbranch) missing in current:\n'
'originalwarning\n'
'*** Diff Title : 1 new warnings not in original (oldbranch)\n'
'newwarning\n'
'*** Diff Title : Identical warning quantity changed\n'
'* Appeared 2 times in original (oldbranch), now 1:\n'
'duplicateoriginalwarning\n'));
});

test('verify output when nothing changes', () {
expect(
printWarningDelta('Diff Title 2', 'oldbranch2', original, original),
equals(
'*** Diff Title 2 : No difference in warning output from original (oldbranch2) (3 warnings found)\n'));
});
});
}
Loading