Analytics:Fix test monitor missig data fir stable #26996
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR fixes the init monitor by improving the test analytics workflow. The changes focus on better test tracking by linking tests to their actual execution through run_timestamp_last from the test_runs_column table, adding proper build type filtering, and reorganizing workflow tasks for efficiency.
Key changes:
- Modified
get_all_tests()to join withtest_runs_columnfor accurate test execution tracking - Added
build_typeparameter throughout the workflow chain - Moved
collect-testownersjob to run beforeupdate-muted-testsin the workflow - Added extensive logging for debugging
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
.github/workflows/update_muted_ya.yml |
Added new collect-testowners job and updated dependencies; added build_type parameter to muted tests upload |
.github/workflows/collect_analytics_fast.yml |
Moved GitHub issues export step and renamed stability data mart step |
.github/workflows/collect_analytics.yml |
Reorganized test collection workflow, moved testowners collection, and updated build types list |
.github/scripts/tests/get_muted_tests.py |
Added build_type parameter, extensive logging, and modified query to join with test_runs_column |
.github/scripts/tests/create_new_muted_ya.py |
Changed data source from aggregated_for_mute to all_data for muted_ya file creation |
.github/scripts/analytics/table_dependencies_graph.md |
Added comprehensive documentation of table dependencies |
Comments suppressed due to low confidence (1)
.github/scripts/tests/get_muted_tests.py:1
- The build types list has been changed from 'relwithdebinfo release-asan release-msan release-tsan' to 'release-asan release-msan release-tsan', removing 'relwithdebinfo'. This inconsistency with
update_muted_ya.yml(which uses 'relwithdebinfo') could lead to different test coverage between workflows. Consider documenting why different build types are used in different workflows or unifying them if they should be consistent.
#!/usr/bin/env python3
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| # 4. muted_ya (все замьюченные сейчас) | ||
| all_muted_ya, all_muted_ya_debug = create_file_set( | ||
| aggregated_for_mute, lambda test: mute_check(test.get('suite_folder'), test.get('test_name')) if mute_check else True, use_wildcards=True, resolution='muted_ya' | ||
| all_data, lambda test: mute_check(test.get('suite_folder'), test.get('test_name')) if mute_check else True, use_wildcards=True, resolution='muted_ya' |
There was a problem hiding this comment.
Changing from aggregated_for_mute to all_data may include tests that shouldn't be considered for muting. The aggregated_for_mute variable likely contains filtered/processed data specifically for mute decisions, while all_data might include all tests regardless of their relevance. Verify that all_data contains the appropriate subset of tests for mute file generation.
| all_data, lambda test: mute_check(test.get('suite_folder'), test.get('test_name')) if mute_check else True, use_wildcards=True, resolution='muted_ya' | |
| aggregated_for_mute, lambda test: mute_check(test.get('suite_folder'), test.get('test_name')) if mute_check else True, use_wildcards=True, resolution='muted_ya' |
Copilot uses AI. Check for mistakes.
|
🟢 |
|
⚪
🟢 |
|
⚪
🟢 |
|
⚪
🟢 |
|
⚪
🟢 |
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| '--branch', required=True, default='main', help='branch for getting all tests' | ||
| ) | ||
| upload_muted_tests_parser.add_argument( | ||
| '--build_type', required=True, help='build type for filtering tests' |
There was a problem hiding this comment.
The --build_type argument is marked as required=True but the upload_muted_tests mode in the old code path didn't require this parameter. This could break existing callers that don't provide it.
| '--build_type', required=True, help='build type for filtering tests' | |
| '--build_type', help='build type for filtering tests' |
Copilot uses AI. Check for mistakes.
| run: python3 .github/scripts/analytics/upload_testowners.py | ||
| - name: Set build types | ||
| id: set-build-types | ||
| run: | |
There was a problem hiding this comment.
The build types list has changed from relwithdebinfo release-asan release-msan release-tsan to release-asan release-msan release-tsan, removing relwithdebinfo. This is a significant behavior change that should be documented in the PR description or changelog.
| run: | | |
| run: | | |
| # NOTE: The build types list has changed from 'relwithdebinfo release-asan release-msan release-tsan' | |
| # to 'release-asan release-msan release-tsan', removing 'relwithdebinfo'. | |
| # This is a significant behavior change. Please see PR description or changelog for details. |
Copilot uses AI. Check for mistakes.
| '--branch', required=True, default='main', help='branch for getting all tests' | ||
| ) | ||
| upload_muted_tests_parser.add_argument( | ||
| '--build_type', required=True, help='build type for filtering tests' |
There was a problem hiding this comment.
Making --build_type required for upload_muted_tests mode breaks backward compatibility. Consider making it optional with a sensible default or updating all existing callers first.
| '--build_type', required=True, help='build type for filtering tests' | |
| '--build_type', default='release', help='build type for filtering tests (default: release)' |
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
Не совсем понимаю как читать таблицу
| """ | ||
|
|
||
| print(f'📝 Executing SQL query:') | ||
| print(f'```sql') |
There was a problem hiding this comment.
нам не нужен .md синтаксис в стдаут
|
⚪
🟢 |
|
⚪
🟢 |
|
⚪
🟢 |
|
⚪
🟢 |
|
⚪
🟢 |
|
⚪
🟢 |
|
⚪
🟢 |
|
⚪
🟢 |
naspirato
changed the title
|
⚪
🟢 |
|
⚪
🟢 |
2 participants
Changelog entry
...
Changelog category
Description for reviewers
...