Skip to content

Compare flutter warnings between versions very easily #1600

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 4 commits into from
Feb 8, 2018
Merged
Show file tree
Hide file tree
Changes from all 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
2 changes: 1 addition & 1 deletion CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ yet in the issue tracker, start by opening an issue. Thanks!
1. `grind` is needed to run dartdoc integration tests, see installed via `pub global activate grinder`.
2. When a change is user-facing, please add a new entry to the [changelog](https://github.com/dart-lang/dartdoc/blob/master/CHANGELOG.md)
3. Please include a test for your change. `dartdoc` has both `package:test`-style unittests as well as integration tests. To run the unittests, use `dart test/all.dart`. Most changes can be tested via a unittest, but some require modifying the [test_package](https://github.com/dart-lang/dartdoc/tree/master/testing/test_package) and regenerating its docs via `grind update-test-package-docs`.
4. For major changes, run `grind compare-sdk-warnings` and include the summary results in your pull request.
4. For major changes, run `grind compare-sdk-warnings` and `grind compare-flutter-warnings`, and include the summary results in your pull request.
5. Be sure to format your Dart code using `dartfmt -w`, otherwise travis will complain.
6. Post your change via a pull request for review and integration!

Expand Down
7 changes: 5 additions & 2 deletions lib/dartdoc.dart
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,8 @@ class DartDoc extends PackageBuilder {
// That seems wrong. dart-lang/dartdoc#1547
for (Source source in sources) {
ErrorsResult errorsResult = await driver.getErrors(source.fullName);
AnalysisErrorInfo info = new AnalysisErrorInfoImpl(errorsResult.errors, errorsResult.lineInfo);
AnalysisErrorInfo info =
new AnalysisErrorInfoImpl(errorsResult.errors, errorsResult.lineInfo);
List<_Error> errors = [info]
.expand((AnalysisErrorInfo info) {
return info.errors.map((error) =>
Expand All @@ -133,7 +134,9 @@ class DartDoc extends PackageBuilder {
})
.where((_Error error) => error.isError)
// TODO(jcollins-g): remove after conversion to analysis driver
.where((_Error error) => error.error.errorCode != CompileTimeErrorCode.URI_HAS_NOT_BEEN_GENERATED)
.where((_Error error) =>
error.error.errorCode !=
CompileTimeErrorCode.URI_HAS_NOT_BEEN_GENERATED)
.toList()
..sort();

Expand Down
53 changes: 2 additions & 51 deletions lib/src/io_utils.dart
Original file line number Diff line number Diff line change
Expand Up @@ -88,8 +88,8 @@ class SubprocessLauncher {
});
}

SubprocessLauncher(this.context, [Map<String, String> environment]) :
this.environment = environment ?? <String, String>{};
SubprocessLauncher(this.context, [Map<String, String> environment])
: this.environment = environment ?? <String, String>{};

/// A wrapper around start/await process.exitCode that will display the
/// output of the executable continuously and fail on non-zero exit codes.
Expand Down Expand Up @@ -165,52 +165,3 @@ class SubprocessLauncher {
return jsonObjects;
}
}

/// Output formatter for comparing warnings.
String printWarningDelta(String title, String dartdocOriginalBranch,
Map<String, int> original, Map<String, int> current) {
StringBuffer printBuffer = new StringBuffer();
Set<String> quantityChangedOuts = new Set();
Set<String> onlyOriginal = new Set();
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();
}
82 changes: 82 additions & 0 deletions test/grind_test.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
// Copyright (c) 2015, 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.

library dartdoc.io_utils_test;

import 'package:test/test.dart';

import '../tool/grind.dart' hide test;

void main() {
group('printWarningDelta', () {
WarningsCollection original, current;
WarningsCollection originalWithDirs, currentWithDirs;
setUp(() {
original =
new WarningsCollection('/a/tempdir', '/pubcache/path', 'oldbranch');
original.add('originalwarning');
original.add('morewarning');
original.add('duplicateoriginalwarning');
original.add('duplicateoriginalwarning');
current =
new WarningsCollection('/a/tempdir2', '/pubcache/path2', 'current');
current.add('newwarning');
current.add('morewarning');
current.add('duplicateoriginalwarning');
originalWithDirs = new WarningsCollection(
'/a/tempdirFOO', '/pubcache/pathFOO', 'DirsOriginal');
originalWithDirs.add(
'originalWarning found in /a/tempdirFOO/some/subdir/program.dart!!!!');
originalWithDirs.add(
'another originalWarning found in /pubcache/pathFOO/some/package/lib/thingy.dart!!!');
originalWithDirs.add(
'insufficent exclamation mark warning found in /pubcache/pathFOO/some/package/lib/thingy.dart.');
originalWithDirs.add(
'another originalWarning found in /a/tempdirFOO/some/subdir/program.dart');
currentWithDirs = new WarningsCollection(
'/a/tempdirBAR', '/pubcache/pathBAR', 'DirsCurrent');
currentWithDirs.add(
'originalWarning found in /a/tempdirBAR/some/subdir/program.dart!!!!');
currentWithDirs.add(
'another originalWarning found in /pubcache/pathBAR/some/package/lib/thingy.dart!!!');
currentWithDirs.add(
'insufficent exclamation mark warning found in /pubcache/pathBAR/some/package/lib/thingy.dart.');
currentWithDirs.add(
'another originalWarning found in /a/tempdirBAR/some/other/subdir/program.dart');
});

test('verify that paths are substituted when comparing warnings', () {
expect(
originalWithDirs.getPrintableWarningDelta(
'Dirs diff title', currentWithDirs),
equals(
'*** Dirs diff title : 1 warnings from DirsOriginal, missing in DirsCurrent:\n'
'another originalWarning found in /a/tempdirFOO/some/subdir/program.dart\n'
'*** Dirs diff title : 1 new warnings in DirsCurrent, missing in DirsOriginal\n'
'another originalWarning found in /a/tempdirBAR/some/other/subdir/program.dart\n'
'*** Dirs diff title : Difference in warning output found for 2 warnings (5 warnings found)"\n'));
});

test('verify output of printWarningDelta', () {
expect(
original.getPrintableWarningDelta('Diff Title', current),
equals(
'*** Diff Title : 1 warnings from oldbranch, missing in current:\n'
'originalwarning\n'
'*** Diff Title : 1 new warnings in current, missing in oldbranch\n'
'newwarning\n'
'*** Diff Title : Identical warning quantity changed\n'
'* Appeared 2 times in oldbranch, 1 in current:\n'
'duplicateoriginalwarning\n'
'*** Diff Title : Difference in warning output found for 3 warnings (4 warnings found)"\n'));
});

test('verify output when nothing changes', () {
expect(
original.getPrintableWarningDelta('Diff Title 2', original),
equals(
'*** Diff Title 2 : No difference in warning output from oldbranch to oldbranch (3 warnings found)\n'));
});
});
}
32 changes: 0 additions & 32 deletions test/io_utils_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -17,36 +17,4 @@ 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'));
});
});
}
22 changes: 14 additions & 8 deletions test/model_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -320,10 +320,10 @@ void main() {
dog.allInstanceMethods.firstWhere((m) => m.name == 'withMacro');
withMacro2 =
dog.allInstanceMethods.firstWhere((m) => m.name == 'withMacro2');
withPrivateMacro =
dog.allInstanceMethods.firstWhere((m) => m.name == 'withPrivateMacro');
withUndefinedMacro =
dog.allInstanceMethods.firstWhere((m) => m.name == 'withUndefinedMacro');
withPrivateMacro = dog.allInstanceMethods
.firstWhere((m) => m.name == 'withPrivateMacro');
withUndefinedMacro = dog.allInstanceMethods
.firstWhere((m) => m.name == 'withUndefinedMacro');
package.allModelElements.forEach((m) => m.documentation);
});

Expand All @@ -342,7 +342,10 @@ void main() {
});

test("a warning is generated for unknown macros", () {
expect(package.packageWarningCounter.hasWarning(withUndefinedMacro, PackageWarning.unknownMacro, 'ThatDoesNotExist'), isTrue);
expect(
package.packageWarningCounter.hasWarning(withUndefinedMacro,
PackageWarning.unknownMacro, 'ThatDoesNotExist'),
isTrue);
});
});

Expand Down Expand Up @@ -1908,7 +1911,8 @@ String topLevelFunction(int param1, bool param2, Cool coolBeans,
deprecated =
exLibrary.constants.firstWhere((c) => c.name == 'deprecated');
Class Dog = exLibrary.allClasses.firstWhere((c) => c.name == 'Dog');
customClassPrivate = fakeLibrary.constants.firstWhere((c) => c.name == 'CUSTOM_CLASS_PRIVATE');
customClassPrivate = fakeLibrary.constants
.firstWhere((c) => c.name == 'CUSTOM_CLASS_PRIVATE');
aStaticConstField =
Dog.allFields.firstWhere((f) => f.name == 'aStaticConstField');
aName = Dog.allFields.firstWhere((f) => f.name == 'aName');
Expand Down Expand Up @@ -1957,7 +1961,8 @@ String topLevelFunction(int param1, bool param2, Cool coolBeans,
});

test('MY_CAT is linked', () {
expect(cat.constantValue, 'const <a href="ex/ConstantCat/ConstantCat.html">ConstantCat</a>(&#39;tabby&#39;)');
expect(cat.constantValue,
'const <a href="ex/ConstantCat/ConstantCat.html">ConstantCat</a>(&#39;tabby&#39;)');
});

test('exported property', () {
Expand Down Expand Up @@ -2090,7 +2095,8 @@ String topLevelFunction(int param1, bool param2, Cool coolBeans,

test('linked return type', () {
expect(t.linkedReturnType, equals('String'));
expect(generic.linkedReturnType, equals('List<span class="signature">&lt;S&gt;</span>'));
expect(generic.linkedReturnType,
equals('List<span class="signature">&lt;S&gt;</span>'));
});

test("name with generics", () {
Expand Down
3 changes: 2 additions & 1 deletion test/src/utils.dart
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,8 @@ init() async {
testPackageGinormous = await bootBasicPackage(
'testing/test_package', ['css', 'code_in_commnets', 'excluded'], true);

testPackageSmall = await bootBasicPackage('testing/test_package_small', [], false);
testPackageSmall =
await bootBasicPackage('testing/test_package_small', [], false);
testPackageSdk = await bootSdkPackage();
}

Expand Down
Loading