diff --git a/CHANGELOG.md b/CHANGELOG.md index d163669..75ac918 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/lib/src/cli/commands/command_mixin.dart b/lib/src/cli/commands/command_mixin.dart index 1afa88b..ab4df17 100644 --- a/lib/src/cli/commands/command_mixin.dart +++ b/lib/src/cli/commands/command_mixin.dart @@ -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( @@ -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( diff --git a/lib/src/cli/package_ref.dart b/lib/src/cli/package_ref.dart index 1219220..e39f853 100644 --- a/lib/src/cli/package_ref.dart +++ b/lib/src/cli/package_ref.dart @@ -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 diff --git a/lib/src/diff/report/json_diff_reporter.dart b/lib/src/diff/report/json_diff_reporter.dart index ede6988..9f8de98 100644 --- a/lib/src/diff/report/json_diff_reporter.dart +++ b/lib/src/diff/report/json_diff_reporter.dart @@ -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 diff --git a/lib/src/tooling/pub_interaction.dart b/lib/src/tooling/pub_interaction.dart index 935b6ea..f9ccda8 100644 --- a/lib/src/tooling/pub_interaction.dart +++ b/lib/src/tooling/pub_interaction.dart @@ -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 installPackageToCache( String packageName, - String version, { + String? version, { StdoutSession? stdoutSession, }) async { await DartInteraction.runDartCommand( @@ -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); } @@ -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 getAllPackageVersionsForPackageInCache( + String packageName) { + final cacheDir = pubCacheDir; + final hostedDir = path.join(cacheDir, 'hosted'); + + final versions = {}; + + final repositoryDirs = + Directory(hostedDir).listSync().whereType().toList(); + + for (final repositoryDir in repositoryDirs) { + for (final potentialPackageDir + in repositoryDir.listSync().whereType()) { + 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 hostedUrls) { for (final hostedUrl in hostedUrls) { final packagePath = path.join(hostedUrl, '$packageName-$version'); diff --git a/pubspec.yaml b/pubspec.yaml index f75e89a..d2c66b7 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -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" @@ -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: diff --git a/test/integration_tests/cli/diff_command_test.dart b/test/integration_tests/cli/diff_command_test.dart index 280c3ea..dadb712 100644 --- a/test/integration_tests/cli/diff_command_test.dart +++ b/test/integration_tests/cli/diff_command_test.dart @@ -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('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, + ); }); } diff --git a/test/unit_tests/diff/report/json_diff_reporter_test.dart b/test/unit_tests/diff/report/json_diff_reporter_test.dart new file mode 100644 index 0000000..511ab33 --- /dev/null +++ b/test/unit_tests/diff/report/json_diff_reporter_test.dart @@ -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()); + }); + + 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); + }); + }); +}