ci: publish changed package versions automatically - #3
Merged
Merged
Conversation
审阅者指南（Reviewer's Guide）在 文件级变更
提示和命令与 Sourcery 交互
自定义你的体验访问你的 控制面板 以：
获取帮助Original review guide in EnglishReviewer's GuideIntroduce automatic PyPI publishing on main when package versions change, refactor release planning to support multiple distributions in dependency order, and document the new release behavior and environments. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - 我发现了 3 个问题，并留下了一些更高层次的反馈：
- 新的发布作业中包含了相当多重复的逻辑（构建、twine 检查、冒烟测试、制品上传）；建议将这些步骤抽取到一个可复用的 workflow 或按包参数化的 composite action 中，以减少重复并降低未来改动出错的概率。
- 现在固定的并发组
pypi-publish会将所有来自标签和 main 分支的发布串行化；如果不同发布之间可以接受并行执行，你可能希望按包或按 ref 来限定并发组的 key，以避免不必要的排队。 publish_sra、publish_m7a和publish_meta的串联needs/if条件相当复杂；可以考虑将依赖逻辑封装到一个辅助表达式中（例如使用 workflow 级别的env或单一的可复用条件），这样可以让发布顺序更易于理解和维护。
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new publish jobs have quite a bit of duplicated logic (build, twine check, smoke test, artifact upload); consider extracting these into a reusable workflow or composite action parametrized by package to reduce repetition and make future changes less error-prone.
- The fixed concurrency group `pypi-publish` now serializes all releases across tags and main; if parallelism between unrelated releases is acceptable, you may want to scope the group key by package or ref to avoid unnecessary queuing.
- The chained `needs`/`if` conditions for `publish_sra`, `publish_m7a`, and `publish_meta` are fairly complex; encapsulating the dependency logic in a helper expression (e.g., via workflow-level `env` or a single reusable condition) would make the publication order easier to reason about and maintain.
## Individual Comments
### Comment 1
<location path="tests/test_release.py" line_range="58-62" />
<code_context>
ref="refs/tags/v0.1.0",
)
+
+ def test_unchanged_versions_do_not_publish(self) -> None:
+ previous = {package: self._version(package) for package in PACKAGE_CONFIG}
+ self.assertEqual(resolve_changed_releases(previous), ())
+
+ def test_changed_versions_follow_dependency_order(self) -> None:
</code_context>
<issue_to_address>
**suggestion (testing):** Add tests for `resolve_changed_releases` handling newly added packages (missing previous version entries).
`resolve_changed_releases` is only tested with a fully populated `previous` mapping. Because it uses `previous_versions.get(package)`, new packages without an entry should be treated as `None` and published. Please add a test where one package is omitted from `previous`, verify that it’s selected for release, and confirm the dependency order is preserved. This will help prevent regressions in how missing entries are handled.
```suggestion
def test_unchanged_versions_do_not_publish(self) -> None:
previous = {package: self._version(package) for package in PACKAGE_CONFIG}
self.assertEqual(resolve_changed_releases(previous), ())
def test_new_package_without_previous_version_is_published(self) -> None:
# Omit one package from the previous mapping to simulate a newly added package
new_package = next(iter(PACKAGE_CONFIG))
previous = {
package: self._version(package)
for package in PACKAGE_CONFIG
if package != new_package
}
targets = resolve_changed_releases(previous)
target_packages = tuple(target.package for target in targets)
# The new package (missing in previous) should be selected for release
self.assertIn(new_package, target_packages)
# The result order should follow RELEASE_ORDER, restricted to the selected packages
expected_order = tuple(
package for package in RELEASE_ORDER if package == new_package
)
self.assertEqual(target_packages, expected_order)
def test_changed_versions_follow_dependency_order(self) -> None:
```
</issue_to_address>
### Comment 2
<location path="tests/test_release.py" line_range="76-85" />
<code_context>
+ def test_github_outputs_mark_only_selected_packages(self) -> None:
</code_context>
<issue_to_address>
**suggestion (testing):** Add coverage for `_write_github_outputs` when no targets are selected and for multiple selected targets.
The existing test only covers the case where a single package (`m7a`) is selected. Please add: (1) a test with `targets` empty, asserting `has_targets == "false"`, `packages` is empty, all `*_selected` flags are `false`, and `*_version`/`*_artifact` fields are empty; and (2) a test with multiple selected packages, asserting `packages` includes all selected packages in order and each package’s `*_selected`, `*_version`, and `*_artifact` values are set correctly. This will more completely validate the GitHub outputs the workflow relies on.
Suggested implementation:
```python
def test_github_outputs_mark_only_selected_packages(self) -> None:
previous = {package: self._version(package) for package in PACKAGE_CONFIG}
previous["automas-hsr-adapter-m7a"] = "previous-version"
targets = resolve_changed_releases(previous)
with TemporaryDirectory() as temp_dir:
output_path = Path(temp_dir) / "github-output.txt"
_write_github_outputs(targets, output_path)
values = dict(
line.split("=", 1)
for line in output_path.read_text(encoding="utf-8").splitlines()
)
# has_targets and packages reflect the single selected package
self.assertEqual(values["has_targets"], "true")
self.assertEqual(values["packages"], "automas-hsr-adapter-m7a")
# Only the selected package is marked as selected; all others are false
for package in PACKAGE_CONFIG:
key_prefix = package.replace("-", "_")
if package == "automas-hsr-adapter-m7a":
self.assertEqual(values[f"{key_prefix}_selected"], "true")
self.assertNotEqual(values[f"{key_prefix}_version"], "")
self.assertNotEqual(values[f"{key_prefix}_artifact"], "")
else:
self.assertEqual(values[f"{key_prefix}_selected"], "false")
self.assertEqual(values[f"{key_prefix}_version"], "")
self.assertEqual(values[f"{key_prefix}_artifact"], "")
def test_github_outputs_with_no_selected_targets(self) -> None:
previous = {package: self._version(package) for package in PACKAGE_CONFIG}
# Ensure there are no changed releases
targets = resolve_changed_releases(previous)
self.assertEqual(targets, ())
with TemporaryDirectory() as temp_dir:
output_path = Path(temp_dir) / "github-output.txt"
_write_github_outputs(targets, output_path)
values = dict(
line.split("=", 1)
for line in output_path.read_text(encoding="utf-8").splitlines()
)
# No targets selected
self.assertEqual(values["has_targets"], "false")
self.assertEqual(values["packages"], "")
# All packages are unselected and have empty version/artifact fields
for package in PACKAGE_CONFIG:
key_prefix = package.replace("-", "_")
self.assertEqual(values[f"{key_prefix}_selected"], "false")
self.assertEqual(values[f"{key_prefix}_version"], "")
self.assertEqual(values[f"{key_prefix}_artifact"], "")
def test_github_outputs_with_multiple_selected_targets(self) -> None:
# Pick two packages from PACKAGE_CONFIG to mark as changed
packages = list(PACKAGE_CONFIG.keys())
# Guard against pathological configs
self.assertGreaterEqual(len(packages), 2)
first, second = packages[0], packages[1]
previous = {package: self._version(package) for package in PACKAGE_CONFIG}
previous[first] = "previous-version-1"
previous[second] = "previous-version-2"
targets = resolve_changed_releases(previous)
with TemporaryDirectory() as temp_dir:
output_path = Path(temp_dir) / "github-output.txt"
_write_github_outputs(targets, output_path)
values = dict(
line.split("=", 1)
for line in output_path.read_text(encoding="utf-8").splitlines()
)
selected_packages = tuple(target.package for target in targets)
self.assertGreaterEqual(len(selected_packages), 2)
# The packages output lists all selected packages in order
self.assertEqual(
values["packages"],
",".join(selected_packages),
)
self.assertEqual(values["has_targets"], "true")
# Each selected package has *_selected true and non-empty version/artifact
selected_set = set(selected_packages)
for package in PACKAGE_CONFIG:
key_prefix = package.replace("-", "_")
if package in selected_set:
self.assertEqual(values[f"{key_prefix}_selected"], "true")
self.assertNotEqual(values[f"{key_prefix}_version"], "")
self.assertNotEqual(values[f"{key_prefix}_artifact"], "")
else:
self.assertEqual(values[f"{key_prefix}_selected"], "false")
self.assertEqual(values[f"{key_prefix}_version"], "")
self.assertEqual(values[f"{key_prefix}_artifact"], "")
```
These changes assume `_write_github_outputs` uses the following conventions:
1. A `has_targets` output of `"true"`/`"false"`.
2. A `packages` output that is a comma-separated list of package names matching `target.package`.
3. Per-package outputs named using `package.replace("-", "_")` as a prefix, e.g. `automas_hsr_adapter_m7a_selected`, `automas_hsr_adapter_m7a_version`, and `automas_hsr_adapter_m7a_artifact`, with `"true"`/`"false"` for `*_selected` and empty strings for unselected packages' `*_version`/`*_artifact`.
If your actual output naming or formatting differs (for example, if you use short names like `m7a_selected` or a different separator than a comma for `packages`), please adjust the key constructions and assertions accordingly to match the real `_write_github_outputs` behavior.
</issue_to_address>
### Comment 3
<location path="tests/test_release.py" line_range="7-13" />
<code_context>
+from tempfile import TemporaryDirectory
-from scripts.release import PACKAGE_CONFIG, project_version, resolve_release
+from scripts.release import (
+ PACKAGE_CONFIG,
+ RELEASE_ORDER,
+ _write_github_outputs,
+ project_version,
+ resolve_changed_releases,
+ resolve_release,
+)
</code_context>
<issue_to_address>
**issue (testing):** Consider adding tests for the new `resolve_release_plan` logic, especially error paths and automatic main releases.
Right now `resolve_release_plan` (automatic main-branch flow with `before_sha` plus manual/tag releases) isn’t covered by dedicated tests. Please add tests that:
1) Confirm `workflow_dispatch` and tag refs delegate to `resolve_release`.
2) Assert unsupported `event_name`/`ref` combinations raise the documented `ValueError`.
3) Check that missing or blank `before_sha` for `push` on `refs/heads/main` raises the correct error.
These will validate the new entry point and guard against misconfiguration.
</issue_to_address>Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Original comment in English
Hey - I've found 3 issues, and left some high level feedback:
- The new publish jobs have quite a bit of duplicated logic (build, twine check, smoke test, artifact upload); consider extracting these into a reusable workflow or composite action parametrized by package to reduce repetition and make future changes less error-prone.
- The fixed concurrency group
pypi-publishnow serializes all releases across tags and main; if parallelism between unrelated releases is acceptable, you may want to scope the group key by package or ref to avoid unnecessary queuing. - The chained
needs/ifconditions forpublish_sra,publish_m7a, andpublish_metaare fairly complex; encapsulating the dependency logic in a helper expression (e.g., via workflow-levelenvor a single reusable condition) would make the publication order easier to reason about and maintain.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new publish jobs have quite a bit of duplicated logic (build, twine check, smoke test, artifact upload); consider extracting these into a reusable workflow or composite action parametrized by package to reduce repetition and make future changes less error-prone.
- The fixed concurrency group `pypi-publish` now serializes all releases across tags and main; if parallelism between unrelated releases is acceptable, you may want to scope the group key by package or ref to avoid unnecessary queuing.
- The chained `needs`/`if` conditions for `publish_sra`, `publish_m7a`, and `publish_meta` are fairly complex; encapsulating the dependency logic in a helper expression (e.g., via workflow-level `env` or a single reusable condition) would make the publication order easier to reason about and maintain.
## Individual Comments
### Comment 1
<location path="tests/test_release.py" line_range="58-62" />
<code_context>
ref="refs/tags/v0.1.0",
)
+
+ def test_unchanged_versions_do_not_publish(self) -> None:
+ previous = {package: self._version(package) for package in PACKAGE_CONFIG}
+ self.assertEqual(resolve_changed_releases(previous), ())
+
+ def test_changed_versions_follow_dependency_order(self) -> None:
</code_context>
<issue_to_address>
**suggestion (testing):** Add tests for `resolve_changed_releases` handling newly added packages (missing previous version entries).
`resolve_changed_releases` is only tested with a fully populated `previous` mapping. Because it uses `previous_versions.get(package)`, new packages without an entry should be treated as `None` and published. Please add a test where one package is omitted from `previous`, verify that it’s selected for release, and confirm the dependency order is preserved. This will help prevent regressions in how missing entries are handled.
```suggestion
def test_unchanged_versions_do_not_publish(self) -> None:
previous = {package: self._version(package) for package in PACKAGE_CONFIG}
self.assertEqual(resolve_changed_releases(previous), ())
def test_new_package_without_previous_version_is_published(self) -> None:
# Omit one package from the previous mapping to simulate a newly added package
new_package = next(iter(PACKAGE_CONFIG))
previous = {
package: self._version(package)
for package in PACKAGE_CONFIG
if package != new_package
}
targets = resolve_changed_releases(previous)
target_packages = tuple(target.package for target in targets)
# The new package (missing in previous) should be selected for release
self.assertIn(new_package, target_packages)
# The result order should follow RELEASE_ORDER, restricted to the selected packages
expected_order = tuple(
package for package in RELEASE_ORDER if package == new_package
)
self.assertEqual(target_packages, expected_order)
def test_changed_versions_follow_dependency_order(self) -> None:
```
</issue_to_address>
### Comment 2
<location path="tests/test_release.py" line_range="76-85" />
<code_context>
+ def test_github_outputs_mark_only_selected_packages(self) -> None:
</code_context>
<issue_to_address>
**suggestion (testing):** Add coverage for `_write_github_outputs` when no targets are selected and for multiple selected targets.
The existing test only covers the case where a single package (`m7a`) is selected. Please add: (1) a test with `targets` empty, asserting `has_targets == "false"`, `packages` is empty, all `*_selected` flags are `false`, and `*_version`/`*_artifact` fields are empty; and (2) a test with multiple selected packages, asserting `packages` includes all selected packages in order and each package’s `*_selected`, `*_version`, and `*_artifact` values are set correctly. This will more completely validate the GitHub outputs the workflow relies on.
Suggested implementation:
```python
def test_github_outputs_mark_only_selected_packages(self) -> None:
previous = {package: self._version(package) for package in PACKAGE_CONFIG}
previous["automas-hsr-adapter-m7a"] = "previous-version"
targets = resolve_changed_releases(previous)
with TemporaryDirectory() as temp_dir:
output_path = Path(temp_dir) / "github-output.txt"
_write_github_outputs(targets, output_path)
values = dict(
line.split("=", 1)
for line in output_path.read_text(encoding="utf-8").splitlines()
)
# has_targets and packages reflect the single selected package
self.assertEqual(values["has_targets"], "true")
self.assertEqual(values["packages"], "automas-hsr-adapter-m7a")
# Only the selected package is marked as selected; all others are false
for package in PACKAGE_CONFIG:
key_prefix = package.replace("-", "_")
if package == "automas-hsr-adapter-m7a":
self.assertEqual(values[f"{key_prefix}_selected"], "true")
self.assertNotEqual(values[f"{key_prefix}_version"], "")
self.assertNotEqual(values[f"{key_prefix}_artifact"], "")
else:
self.assertEqual(values[f"{key_prefix}_selected"], "false")
self.assertEqual(values[f"{key_prefix}_version"], "")
self.assertEqual(values[f"{key_prefix}_artifact"], "")
def test_github_outputs_with_no_selected_targets(self) -> None:
previous = {package: self._version(package) for package in PACKAGE_CONFIG}
# Ensure there are no changed releases
targets = resolve_changed_releases(previous)
self.assertEqual(targets, ())
with TemporaryDirectory() as temp_dir:
output_path = Path(temp_dir) / "github-output.txt"
_write_github_outputs(targets, output_path)
values = dict(
line.split("=", 1)
for line in output_path.read_text(encoding="utf-8").splitlines()
)
# No targets selected
self.assertEqual(values["has_targets"], "false")
self.assertEqual(values["packages"], "")
# All packages are unselected and have empty version/artifact fields
for package in PACKAGE_CONFIG:
key_prefix = package.replace("-", "_")
self.assertEqual(values[f"{key_prefix}_selected"], "false")
self.assertEqual(values[f"{key_prefix}_version"], "")
self.assertEqual(values[f"{key_prefix}_artifact"], "")
def test_github_outputs_with_multiple_selected_targets(self) -> None:
# Pick two packages from PACKAGE_CONFIG to mark as changed
packages = list(PACKAGE_CONFIG.keys())
# Guard against pathological configs
self.assertGreaterEqual(len(packages), 2)
first, second = packages[0], packages[1]
previous = {package: self._version(package) for package in PACKAGE_CONFIG}
previous[first] = "previous-version-1"
previous[second] = "previous-version-2"
targets = resolve_changed_releases(previous)
with TemporaryDirectory() as temp_dir:
output_path = Path(temp_dir) / "github-output.txt"
_write_github_outputs(targets, output_path)
values = dict(
line.split("=", 1)
for line in output_path.read_text(encoding="utf-8").splitlines()
)
selected_packages = tuple(target.package for target in targets)
self.assertGreaterEqual(len(selected_packages), 2)
# The packages output lists all selected packages in order
self.assertEqual(
values["packages"],
",".join(selected_packages),
)
self.assertEqual(values["has_targets"], "true")
# Each selected package has *_selected true and non-empty version/artifact
selected_set = set(selected_packages)
for package in PACKAGE_CONFIG:
key_prefix = package.replace("-", "_")
if package in selected_set:
self.assertEqual(values[f"{key_prefix}_selected"], "true")
self.assertNotEqual(values[f"{key_prefix}_version"], "")
self.assertNotEqual(values[f"{key_prefix}_artifact"], "")
else:
self.assertEqual(values[f"{key_prefix}_selected"], "false")
self.assertEqual(values[f"{key_prefix}_version"], "")
self.assertEqual(values[f"{key_prefix}_artifact"], "")
```
These changes assume `_write_github_outputs` uses the following conventions:
1. A `has_targets` output of `"true"`/`"false"`.
2. A `packages` output that is a comma-separated list of package names matching `target.package`.
3. Per-package outputs named using `package.replace("-", "_")` as a prefix, e.g. `automas_hsr_adapter_m7a_selected`, `automas_hsr_adapter_m7a_version`, and `automas_hsr_adapter_m7a_artifact`, with `"true"`/`"false"` for `*_selected` and empty strings for unselected packages' `*_version`/`*_artifact`.
If your actual output naming or formatting differs (for example, if you use short names like `m7a_selected` or a different separator than a comma for `packages`), please adjust the key constructions and assertions accordingly to match the real `_write_github_outputs` behavior.
</issue_to_address>
### Comment 3
<location path="tests/test_release.py" line_range="7-13" />
<code_context>
+from tempfile import TemporaryDirectory
-from scripts.release import PACKAGE_CONFIG, project_version, resolve_release
+from scripts.release import (
+ PACKAGE_CONFIG,
+ RELEASE_ORDER,
+ _write_github_outputs,
+ project_version,
+ resolve_changed_releases,
+ resolve_release,
+)
</code_context>
<issue_to_address>
**issue (testing):** Consider adding tests for the new `resolve_release_plan` logic, especially error paths and automatic main releases.
Right now `resolve_release_plan` (automatic main-branch flow with `before_sha` plus manual/tag releases) isn’t covered by dedicated tests. Please add tests that:
1) Confirm `workflow_dispatch` and tag refs delegate to `resolve_release`.
2) Assert unsupported `event_name`/`ref` combinations raise the documented `ValueError`.
3) Check that missing or blank `before_sha` for `push` on `refs/heads/main` raises the correct error.
These will validate the new entry point and guard against misconfiguration.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+58
to
+62
| def test_unchanged_versions_do_not_publish(self) -> None: | ||
| previous = {package: self._version(package) for package in PACKAGE_CONFIG} | ||
| self.assertEqual(resolve_changed_releases(previous), ()) | ||
|
|
||
| def test_changed_versions_follow_dependency_order(self) -> None: |
There was a problem hiding this comment.
suggestion (testing): 添加针对 resolve_changed_releases 处理新加入包（缺少上一版本记录）的测试。
目前 resolve_changed_releases 仅在 previous 映射完整填充的情况下进行了测试。由于它使用的是 previous_versions.get(package)，对于在 previous 中没有条目的新包应该被视为 None 并进行发布。请添加一个测试，其中从 previous 中省略一个包，验证该包会被选中发布，并确认依赖顺序仍然被正确保持。这将有助于防止在处理缺失条目时出现回归问题。
Suggested change
| def test_unchanged_versions_do_not_publish(self) -> None: | |
| previous = {package: self._version(package) for package in PACKAGE_CONFIG} | |
| self.assertEqual(resolve_changed_releases(previous), ()) | |
| def test_changed_versions_follow_dependency_order(self) -> None: | |
| def test_unchanged_versions_do_not_publish(self) -> None: | |
| previous = {package: self._version(package) for package in PACKAGE_CONFIG} | |
| self.assertEqual(resolve_changed_releases(previous), ()) | |
| def test_new_package_without_previous_version_is_published(self) -> None: | |
| # Omit one package from the previous mapping to simulate a newly added package | |
| new_package = next(iter(PACKAGE_CONFIG)) | |
| previous = { | |
| package: self._version(package) | |
| for package in PACKAGE_CONFIG | |
| if package != new_package | |
| } | |
| targets = resolve_changed_releases(previous) | |
| target_packages = tuple(target.package for target in targets) | |
| # The new package (missing in previous) should be selected for release | |
| self.assertIn(new_package, target_packages) | |
| # The result order should follow RELEASE_ORDER, restricted to the selected packages | |
| expected_order = tuple( | |
| package for package in RELEASE_ORDER if package == new_package | |
| ) | |
| self.assertEqual(target_packages, expected_order) | |
| def test_changed_versions_follow_dependency_order(self) -> None: |
Original comment in English
suggestion (testing): Add tests for resolve_changed_releases handling newly added packages (missing previous version entries).
resolve_changed_releases is only tested with a fully populated previous mapping. Because it uses previous_versions.get(package), new packages without an entry should be treated as None and published. Please add a test where one package is omitted from previous, verify that it’s selected for release, and confirm the dependency order is preserved. This will help prevent regressions in how missing entries are handled.
Suggested change
| def test_unchanged_versions_do_not_publish(self) -> None: | |
| previous = {package: self._version(package) for package in PACKAGE_CONFIG} | |
| self.assertEqual(resolve_changed_releases(previous), ()) | |
| def test_changed_versions_follow_dependency_order(self) -> None: | |
| def test_unchanged_versions_do_not_publish(self) -> None: | |
| previous = {package: self._version(package) for package in PACKAGE_CONFIG} | |
| self.assertEqual(resolve_changed_releases(previous), ()) | |
| def test_new_package_without_previous_version_is_published(self) -> None: | |
| # Omit one package from the previous mapping to simulate a newly added package | |
| new_package = next(iter(PACKAGE_CONFIG)) | |
| previous = { | |
| package: self._version(package) | |
| for package in PACKAGE_CONFIG | |
| if package != new_package | |
| } | |
| targets = resolve_changed_releases(previous) | |
| target_packages = tuple(target.package for target in targets) | |
| # The new package (missing in previous) should be selected for release | |
| self.assertIn(new_package, target_packages) | |
| # The result order should follow RELEASE_ORDER, restricted to the selected packages | |
| expected_order = tuple( | |
| package for package in RELEASE_ORDER if package == new_package | |
| ) | |
| self.assertEqual(target_packages, expected_order) | |
| def test_changed_versions_follow_dependency_order(self) -> None: |
Comment on lines
+76
to
+85
| def test_github_outputs_mark_only_selected_packages(self) -> None: | ||
| previous = {package: self._version(package) for package in PACKAGE_CONFIG} | ||
| previous["automas-hsr-adapter-m7a"] = "previous-version" | ||
| targets = resolve_changed_releases(previous) | ||
|
|
||
| with TemporaryDirectory() as temp_dir: | ||
| output_path = Path(temp_dir) / "github-output.txt" | ||
| _write_github_outputs(targets, output_path) | ||
| values = dict( | ||
| line.split("=", 1) |
There was a problem hiding this comment.
suggestion (testing): 为 _write_github_outputs 增加在未选择任何目标以及选择多个目标时的测试覆盖。
现有测试只覆盖了单个包（m7a）被选中的情况。请补充：（1）targets 为空的测试，断言 has_targets == "false"，packages 为空，所有 *_selected 标记为 false，并且 *_version/*_artifact 字段为空；以及（2）多个包被选中的测试，断言 packages 按顺序包含所有选中包，并且每个包的 *_selected、*_version 和 *_artifact 值都被正确设置。这样可以更完整地验证该工作流依赖的 GitHub 输出。
建议的实现如下：
def test_github_outputs_mark_only_selected_packages(self) -> None:
previous = {package: self._version(package) for package in PACKAGE_CONFIG}
previous["automas-hsr-adapter-m7a"] = "previous-version"
targets = resolve_changed_releases(previous)
with TemporaryDirectory() as temp_dir:
output_path = Path(temp_dir) / "github-output.txt"
_write_github_outputs(targets, output_path)
values = dict(
line.split("=", 1)
for line in output_path.read_text(encoding="utf-8").splitlines()
)
# has_targets and packages reflect the single selected package
self.assertEqual(values["has_targets"], "true")
self.assertEqual(values["packages"], "automas-hsr-adapter-m7a")
# Only the selected package is marked as selected; all others are false
for package in PACKAGE_CONFIG:
key_prefix = package.replace("-", "_")
if package == "automas-hsr-adapter-m7a":
self.assertEqual(values[f"{key_prefix}_selected"], "true")
self.assertNotEqual(values[f"{key_prefix}_version"], "")
self.assertNotEqual(values[f"{key_prefix}_artifact"], "")
else:
self.assertEqual(values[f"{key_prefix}_selected"], "false")
self.assertEqual(values[f"{key_prefix}_version"], "")
self.assertEqual(values[f"{key_prefix}_artifact"], "")
def test_github_outputs_with_no_selected_targets(self) -> None:
previous = {package: self._version(package) for package in PACKAGE_CONFIG}
# Ensure there are no changed releases
targets = resolve_changed_releases(previous)
self.assertEqual(targets, ())
with TemporaryDirectory() as temp_dir:
output_path = Path(temp_dir) / "github-output.txt"
_write_github_outputs(targets, output_path)
values = dict(
line.split("=", 1)
for line in output_path.read_text(encoding="utf-8").splitlines()
)
# No targets selected
self.assertEqual(values["has_targets"], "false")
self.assertEqual(values["packages"], "")
# All packages are unselected and have empty version/artifact fields
for package in PACKAGE_CONFIG:
key_prefix = package.replace("-", "_")
self.assertEqual(values[f"{key_prefix}_selected"], "false")
self.assertEqual(values[f"{key_prefix}_version"], "")
self.assertEqual(values[f"{key_prefix}_artifact"], "")
def test_github_outputs_with_multiple_selected_targets(self) -> None:
# Pick two packages from PACKAGE_CONFIG to mark as changed
packages = list(PACKAGE_CONFIG.keys())
# Guard against pathological configs
self.assertGreaterEqual(len(packages), 2)
first, second = packages[0], packages[1]
previous = {package: self._version(package) for package in PACKAGE_CONFIG}
previous[first] = "previous-version-1"
previous[second] = "previous-version-2"
targets = resolve_changed_releases(previous)
with TemporaryDirectory() as temp_dir:
output_path = Path(temp_dir) / "github-output.txt"
_write_github_outputs(targets, output_path)
values = dict(
line.split("=", 1)
for line in output_path.read_text(encoding="utf-8").splitlines()
)
selected_packages = tuple(target.package for target in targets)
self.assertGreaterEqual(len(selected_packages), 2)
# The packages output lists all selected packages in order
self.assertEqual(
values["packages"],
",".join(selected_packages),
)
self.assertEqual(values["has_targets"], "true")
# Each selected package has *_selected true and non-empty version/artifact
selected_set = set(selected_packages)
for package in PACKAGE_CONFIG:
key_prefix = package.replace("-", "_")
if package in selected_set:
self.assertEqual(values[f"{key_prefix}_selected"], "true")
self.assertNotEqual(values[f"{key_prefix}_version"], "")
self.assertNotEqual(values[f"{key_prefix}_artifact"], "")
else:
self.assertEqual(values[f"{key_prefix}_selected"], "false")
self.assertEqual(values[f"{key_prefix}_version"], "")
self.assertEqual(values[f"{key_prefix}_artifact"], "")这些改动假设 _write_github_outputs 使用以下约定：
has_targets输出为"true"/"false"。packages输出为逗号分隔的包名列表，与target.package匹配。- 逐包的输出名称以
package.replace("-", "_")作为前缀，例如automas_hsr_adapter_m7a_selected、automas_hsr_adapter_m7a_version和automas_hsr_adapter_m7a_artifact，其中*_selected使用"true"/"false"，未选中包的*_version/*_artifact使用空字符串。
如果你实际的输出命名或格式不同（例如使用类似 m7a_selected 的短名称，或者在 packages 中使用不同的分隔符），请相应调整 key 构造和断言，以匹配真实的 _write_github_outputs 行为。
Original comment in English
suggestion (testing): Add coverage for _write_github_outputs when no targets are selected and for multiple selected targets.
The existing test only covers the case where a single package (m7a) is selected. Please add: (1) a test with targets empty, asserting has_targets == "false", packages is empty, all *_selected flags are false, and *_version/*_artifact fields are empty; and (2) a test with multiple selected packages, asserting packages includes all selected packages in order and each package’s *_selected, *_version, and *_artifact values are set correctly. This will more completely validate the GitHub outputs the workflow relies on.
Suggested implementation:
def test_github_outputs_mark_only_selected_packages(self) -> None:
previous = {package: self._version(package) for package in PACKAGE_CONFIG}
previous["automas-hsr-adapter-m7a"] = "previous-version"
targets = resolve_changed_releases(previous)
with TemporaryDirectory() as temp_dir:
output_path = Path(temp_dir) / "github-output.txt"
_write_github_outputs(targets, output_path)
values = dict(
line.split("=", 1)
for line in output_path.read_text(encoding="utf-8").splitlines()
)
# has_targets and packages reflect the single selected package
self.assertEqual(values["has_targets"], "true")
self.assertEqual(values["packages"], "automas-hsr-adapter-m7a")
# Only the selected package is marked as selected; all others are false
for package in PACKAGE_CONFIG:
key_prefix = package.replace("-", "_")
if package == "automas-hsr-adapter-m7a":
self.assertEqual(values[f"{key_prefix}_selected"], "true")
self.assertNotEqual(values[f"{key_prefix}_version"], "")
self.assertNotEqual(values[f"{key_prefix}_artifact"], "")
else:
self.assertEqual(values[f"{key_prefix}_selected"], "false")
self.assertEqual(values[f"{key_prefix}_version"], "")
self.assertEqual(values[f"{key_prefix}_artifact"], "")
def test_github_outputs_with_no_selected_targets(self) -> None:
previous = {package: self._version(package) for package in PACKAGE_CONFIG}
# Ensure there are no changed releases
targets = resolve_changed_releases(previous)
self.assertEqual(targets, ())
with TemporaryDirectory() as temp_dir:
output_path = Path(temp_dir) / "github-output.txt"
_write_github_outputs(targets, output_path)
values = dict(
line.split("=", 1)
for line in output_path.read_text(encoding="utf-8").splitlines()
)
# No targets selected
self.assertEqual(values["has_targets"], "false")
self.assertEqual(values["packages"], "")
# All packages are unselected and have empty version/artifact fields
for package in PACKAGE_CONFIG:
key_prefix = package.replace("-", "_")
self.assertEqual(values[f"{key_prefix}_selected"], "false")
self.assertEqual(values[f"{key_prefix}_version"], "")
self.assertEqual(values[f"{key_prefix}_artifact"], "")
def test_github_outputs_with_multiple_selected_targets(self) -> None:
# Pick two packages from PACKAGE_CONFIG to mark as changed
packages = list(PACKAGE_CONFIG.keys())
# Guard against pathological configs
self.assertGreaterEqual(len(packages), 2)
first, second = packages[0], packages[1]
previous = {package: self._version(package) for package in PACKAGE_CONFIG}
previous[first] = "previous-version-1"
previous[second] = "previous-version-2"
targets = resolve_changed_releases(previous)
with TemporaryDirectory() as temp_dir:
output_path = Path(temp_dir) / "github-output.txt"
_write_github_outputs(targets, output_path)
values = dict(
line.split("=", 1)
for line in output_path.read_text(encoding="utf-8").splitlines()
)
selected_packages = tuple(target.package for target in targets)
self.assertGreaterEqual(len(selected_packages), 2)
# The packages output lists all selected packages in order
self.assertEqual(
values["packages"],
",".join(selected_packages),
)
self.assertEqual(values["has_targets"], "true")
# Each selected package has *_selected true and non-empty version/artifact
selected_set = set(selected_packages)
for package in PACKAGE_CONFIG:
key_prefix = package.replace("-", "_")
if package in selected_set:
self.assertEqual(values[f"{key_prefix}_selected"], "true")
self.assertNotEqual(values[f"{key_prefix}_version"], "")
self.assertNotEqual(values[f"{key_prefix}_artifact"], "")
else:
self.assertEqual(values[f"{key_prefix}_selected"], "false")
self.assertEqual(values[f"{key_prefix}_version"], "")
self.assertEqual(values[f"{key_prefix}_artifact"], "")These changes assume _write_github_outputs uses the following conventions:
- A
has_targetsoutput of"true"/"false". - A
packagesoutput that is a comma-separated list of package names matchingtarget.package. - Per-package outputs named using
package.replace("-", "_")as a prefix, e.g.automas_hsr_adapter_m7a_selected,automas_hsr_adapter_m7a_version, andautomas_hsr_adapter_m7a_artifact, with"true"/"false"for*_selectedand empty strings for unselected packages'*_version/*_artifact.
If your actual output naming or formatting differs (for example, if you use short names like m7a_selected or a different separator than a comma for packages), please adjust the key constructions and assertions accordingly to match the real _write_github_outputs behavior.
Comment on lines
+7
to
+13
| from scripts.release import ( | ||
| PACKAGE_CONFIG, | ||
| RELEASE_ORDER, | ||
| _write_github_outputs, | ||
| project_version, | ||
| resolve_changed_releases, | ||
| resolve_release, |
There was a problem hiding this comment.
issue (testing): 请考虑为新的 resolve_release_plan 逻辑添加测试，尤其是错误路径以及自动 main 分支发布的相关场景。
目前 resolve_release_plan（在 main 分支上基于 before_sha 的自动流程，外加手动/标签发布）还没有专门测试覆盖。请添加以下测试：
1）确认 workflow_dispatch 和标签 ref 会委托给 resolve_release。
2）断言不支持的 event_name/ref 组合会抛出已记录的 ValueError。
3）检查在 refs/heads/main 上 push 时，缺失或为空的 before_sha 会抛出正确的错误。
这些测试将验证新的入口逻辑，并防止配置错误。
Original comment in English
issue (testing): Consider adding tests for the new resolve_release_plan logic, especially error paths and automatic main releases.
Right now resolve_release_plan (automatic main-branch flow with before_sha plus manual/tag releases) isn’t covered by dedicated tests. Please add tests that:
- Confirm
workflow_dispatchand tag refs delegate toresolve_release. - Assert unsupported
event_name/refcombinations raise the documentedValueError. - Check that missing or blank
before_shaforpushonrefs/heads/mainraises the correct error.
These will validate the new entry point and guard against misconfiguration.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Validation
No package version file is changed by this PR, so merging it does not publish a release.
Summary by Sourcery
在
main分支上，当四个发行包的pyproject版本发生变化时，自动发布到 PyPI，并在保持手动/基于标签的恢复路径的同时，强制执行依赖安全的发布顺序。New Features:
pyproject.toml文件在main上发生变化时触发发布工作流，自动选择相较于上一提交版本号发生变化的发行包。core → SRA adapter → M7A adapter → meta。automas-script-hsr、SRA adapter、M7A adapter 和automas-hsr添加按包划分的发布任务和环境，使用共享构建任务生成的工件以及 PyPI Trusted Publishing。Bug Fixes:
main提交必须有版本变更并跳过未变化的发行包，防止非预期发布。Enhancements:
release.py，以便为手动/标签触发和自动main推送两种情况计算发布计划，并输出结构化的 GitHub 输出，描述将发布哪些包。Documentation:
AGENTS.md中记录基于版本变化的自动发布、发行包到pyproject文件的映射以及发布顺序规则。Tests:
Original summary in English
Summary by Sourcery
Automate PyPI publishing for the four distributions on main when their pyproject versions change, and enforce a dependency-safe release order while preserving manual/tag-based recovery paths.
New Features:
Bug Fixes:
Enhancements:
Documentation:
Tests: