Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
21 changes: 14 additions & 7 deletions .github/workflows/release.yml
Original file line number Diff line number Diff line change
@@ -1,12 +1,18 @@
name: release
name: Reusable Release
on:
push:
branches:
- main

workflow_call:
inputs:
publish-args:
required: true
type: string
workflow-name:
required: true
type: string
branch-name:
required: true
type: string
# Declare default permissions as read only.
permissions: read-all

jobs:
release:
if: github.repository_owner == 'flutter'
Expand All @@ -21,6 +27,7 @@ jobs:
uses: actions/checkout@8e8c483db84b4bee98b60c0593521ed34d9990e8
with:
fetch-depth: 0 # Fetch all history so the tool can get all the tags to determine version.
ref: ${{ inputs.branch-name }}
- name: "Install Flutter"
uses: ./.github/workflows/internals/install_flutter
- name: Set up tools
Expand Down Expand Up @@ -66,5 +73,5 @@ jobs:
run: |
git config --global user.name ${{ secrets.USER_NAME }}
git config --global user.email ${{ secrets.USER_EMAIL }}
dart ./script/tool/lib/src/main.dart publish --all-changed --base-sha=HEAD~ --skip-confirmation --remote=origin
dart ./script/tool/lib/src/main.dart publish ${{ inputs.publish-args }}
env: {PUB_CREDENTIALS: "${{ secrets.PUB_CREDENTIALS }}"}
13 changes: 13 additions & 0 deletions .github/workflows/release_from_branches.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
name: Batch Release
on:
push:
branches:
- 'release-go-router'
jobs:
Copy link
Contributor

Choose a reason for hiding this comment

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

If we were to reuse a common release sub workflow for both release from branches and release from main in the future, we should refactor it now for release from branches.

because we have to make sure the sub workflow works before using it for release from main, so we might as well do it now for release from branches.

release:
uses: ./.github/workflows/release.yml
with:
publish-args: '--all-changed --batch-release --base-sha=HEAD~ --skip-confirmation --remote=origin'
Copy link
Contributor

Choose a reason for hiding this comment

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

feels like this can be a boolean?

workflow-name: 'Batch Release ${{ github.ref_name }}'
branch-name: '${{ github.ref_name }}'
secrets: inherit
13 changes: 13 additions & 0 deletions .github/workflows/release_from_main.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
name: Main Release
on:
push:
branches:
- main
jobs:
release:
uses: ./.github/workflows/release.yml
with:
publish-args: '--all-changed --base-sha=HEAD~ --skip-confirmation --remote=origin'
workflow-name: 'Main Release'
branch-name: 'main'
secrets: inherit
31 changes: 31 additions & 0 deletions script/tool/lib/src/publish_command.dart
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import 'package:platform/platform.dart';
import 'package:pub_semver/pub_semver.dart';
import 'package:yaml/yaml.dart';

import 'common/ci_config.dart';
import 'common/core.dart';
import 'common/file_utils.dart';
import 'common/output_utils.dart';
Expand Down Expand Up @@ -83,6 +84,10 @@ class PublishCommand extends PackageLoopingCommand {
'Release all packages that contains pubspec changes at the current commit compares to the base-sha.\n'
'The --packages option is ignored if this is on.',
);
argParser.addFlag(
Copy link
Collaborator

Choose a reason for hiding this comment

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

Flags are booleans; they don't take arguments. Options take arguments. This is why 30+ unit tests are failing.

_batchReleaseFlag,
help: 'only release the packages that opt-in for batch release option.',
);
argParser.addFlag(
_dryRunFlag,
help:
Expand All @@ -109,6 +114,7 @@ class PublishCommand extends PackageLoopingCommand {
static const String _pubFlagsOption = 'pub-publish-flags';
static const String _remoteOption = 'remote';
static const String _allChangedFlag = 'all-changed';
static const String _batchReleaseFlag = 'batch-release';
static const String _dryRunFlag = 'dry-run';
static const String _skipConfirmationFlag = 'skip-confirmation';
static const String _tagForAutoPublishFlag = 'tag-for-auto-publish';
Expand Down Expand Up @@ -196,6 +202,31 @@ class PublishCommand extends PackageLoopingCommand {
.toList();

for (final pubspecPath in changedPubspecs) {
// Read the ci_config.yaml file if it exists

Copy link
Collaborator

Choose a reason for hiding this comment

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

Implementation comments should be attached to the code they are describing, not separated from it, so this blank line should be removed.

final String packageName = p.basename(p.dirname(pubspecPath));
bool isBatchReleasePackage;
Copy link
Collaborator

Choose a reason for hiding this comment

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

This should be final.

try {
final File ciConfigFile = packagesDir.fileSystem
.file(pubspecPath)
.parent
.childFile('ci_config.yaml');
if (!ciConfigFile.existsSync()) {
isBatchReleasePackage = false;
} else {
final CIConfig ciConfig =
CIConfig.parse(ciConfigFile.readAsStringSync());
isBatchReleasePackage = ciConfig.isBatchRelease;
}
} catch (e) {
printError('Could not parse ci_config.yaml for $packageName: $e');
Copy link
Contributor

Choose a reason for hiding this comment

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

we should throw tool exit in this case

Copy link
Contributor

Choose a reason for hiding this comment

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

also should add a test for this

continue;
}
// Skip the package if batch release flag is not set to match the ci_config.yaml
if (getBoolArg(_batchReleaseFlag) != isBatchReleasePackage) {
continue;
}

// git outputs a relativa, Posix-style path.
final File pubspecFile = childFileWithSubcomponents(
packagesDir.fileSystem.directory((await gitDir).path),
Expand Down
220 changes: 220 additions & 0 deletions script/tool/test/publish_command_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -1284,6 +1284,226 @@ void main() {
);
});

group('--batch-release flag', () {
test(
'filters packages based on the existence of ci_config.yaml',
() async {
// Mock pub.dev responses.
mockHttpResponses['plugin1'] = <String, dynamic>{
'name': 'plugin1',
'versions': <String>['0.0.1'],
};
mockHttpResponses['plugin2'] = <String, dynamic>{
'name': 'plugin2',
'versions': <String>['0.0.1'],
};

// Mock packages.
final RepositoryPackage plugin1 = createFakePlugin(
'plugin1',
packagesDir,
version: '0.0.2',
batchRelease: true,
);

final RepositoryPackage plugin2 = createFakePlugin(
'plugin2',
packagesDir,
version: '0.0.2',
);

expect(plugin1.ciConfigFile.existsSync(), true);
expect(plugin2.ciConfigFile.existsSync(), false);

// Mock git diff to show both packages have changed.
processRunner
.mockProcessesForExecutable['git-diff'] = <FakeProcessInfo>[
FakeProcessInfo(
MockProcess(
stdout:
'${plugin1.pubspecFile.path}\n${plugin2.pubspecFile.path}',
),
),
];

mockStdin.readLineOutput = 'y';

final List<String> output = await runCapturingPrint(
commandRunner,
<String>[
'publish',
'--all-changed',
'--base-sha=HEAD~',
'--batch-release',
],
);
// Package1 is published in batch realease, pacakge2 is not.
expect(
output,
containsAllInOrder(<Matcher>[
contains('Running `pub publish ` in ${plugin1.path}...'),
contains('Published plugin1 successfully!'),
]),
);

expect(
output,
isNot(
contains(
contains('Running `pub publish ` in ${plugin2.path}...!'),
),
),
);
expect(
output,
isNot(contains(contains('Published plugin2 successfully!'))),
);
},
);

test(
'filters packages based on the batch release flag value in ci_config.yaml',
() async {
// Mock pub.dev responses.
mockHttpResponses['plugin1'] = <String, dynamic>{
'name': 'plugin1',
'versions': <String>['0.0.1'],
};
mockHttpResponses['plugin2'] = <String, dynamic>{
'name': 'plugin2',
'versions': <String>['0.0.1'],
};

// Mock packages.
final RepositoryPackage plugin1 = createFakePlugin(
'plugin1',
packagesDir,
version: '0.0.2',
batchRelease: true,
);

final RepositoryPackage plugin2 = createFakePlugin(
'plugin2',
packagesDir,
version: '0.0.2',
batchRelease: false,
);

// Mock git diff to show both packages have changed.
processRunner
.mockProcessesForExecutable['git-diff'] = <FakeProcessInfo>[
FakeProcessInfo(
MockProcess(
stdout:
'${plugin1.pubspecFile.path}\n${plugin2.pubspecFile.path}',
),
),
];

mockStdin.readLineOutput = 'y';

final List<String> output = await runCapturingPrint(
commandRunner,
<String>[
'publish',
'--all-changed',
'--base-sha=HEAD~',
'--batch-release',
],
);
// Package1 is published in batch realease, pacakge2 is not.
expect(
output,
containsAllInOrder(<Matcher>[
contains('Running `pub publish ` in ${plugin1.path}...'),
contains('Published plugin1 successfully!'),
]),
);

expect(
output,
isNot(
contains(
contains('Running `pub publish ` in ${plugin2.path}...!'),
),
),
);
expect(
output,
isNot(contains(contains('Published plugin2 successfully!'))),
);
},
);

test(
'when --batch-release flag value is false, batch release packages are filtered out',
() async {
// Mock pub.dev responses.
mockHttpResponses['plugin1'] = <String, dynamic>{
'name': 'plugin1',
'versions': <String>['0.0.1'],
};
mockHttpResponses['plugin2'] = <String, dynamic>{
'name': 'plugin2',
'versions': <String>['0.0.1'],
};

// Mock packages.
final RepositoryPackage plugin1 = createFakePlugin(
'plugin1',
packagesDir,
version: '0.0.2',
);

final RepositoryPackage plugin2 = createFakePlugin(
'plugin2',
packagesDir,
version: '0.0.2',
batchRelease: true,
);

// Mock git diff to show both packages have changed.
processRunner
.mockProcessesForExecutable['git-diff'] = <FakeProcessInfo>[
FakeProcessInfo(
MockProcess(
stdout:
'${plugin1.pubspecFile.path}\n${plugin2.pubspecFile.path}',
),
),
];

mockStdin.readLineOutput = 'y';

final List<String> output = await runCapturingPrint(
commandRunner,
<String>['publish', '--all-changed', '--base-sha=HEAD~'],
);
// Package1 is published in batch realease, pacakge2 is not.
expect(
output,
containsAllInOrder(<Matcher>[
contains('Running `pub publish ` in ${plugin1.path}...'),
contains('Published plugin1 successfully!'),
]),
);

expect(
output,
isNot(
contains(
contains('Running `pub publish ` in ${plugin2.path}...!'),
),
),
);
expect(
output,
isNot(contains(contains('Published plugin2 successfully!'))),
);
},
);
});

test('Do not release flutter_plugin_tools', () async {
mockHttpResponses['plugin1'] = <String, dynamic>{
'name': 'flutter_plugin_tools',
Expand Down
16 changes: 16 additions & 0 deletions script/tool/test/util.dart
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ RepositoryPackage createFakePlugin(
String? version = '0.0.1',
String flutterConstraint = _defaultFlutterConstraint,
String dartConstraint = _defaultDartConstraint,
bool? batchRelease,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nullable booleans need clear documentation about what null means. Here it seems to be that null means there's no CI config, while false means that it explicitly has a config that doesn't do batch release.

Although that seems like more than a documentation problem; this won't scale to adding other CI config options in the future. I think it would be much clearer to have a separate helper method to add a ci_config.yaml to a RepositoryPackage, and then that can take a bunch of arguments for what settings to use, without those implicitly controlling the creation of the file.

Copy link
Collaborator

Choose a reason for hiding this comment

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

(Also, that will resolve the problem that this is forcing all tests to creating plugins, even if they only want to test non-plugin packages.)

}) {
final RepositoryPackage package = createFakePackage(
name,
Expand All @@ -117,6 +118,9 @@ RepositoryPackage createFakePlugin(
flutterConstraint: flutterConstraint,
dartConstraint: dartConstraint,
);
if (batchRelease != null) {
createFakeCiConfig(batchRelease, package);
}

return package;
}
Expand Down Expand Up @@ -287,6 +291,18 @@ $pluginSection
package.pubspecFile.writeAsStringSync(yaml);
}

/// Creates a `ci_config.yaml` file for [package].
void createFakeCiConfig(bool batchRelease, RepositoryPackage package) {
final yaml =
'''
release:
batch: $batchRelease
''';

package.ciConfigFile.createSync();
package.ciConfigFile.writeAsStringSync(yaml);
}

String _pluginPlatformSection(
String platform,
PlatformDetails support,
Expand Down
Loading