Skip to content

Commit

Permalink
Report major breaking change type in JSON report and support pub refs…
Browse files Browse the repository at this point in the history
… without version (#189)

* Version 0.19.0

* Version 0.19.1-dev

* fix "type" field in JSON export to contain "major" for breaking changes

* support omitting version in pub refs
  • Loading branch information
devmil authored Oct 14, 2024
1 parent 04e6ec7 commit 0f46de7
Show file tree
Hide file tree
Showing 8 changed files with 276 additions and 10 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
# Changelog

## Version 0.19.1
- fix JSON report to specify "major" as change type for breaking changes
- adapt pub ref mechanism to also support omitting the version (and using the latest version if no version has been specified)

## Version 0.19.0
- introduces `force-use-flutter` option for all commands to force dart_apitool to use the `flutter` command.
- extend type usage tracking and fix situations in which types that are used in @visibleForTesting contexts were detected as not exported
Expand Down
8 changes: 4 additions & 4 deletions lib/src/cli/commands/command_mixin.dart
Original file line number Diff line number Diff line change
Expand Up @@ -69,12 +69,12 @@ OBSOLETE: Has no effect anymore.
isInCache: false,
));
} else if (ref.isPubRef) {
await stdoutSession
.writeln('Preparing ${ref.pubPackage!}:${ref.pubVersion!}');
await stdoutSession.writeln(
'Preparing ${ref.pubPackage!}:${ref.pubVersion ?? 'latest'}');
await stdoutSession.writeln('Downloading');
String sourceDir = await PubInteraction.installPackageToCache(
ref.pubPackage!,
ref.pubVersion!,
ref.pubVersion,
stdoutSession: stdoutSession,
);
sources.add(SourceItem(
Expand Down Expand Up @@ -156,7 +156,7 @@ OBSOLETE: Has no effect anymore.
if (preparedRef.packageRef.isPubRef) {
path = PubInteraction.getPackagePathInCache(
preparedRef.packageRef.pubPackage!,
preparedRef.packageRef.pubVersion!);
preparedRef.packageRef.pubVersion);
}
if (path == null) {
throw ArgumentError(
Expand Down
6 changes: 5 additions & 1 deletion lib/src/cli/package_ref.dart
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,11 @@ class PackageRef {
return null;
}
final uri = Uri.parse(ref);
return uri.path.replaceAll('/', '');
final path = uri.path.replaceAll('/', '');
if (path.isEmpty) {
return null;
}
return path;
}

@override
Expand Down
6 changes: 5 additions & 1 deletion lib/src/diff/report/json_diff_reporter.dart
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,11 @@ class JsonDiffReporter extends DiffReporter {
'changeDescription': c.changeDescription,
'changeCode': c.changeCode.code,
'isBreaking': c.isBreaking,
'type': c.type.requiresMinorBump ? 'minor' : 'patch',
'type': c.isBreaking
? 'major'
: c.type.requiresMinorBump
? 'minor'
: 'patch',
})
.toList();
final childNodes = n.children.values
Expand Down
58 changes: 55 additions & 3 deletions lib/src/tooling/pub_interaction.dart
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,17 @@ import 'dart:io';
import 'package:dart_apitool/src/tooling/dart_interaction.dart';
import 'package:freezed_annotation/freezed_annotation.dart';
import 'package:path/path.dart' as path;
import 'package:pub_semver/pub_semver.dart';
import '../errors/run_dart_error.dart';
import '../utils/utils.dart';

/// helper class for interactions with pub
abstract class PubInteraction {
/// installs a package to the pub cache and returns the path to it throws [RunDartError] on failure
/// if no [version] is provided dart will decide what version to use ("the best of all known versions")
static Future<String> installPackageToCache(
String packageName,
String version, {
String? version, {
StdoutSession? stdoutSession,
}) async {
await DartInteraction.runDartCommand(
Expand All @@ -20,10 +22,17 @@ abstract class PubInteraction {
'cache',
'add',
packageName,
'-v $version',
if (version != null) '-v $version',
],
stdoutSession: stdoutSession,
);
if (version == null) {
await stdoutSession?.writeln(
'No version for $packageName specified, using latest version');
final latestVersion = getLatestVersionInCacheFor(packageName);
version = latestVersion.toString();
await stdoutSession?.writeln('Using version $version');
}
return getPackagePathInCache(packageName, version);
}

Expand Down Expand Up @@ -56,8 +65,51 @@ abstract class PubInteraction {
return cacheDir;
}

static Version getLatestVersionInCacheFor(String packageName) {
final allVersions = getAllPackageVersionsForPackageInCache(packageName);
final versions = allVersions.keys.toList(growable: false);
if (versions.isEmpty) {
throw RunDartError('No version of $packageName found');
}

return versions.reduce((v1, v2) => v1 > v2 ? v1 : v2);
}

static Map<Version, String> getAllPackageVersionsForPackageInCache(
String packageName) {
final cacheDir = pubCacheDir;
final hostedDir = path.join(cacheDir, 'hosted');

final versions = <Version, String>{};

final repositoryDirs =
Directory(hostedDir).listSync().whereType<Directory>().toList();

for (final repositoryDir in repositoryDirs) {
for (final potentialPackageDir
in repositoryDir.listSync().whereType<Directory>()) {
final packageDirName = path.basename(potentialPackageDir.path);
final dirParts = packageDirName.split('-').toList();
final versionPart = dirParts.last;
dirParts.removeLast();
final packageNamePart = dirParts.join('-');

if (packageNamePart != packageName) {
continue;
}

final version = Version.parse(versionPart);
versions[version] = potentialPackageDir.path;
}
}
return versions;
}

/// returns the cache path of a package with the given [packageName] and [version]
static String getPackagePathInCache(String packageName, String version) {
/// if [version] is null the latest version is used
static String getPackagePathInCache(String packageName, String? version) {
version ??= getLatestVersionInCacheFor(packageName).toString();

String? findHostedDirectory(List<String> hostedUrls) {
for (final hostedUrl in hostedUrls) {
final packagePath = path.join(hostedUrl, '$packageName-$version');
Expand Down
3 changes: 2 additions & 1 deletion pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ name: dart_apitool
description: A tool to analyze the public API of a package, create a model of it and diff it against another version to check semver.
repository: https://github.com/bmw-tech/dart_apitool

version: 0.19.0-dev
version: 0.19.1-dev

environment:
sdk: ">=3.0.0 <4.0.0"
Expand Down Expand Up @@ -31,6 +31,7 @@ dev_dependencies:
freezed: ^2.2.0
json_serializable: ^6.3.1
lints: ^3.0.0
mocktail: ^1.0.4
test: ^1.21.4

executables:
Expand Down
20 changes: 20 additions & 0 deletions test/integration_tests/cli/diff_command_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -143,5 +143,25 @@ void main() {
},
timeout: integrationTestTimeout,
);

test(
'diffing cloud_firestore 4.3.1 to latest works',
() async {
// just some random package for testing the diff command for pub refs without version
final diffCommand = DiffCommand();
final runner =
CommandRunner<int>('dart_apitool_tests', 'Test for dart_apitool')
..addCommand(diffCommand);
final exitCode = await runner.run([
'diff',
'--old',
'pub://cloud_firestore/4.3.1',
'--new',
'pub://cloud_firestore',
]);
expect(exitCode, 0);
},
timeout: integrationTestTimeout,
);
});
}
181 changes: 181 additions & 0 deletions test/unit_tests/diff/report/json_diff_reporter_test.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,181 @@
import 'dart:convert';
import 'dart:io';

import 'package:dart_apitool/api_tool_cli.dart';
import 'package:dart_apitool/src/cli/commands/version_check.dart';
import 'package:dart_apitool/src/diff/report/json_diff_reporter.dart';
import 'package:pub_semver/pub_semver.dart';
import 'package:test/test.dart';

import 'package:mocktail/mocktail.dart';

class MockFile extends Mock implements File {}

void main() {
group('JsonDiffReporter', () {
late JsonDiffReporter reporter;
final anyVersionCheckResult = VersionCheckResult.success(
oldVersion: Version(1, 0, 0),
newVersion: Version(2, 0, 0),
explanation: '',
);
final oldPackageRef = PackageRef('pub://package/1.0.0');
final newPackageRef = PackageRef('pub://package/2.0.0');
late MockFile mockFile;
late StringBuffer collectedFileContent;

setUp(() {
collectedFileContent = StringBuffer();
mockFile = MockFile();
reporter = JsonDiffReporter(
oldPackageRef: oldPackageRef,
newPackageRef: newPackageRef,
outputFile: mockFile,
);
when(() => mockFile.writeAsString(any())).thenAnswer((invocation) async {
collectedFileContent.write(invocation.positionalArguments.first);
return mockFile;
});
when(() => mockFile.path).thenReturn('mocked_file_path.json');
});

PackageApiDiffResult createEmptyDiffResult() {
return PackageApiDiffResult();
}

void addBreakingChange(
PackageApiDiffResult diffResult, {
ApiChangeCode changeCode = ApiChangeCode.ci01,
}) {
diffResult.addApiChange(ApiChange(
changeCode: changeCode,
changeDescription: 'Test breaking change: ${changeCode.name}',
contextTrace: [],
isExperimental: false,
type: ApiChangeType.remove,
));
}

void addNonBreakingChange(
PackageApiDiffResult diffResult, {
ApiChangeCode changeCode = ApiChangeCode.ci02,
ApiChangeType changeType = ApiChangeType.addCompatiblePatch,
}) {
diffResult.addApiChange(ApiChange(
changeCode: changeCode,
changeDescription: 'Test non-breaking change: ${changeCode.name}',
contextTrace: [],
isExperimental: false,
type: changeType,
));
}

test('Can be instantiated', () {
// the setup would have failed already
expect(reporter, isA<JsonDiffReporter>());
});

test('Can handle empty diff report', () async {
final diffResult = createEmptyDiffResult();
await reporter.generateReport(diffResult, anyVersionCheckResult);
final jsonReport = jsonDecode(collectedFileContent.toString());
expect(jsonReport['report']['noChangesDetected'], isTrue);
});
test('Can handle diff report with only one breaking change', () async {
final diffResult = createEmptyDiffResult();
addBreakingChange(diffResult, changeCode: ApiChangeCode.ci01);
await reporter.generateReport(diffResult, anyVersionCheckResult);
final jsonReport = jsonDecode(collectedFileContent.toString());
expect(jsonReport['report']['breakingChanges'], isNotNull);
expect(jsonReport['report']['breakingChanges']['children'].length, 1);
expect(
jsonReport['report']['breakingChanges']['children']
.single['changeCode'],
ApiChangeCode.ci01.code);
expect(jsonReport['report']['breakingChanges']['children'].single['type'],
'major');
});
test('Can handle diff report with multiple breaking changes', () async {
final diffResult = createEmptyDiffResult();
addBreakingChange(diffResult, changeCode: ApiChangeCode.ci01);
addBreakingChange(diffResult, changeCode: ApiChangeCode.ci04);
await reporter.generateReport(diffResult, anyVersionCheckResult);
final jsonReport = jsonDecode(collectedFileContent.toString());
expect(jsonReport['report']['breakingChanges'], isNotNull);
expect(jsonReport['report']['breakingChanges']['children'].length, 2);
expect(
jsonReport['report']['breakingChanges']['children'][0]['changeCode'],
ApiChangeCode.ci01.code);
expect(
jsonReport['report']['breakingChanges']['children'][1]['changeCode'],
ApiChangeCode.ci04.code);
});
test('Can handle diff report with only one non-breaking change', () async {
final diffResult = createEmptyDiffResult();
addNonBreakingChange(diffResult, changeCode: ApiChangeCode.ci02);
await reporter.generateReport(diffResult, anyVersionCheckResult);
final jsonReport = jsonDecode(collectedFileContent.toString());
expect(jsonReport['report']['nonBreakingChanges'], isNotNull);
expect(jsonReport['report']['nonBreakingChanges']['children'].length, 1);
expect(
jsonReport['report']['nonBreakingChanges']['children']
.single['changeCode'],
ApiChangeCode.ci02.code);
expect(
jsonReport['report']['nonBreakingChanges']['children'].single['type'],
'patch');
});
test('Can handle diff report with multiple non-breaking changes', () async {
final diffResult = createEmptyDiffResult();
addNonBreakingChange(diffResult, changeCode: ApiChangeCode.ci02);
addNonBreakingChange(diffResult,
changeCode: ApiChangeCode.ci05,
changeType: ApiChangeType.addCompatibleMinor);
await reporter.generateReport(diffResult, anyVersionCheckResult);
final jsonReport = jsonDecode(collectedFileContent.toString());
expect(jsonReport['report']['nonBreakingChanges'], isNotNull);
expect(jsonReport['report']['nonBreakingChanges']['children'].length, 2);
expect(
jsonReport['report']['nonBreakingChanges']['children'][0]
['changeCode'],
ApiChangeCode.ci02.code);
expect(
jsonReport['report']['nonBreakingChanges']['children'][1]
['changeCode'],
ApiChangeCode.ci05.code);
expect(jsonReport['report']['nonBreakingChanges']['children'][0]['type'],
'patch');
expect(jsonReport['report']['nonBreakingChanges']['children'][1]['type'],
'minor');
});
test('Can handle diff report with breaking and non-breaking changes',
() async {
final diffResult = createEmptyDiffResult();
addBreakingChange(diffResult, changeCode: ApiChangeCode.ci01);
addNonBreakingChange(diffResult, changeCode: ApiChangeCode.ci02);
addBreakingChange(diffResult, changeCode: ApiChangeCode.ci04);
addNonBreakingChange(diffResult, changeCode: ApiChangeCode.ci05);

await reporter.generateReport(diffResult, anyVersionCheckResult);
final jsonReport = jsonDecode(collectedFileContent.toString());
expect(jsonReport['report']['breakingChanges'], isNotNull);
expect(jsonReport['report']['breakingChanges']['children'].length, 2);
expect(
jsonReport['report']['breakingChanges']['children'][0]['changeCode'],
ApiChangeCode.ci01.code);
expect(
jsonReport['report']['breakingChanges']['children'][1]['changeCode'],
ApiChangeCode.ci04.code);
expect(jsonReport['report']['nonBreakingChanges'], isNotNull);
expect(jsonReport['report']['nonBreakingChanges']['children'].length, 2);
expect(
jsonReport['report']['nonBreakingChanges']['children'][0]
['changeCode'],
ApiChangeCode.ci02.code);
expect(
jsonReport['report']['nonBreakingChanges']['children'][1]
['changeCode'],
ApiChangeCode.ci05.code);
});
});
}

0 comments on commit 0f46de7

Please sign in to comment.