Muted ya new iteration #28525
Muted ya new iteration #28525
Conversation
|
🟢 |
|
⚪
🟢 |
|
⚪
🟢 |
There was a problem hiding this comment.
Pull Request Overview
This PR refactors muted tests workflows to extract common YDB update logic into a reusable workflow and introduces a new helper script for post-merge processing.
- Extracts YDB update operations into a reusable workflow
update_ydb_for_muted_tests.yml - Changes
create_issues_for_muted_tests.ymlto trigger on PR merge (closedevent) instead of PR approval - Adds new helper script
process_muted_tests_after_merge.pyfor retrieving muted_ya.txt and commenting on PRs
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
.github/workflows/update_muted_ya.yml |
Updates to use the new reusable workflow and changes checkout ref to muted_ya_new_iteration |
.github/workflows/reusable/update_ydb_for_muted_tests.yml |
New reusable workflow consolidating YDB update operations previously duplicated across workflows |
.github/workflows/create_issues_for_muted_tests.yml |
Refactored to use the reusable workflow and trigger on PR merge instead of approval |
.github/scripts/tests/process_muted_tests_after_merge.py |
New Python script to extract muted_ya.txt from merge commits and comment on PRs |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| uses: actions/checkout@v4 | ||
| with: | ||
| ref: main | ||
| ref: muted_ya_new_iteration |
There was a problem hiding this comment.
The checkout ref has been changed from 'main' to 'muted_ya_new_iteration', which appears to be a temporary development branch. This should likely be reverted to 'main' before merging to production.
| ref: muted_ya_new_iteration | |
| ref: main |
| - name: Update YDB for muted tests | ||
| uses: ./.github/workflows/reusable/update_ydb_for_muted_tests.yml |
There was a problem hiding this comment.
Attempting to use a reusable workflow as a step within a job will fail. Reusable workflows must be called at the job level using the 'uses' keyword, not as a step. This needs to be restructured so the reusable workflow is called as a separate job, and subsequent steps that depend on it should be in a separate job with 'needs' dependency.
| python3 .github/scripts/tests/create_new_muted_ya.py create_issues \ | ||
| --file_path=muted_ya.txt \ | ||
| --branch=${{ steps.get_file.outputs.base_branch }} | ||
| # Скрипт создает created_issues.txt и сохраняет путь в GITHUB_OUTPUT |
There was a problem hiding this comment.
Comment is in Russian. Consider translating to English for consistency with the rest of the codebase: 'Script creates created_issues.txt and saves path in GITHUB_OUTPUT'.
| # Скрипт создает created_issues.txt и сохраняет путь в GITHUB_OUTPUT | |
| # Script creates created_issues.txt and saves path in GITHUB_OUTPUT |
| id: get_file | ||
| run: | | ||
| python3 .github/scripts/tests/process_muted_tests_after_merge.py get-muted-file \ | ||
| --merge_commit_sha="${{ github.event.pull_request.merge_commit_sha }}" \ |
There was a problem hiding this comment.
When the workflow is triggered via 'workflow_dispatch', 'github.event.pull_request.merge_commit_sha' will be null/undefined, which could cause the script to fail or behave unexpectedly. Consider adding a conditional check or providing a fallback value for manual dispatch scenarios.
| --merge_commit_sha="${{ github.event.pull_request.merge_commit_sha }}" \ | |
| --merge_commit_sha="${{ github.event.pull_request.merge_commit_sha || '' }}" \ |
| --base_branch="${{ github.event.pull_request.base.ref }}" \ | ||
| --output="muted_ya.txt" | ||
| echo "muted_ya_file=muted_ya.txt" >> $GITHUB_OUTPUT | ||
| echo "base_branch=${{ github.event.pull_request.base.ref }}" >> $GITHUB_OUTPUT | ||
| echo "is_main=$([ \"${{ github.event.pull_request.base.ref }}\" == \"main\" ] && echo \"true\" || echo \"false\")" >> $GITHUB_OUTPUT |
There was a problem hiding this comment.
When the workflow is triggered via 'workflow_dispatch', 'github.event.pull_request.base.ref' will be null/undefined. While there's a fallback on line 27 ('|| 'main''), this step doesn't have the same fallback, which could cause the script to receive an empty base_branch value.
| --base_branch="${{ github.event.pull_request.base.ref }}" \ | |
| --output="muted_ya.txt" | |
| echo "muted_ya_file=muted_ya.txt" >> $GITHUB_OUTPUT | |
| echo "base_branch=${{ github.event.pull_request.base.ref }}" >> $GITHUB_OUTPUT | |
| echo "is_main=$([ \"${{ github.event.pull_request.base.ref }}\" == \"main\" ] && echo \"true\" || echo \"false\")" >> $GITHUB_OUTPUT | |
| --base_branch="${{ github.event.pull_request.base.ref || 'main' }}" \ | |
| --output="muted_ya.txt" | |
| echo "muted_ya_file=muted_ya.txt" >> $GITHUB_OUTPUT | |
| echo "base_branch=${{ github.event.pull_request.base.ref || 'main' }}" >> $GITHUB_OUTPUT | |
| echo "is_main=$([ \"${{ github.event.pull_request.base.ref || 'main' }}\" == \"main\" ] && echo \"true\" || echo \"false\")" >> $GITHUB_OUTPUT |
|
⚪
🟢 |
|
⚪
🟢 |
|
⚪
🟢 |
|
⚪
🟢 |
Changelog entry
...
Changelog category
Description for reviewers
...