Repository navigation
ci(quality_runners): route module test and coverage logs to artifacts (#140) - #376
srinivasugithub wants to merge 10 commits into
Conversation
…dently and clean up type annotations
|
The created documentation from the pull request is available at: docu-html |
| deps = [ | ||
| ":known_good", | ||
| "//scripts:quality_runners", | ||
| ], |
There was a problem hiding this comment.
this looks like a total mixup now. You added tests to quality runners but placed them in known_good and referenced everything.
IMO test_quality_runners_logging should be placed next to quality runners and get its own test target
There was a problem hiding this comment.
Fixed. Moved test_quality_runners_logging.py to scripts/tests/, defined a dedicated //scripts:quality_runners_tests target in scripts/BUILD, and added it to .github/workflows/internal_tests.yml. Reverted
scripts/known_good/BUILD so known_good_tests no longer depends on quality_runners
| try: | ||
| from known_good.models.known_good import load_known_good | ||
| from known_good.models.module import Module | ||
| except ModuleNotFoundError: | ||
| from scripts.known_good.models.known_good import load_known_good | ||
| from scripts.known_good.models.module import Module |
There was a problem hiding this comment.
I think cleaning up test targets will allow to get rid of that 2 way imports
There was a problem hiding this comment.
Fixed. With the dedicated test target and imports = ["."] configured on //scripts:quality_runners, the try...except ModuleNotFoundError fallback was removed in favor of standard imports from known_good.models.
…mports - Relocate test_quality_runners_logging.py to scripts/tests/ - Define dedicated //scripts:quality_runners_tests Bazel test target - Wire //scripts:quality_runners_tests into internal_tests CI workflow - Remove //scripts:quality_runners dependency from known_good_tests - Remove try/except ModuleNotFoundError import fallback in quality_runners.py
There was a problem hiding this comment.
🟡 Changes recommended
Integration-test output is unintentionally suppressed without being saved to an artifact.
1 open finding
What changed in this PR
Routes quality-runner output into per-module artifacts to reduce CI log volume while retaining failure diagnostics.
Changes:
- Adds configurable log capture, verbosity, and failure tails.
- Adds tests and Bazel targets for logging behavior.
- Uploads logs as CI artifacts and runs the new tests internally.
| File | Description |
|---|---|
scripts/quality_runners.py |
Implements logging and CLI options. |
scripts/tests/test_quality_runners_logging.py |
Tests logging and routing behavior. |
scripts/BUILD |
Defines library and test targets. |
.github/workflows/test_and_docs.yml |
Uploads module logs. |
.github/workflows/internal_tests.yml |
Runs the new Bazel test target. |
🧠 Review effort: Balanced
Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.
| command: list[str], | ||
| log_file: Path | None = None, | ||
| *, | ||
| verbose: bool = False, |
Resolves #140
Currently,
run_command()inscripts/quality_runners.pystreams all raw compiler, Bazel execution, and test analysis output directly to stdout/stderr. During CI runs, this produces tens of thousands of lines of output, making job logs difficult to read and hitting CI log limits.Changes
scripts/quality_runners.py):run_command()withlog_file,verbose, andtail_on_failureparameters.artifacts/logs/<module_name>.log), keeping stdout clean and concise.--log-output-dir(default:artifacts/logs/) and--verboseflags.exit_code != 0), up totail_on_failure(default 30) lines of bothstderrandstdoutare independently printed to the console so failures can be diagnosed immediately in CI without downloading log files..github/workflows/test_and_docs.yml):actions/upload-artifact@v4) right after unit test execution withif: always()andif-no-files-found: warnto preserve module log files across successful and failing runs.scripts/quality_runners.pyas//scripts:quality_runnersinscripts/BUILD.//scripts:quality_runnersinto//scripts/known_good:known_good_tests.scripts/known_good/tests/test_quality_runners_logging.pycovering stream capture, directory creation, failure tailing, argument parsing, and runner routing.Verification
PYTHONPATH=. pytest scripts/known_good/tests -v: 132/132 passed.bazel test //scripts/tooling:tooling_tests //scripts/known_good:known_good_tests: both passed.ruff check scripts/&ruff format --check scripts/: all checks passed.bazel run //:format.check_Starlark_with_buildifier: passed.pre-commit run copyright --all-files: passed.python3 ./scripts/quality_runners.py --modules-to-test score_kyron --trust-cache: verified clean console prints and full log capture inartifacts/logs/score_kyron.log.