Skip to content
This repository was archived by the owner on Feb 22, 2023. It is now read-only.

[flutter_plugin_tool] Fix CHANGELOG validation failure summary #4266

Merged
stuartmorgan-g merged 1 commit into from
Aug 26, 2021
Merged

[flutter_plugin_tool] Fix CHANGELOG validation failure summary #4266

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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 script/tool/lib/src/version_check_command.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ class VersionCheckCommand extends PackageLoopingCommand {

if (!(await _validateChangelogVersion(package,
pubspec: pubspec, pubspecVersionChanged: versionChanged))) {
errors.add('pubspec.yaml and CHANGELOG.md have different versions');
errors.add('CHANGELOG.md failed validation.');
Copy link
Contributor

@cyanglaz cyanglaz Aug 25, 2021

Choose a reason for hiding this comment

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

Do we have a place to explain what are the failing scenarios? If not, maybe we should list them here so it's easier for debugging. (Especially when none-team-member contributor hits this error)

Copy link
Contributor Author

@stuartmorgan-g stuartmorgan-g Aug 25, 2021

Choose a reason for hiding this comment

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

The specific error is output inline, this is just the line that goes in the final summary. The summary block always says to see the full log for details. See https://cirrus-ci.com/task/4541043946291200 for an example (that led to this PR).

}

return errors.isEmpty
Expand Down
66 changes: 25 additions & 41 deletions script/tool/test/version_check_command_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,11 +54,15 @@ void main() {
late List<List<String>> gitDirCommands;
Map<String, String> gitShowResponses;
late MockGitDir gitDir;
// Ignored if mockHttpResponse is set.
int mockHttpStatus;
Map<String, dynamic>? mockHttpResponse;

setUp(() {
fileSystem = MemoryFileSystem();
mockPlatform = MockPlatform();
packagesDir = createPackagesDirectory(fileSystem: fileSystem);

Copy link
Contributor

@cyanglaz cyanglaz Aug 25, 2021

Choose a reason for hiding this comment

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

nits: extra line

Copy link
Contributor Author

@stuartmorgan-g stuartmorgan-g Aug 25, 2021

Choose a reason for hiding this comment

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

I added this on purpose; the next section is a big block of setup for the mock GitDir and having it separated out slightly makes it easier to see the high-level flow vs it being a wall of text.

(I'll likely replace the git setup here with something based on processRunner like I did with the new publish tests, which would help, but didn't want to get into that in this PR.)

gitDirCommands = <List<String>>[];
gitShowResponses = <String, String>{};
gitDir = MockGitDir();
Expand All @@ -81,9 +85,21 @@ void main() {
}
return Future<io.ProcessResult>.value(mockProcessResult);
});

// Default to simulating the plugin never having been published.
mockHttpStatus = 404;
mockHttpResponse = null;
final MockClient mockClient = MockClient((http.Request request) async {
return http.Response(json.encode(mockHttpResponse),
mockHttpResponse == null ? mockHttpStatus : 200);
});

processRunner = RecordingProcessRunner();
final VersionCheckCommand command = VersionCheckCommand(packagesDir,
processRunner: processRunner, platform: mockPlatform, gitDir: gitDir);
processRunner: processRunner,
platform: mockPlatform,
gitDir: gitDir,
httpClient: mockClient);

runner = CommandRunner<void>(
'version_check_command', 'Test for $VersionCheckCommand');
Expand Down Expand Up @@ -456,7 +472,9 @@ void main() {
output,
containsAllInOrder(<Matcher>[
contains('When bumping the version for release, the NEXT section '
'should be incorporated into the new version\'s release notes.')
'should be incorporated into the new version\'s release notes.'),
contains('plugin:\n'
' CHANGELOG.md failed validation.'),
]),
);
});
Expand Down Expand Up @@ -497,23 +515,14 @@ void main() {
});

test('allows valid against pub', () async {
const Map<String, dynamic> httpResponse = <String, dynamic>{
mockHttpResponse = <String, dynamic>{
'name': 'some_package',
'versions': <String>[
'0.0.1',
'0.0.2',
'1.0.0',
],
};
final MockClient mockClient = MockClient((http.Request request) async {
return http.Response(json.encode(httpResponse), 200);
});
final VersionCheckCommand command = VersionCheckCommand(packagesDir,
processRunner: processRunner, gitDir: gitDir, httpClient: mockClient);

runner = CommandRunner<void>(
'version_check_command', 'Test for $VersionCheckCommand');
runner.addCommand(command);

createFakePlugin('plugin', packagesDir, version: '2.0.0');
gitShowResponses = <String, String>{
Expand All @@ -531,22 +540,13 @@ void main() {
});

test('denies invalid against pub', () async {
const Map<String, dynamic> httpResponse = <String, dynamic>{
mockHttpResponse = <String, dynamic>{
'name': 'some_package',
'versions': <String>[
'0.0.1',
'0.0.2',
],
};
final MockClient mockClient = MockClient((http.Request request) async {
return http.Response(json.encode(httpResponse), 200);
});
final VersionCheckCommand command = VersionCheckCommand(packagesDir,
processRunner: processRunner, gitDir: gitDir, httpClient: mockClient);

runner = CommandRunner<void>(
'version_check_command', 'Test for $VersionCheckCommand');
runner.addCommand(command);

createFakePlugin('plugin', packagesDir, version: '2.0.0');
gitShowResponses = <String, String>{
Expand Down Expand Up @@ -578,15 +578,7 @@ ${indentation}Allowed versions: {1.0.0: NextVersionType.BREAKING_MAJOR, 0.1.0: N
test(
'throw and print error message if http request failed when checking against pub',
() async {
final MockClient mockClient = MockClient((http.Request request) async {
return http.Response('xx', 400);
});
final VersionCheckCommand command = VersionCheckCommand(packagesDir,
processRunner: processRunner, gitDir: gitDir, httpClient: mockClient);

runner = CommandRunner<void>(
'version_check_command', 'Test for $VersionCheckCommand');
runner.addCommand(command);
mockHttpStatus = 400;

createFakePlugin('plugin', packagesDir, version: '2.0.0');
gitShowResponses = <String, String>{
Expand All @@ -609,23 +601,15 @@ ${indentation}Allowed versions: {1.0.0: NextVersionType.BREAKING_MAJOR, 0.1.0: N
contains('''
${indentation}Error fetching version on pub for plugin.
${indentation}HTTP Status 400
${indentation}HTTP response: xx
${indentation}HTTP response: null
''')
]),
);
});

test('when checking against pub, allow any version if http status is 404.',
() async {
final MockClient mockClient = MockClient((http.Request request) async {
return http.Response('xx', 404);
});
final VersionCheckCommand command = VersionCheckCommand(packagesDir,
processRunner: processRunner, gitDir: gitDir, httpClient: mockClient);

runner = CommandRunner<void>(
'version_check_command', 'Test for $VersionCheckCommand');
runner.addCommand(command);
mockHttpStatus = 404;

createFakePlugin('plugin', packagesDir, version: '2.0.0');
gitShowResponses = <String, String>{
Expand Down