-
Notifications
You must be signed in to change notification settings - Fork 23
Add GitHub Actions job summary support with build logs #239
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
Important Review skippedBot user detected. To trigger a single review, invoke the You can disable this status message by setting the Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. Comment |
- Add --github-summary CLI argument for custom output file - Auto-detect GitHub Actions environment via GITHUB_ACTIONS env var - Capture stderr output from failed builds - Generate markdown summary with failed builds section - Include build logs in collapsible details sections - Add comprehensive tests for GitHub summary functionality Co-authored-by: Mic92 <[email protected]>
- Add new section explaining GitHub Actions job summaries - Document automatic detection and usage - Include --github-summary flag in reference section Co-authored-by: Mic92 <[email protected]>
|
Builder: github:Mic92/nix-fast-build#checks.aarch64-darwin.treefmt |
- Test complete workflow with environment variables - Test explicit flag overriding environment variables - Test log truncation for very long outputs - All tests passing with full type checking Co-authored-by: Mic92 <[email protected]>
nix_fast_build/__init__.py
Outdated
| help="Override a specific flake input (e.g. `dwarffs/nixpkgs`).", | ||
| ) | ||
| parser.add_argument( | ||
| "--github-summary", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot env variable is enough here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done. Removed the --github-summary CLI argument. Now uses only GITHUB_STEP_SUMMARY environment variable. (c4b7308)
nix_fast_build/__init__.py
Outdated
| ) | ||
|
|
||
| # Capture stderr if available | ||
| if proc.stderr: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot use nix log here instead once a build failed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done. Changed to use nix log command to fetch build logs after build failure instead of capturing stderr during build. (c4b7308)
tests/test_github_integration.py
Outdated
|
|
||
|
|
||
| def test_explicit_github_summary_overrides_env() -> None: | ||
| """Test that explicit --github-summary flag overrides environment variable.""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot remove.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done. Removed test_explicit_github_summary_overrides_env function. (c4b7308)
tests/test_github_summary.py
Outdated
| from pathlib import Path | ||
| from tempfile import TemporaryDirectory | ||
|
|
||
| from nix_fast_build import ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot not needed the one integration test is enough.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done. Removed tests/test_github_summary.py entirely. Kept only the integration test. (c4b7308)
README.md
Outdated
| File to write build results to | ||
| --result-format {json,junit} | ||
| Format of the build result file | ||
| --github-summary GITHUB_SUMMARY |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot drop
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done. Removed --github-summary flag documentation from reference section. (c4b7308)
README.md
Outdated
| [--eval-workers EVAL_WORKERS] | ||
| [--result-file RESULT_FILE] | ||
| [--result-format {json,junit}] | ||
| [--github-summary GITHUB_SUMMARY] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot drop
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done. Removed "GitHub Actions Job Summaries" section from README. (c4b7308)
- Remove --github-summary CLI argument (use GITHUB_STEP_SUMMARY only) - Use nix log to fetch build logs after failure instead of capturing stderr - Remove test_github_summary.py (keep only integration test) - Remove test_explicit_github_summary_overrides_env - Revert README documentation about --github-summary flag - Fix ruff linting issues Co-authored-by: Mic92 <[email protected]>
|
Builder: github:Mic92/nix-fast-build#checks.aarch64-darwin.treefmt ")
treefmt-check> lines.append(f" Build Log ({len(log_lines)} lines)\n")treefmt-check> lines.append("```") treefmt-check> diff --git a/tests/test_github_integration.py b/tests/test_github_integration.py treefmt-check> index 81c2c25..ee9ca6c 100644 treefmt-check> --- a/tests/test_github_integration.py treefmt-check> +++ b/tests/test_github_integration.py treefmt-check> @@ -1,4 +1,5 @@ treefmt-check> """Integration test for GitHub Actions summary feature.""" treefmt-check> + treefmt-check> import os treefmt-check> from pathlib import Path treefmt-check> from tempfile import TemporaryDirectory treefmt-check> @@ -26,7 +27,9 @@ def test_github_actions_workflow() -> None: treefmt-check> os.environ["GITHUB_STEP_SUMMARY"] = str(summary_path) treefmt-check> treefmt-check> # Create options treefmt-check> - opts = Options(flake_url="github:example/repo", flake_fragment="checks.x86_64-linux") treefmt-check> + opts = Options( treefmt-check> + flake_url="github:example/repo", flake_fragment="checks.x86_64-linux" treefmt-check> + ) treefmt-check> treefmt-check> # Verify it picks up the environment variable treefmt-check> github_summary_file = get_github_summary_file() treefmt-check> @@ -145,6 +148,8 @@ def test_long_log_truncation() -> None: treefmt-check> assert "truncated, showing last 100 lines" in content treefmt-check> treefmt-check> # Verify only last 100 lines + truncation message are present treefmt-check> - assert "log line 50" in content # Should be present (line 50 onwards, first of last 100) treefmt-check> + assert ( treefmt-check> + "log line 50" in content treefmt-check> + ) # Should be present (line 50 onwards, first of last 100) treefmt-check> assert "log line 49" not in content # Should be truncated treefmt-check> assert "log line 149" in content # Last line should be present error: build of '/nix/store/s4x41s2264647n8xg4vl6r0spy2hz7ry-treefmt-check.drv' on 'ssh-ng://customer@mac02' failed: builder for '/nix/store/s4x41s2264647n8xg4vl6r0spy2hz7ry-treefmt-check.drv' failed with exit code 1; last 25 log lines: > + > import os > from pathlib import Path > from tempfile import TemporaryDirectory > @@ -26,7 +27,9 @@ def test_github_actions_workflow() -> None: > os.environ["GITHUB_STEP_SUMMARY"] = str(summary_path) > > # Create options > - opts = Options(flake_url="github:example/repo", flake_fragment="checks.x86_64-linux") > + opts = Options( > + flake_url="github:example/repo", flake_fragment="checks.x86_64-linux" > + ) > > # Verify it picks up the environment variable > github_summary_file = get_github_summary_file() > @@ -145,6 +148,8 @@ def test_long_log_truncation() -> None: > assert "truncated, showing last 100 lines" in content > > # Verify only last 100 lines + truncation message are present > - assert "log line 50" in content # Should be present (line 50 onwards, first of last 100) > + assert ( > + "log line 50" in content > + ) # Should be present (line 50 onwards, first of last 100) > assert "log line 49" not in content # Should be truncated > assert "log line 149" in content # Last line should be present For full logs, run: nix log /nix/store/s4x41s2264647n8xg4vl6r0spy2hz7ry-treefmt-check.drv error: Cannot build '/nix/store/s4x41s2264647n8xg4vl6r0spy2hz7ry-treefmt-check.drv'. Reason: builder failed with exit code 1. Output paths: /nix/store/yqimdyx0kybx046lvs3qqa4m845mpavl-treefmt-check program finished with exit code 1 elapsedTime=4.170667 |
- Add blank line after module docstring - Format long lines to match formatter expectations - Split multi-line list comprehension for readability - Format Options call across multiple lines Co-authored-by: Mic92 <[email protected]>
nix_fast_build/__init__.py
Outdated
| async def build( | ||
| self, stack: AsyncExitStack, build_output: IO[str], opts: Options | ||
| ) -> int: | ||
| ) -> tuple[int, str]: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot make this a dataclass
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done. Created BuildResult dataclass to replace the tuple return type. (9cb1c16)
|
|
||
| def is_github_actions() -> bool: | ||
| """Detect if running inside GitHub Actions.""" | ||
| return os.environ.get("GITHUB_ACTIONS") == "true" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot still document this behaviour in the readme.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done. Added "GitHub Actions Job Summaries" section to README documenting the automatic detection and build log behavior. (9cb1c16)
|
@Mic92 Unfortunately I hit an unexpected error while processing your comment. I've automatically reported this to GitHub. You can ask me to try again later by mentioning me in a new comment. If you want to contact GitHub about this error, please mention the following identifier so they can better serve you: Sorry for the inconvenience! |
- Create BuildResult dataclass to replace tuple return type - Update Build.build() to return BuildResult instead of tuple - Add GitHub Actions Job Summaries section to README - Document automatic detection and build log behavior Co-authored-by: Mic92 <[email protected]>
|
why do we get counts for stuff that hasn't be done? in our self-test we don't have cachix/attic: Type Successes Failures |
- Don't add UPLOAD results when copy_to is not configured - Don't add CACHIX results when cachix_cache is not configured - Don't add ATTIC results when attic_cache is not configured - Don't add DOWNLOAD results when remote or download is disabled - Also filter out result types with 0 successes and 0 failures from summary table This prevents showing misleading counts for operations that weren't actually performed. Co-authored-by: Mic92 <[email protected]>
Fixed. Operations that aren't configured (CACHIX, ATTIC, UPLOAD, DOWNLOAD) now skip adding result entries entirely, so they won't appear in the summary table. The summary will only show operation types that actually ran. (e1d3629) |
Original prompt
Fixes #222
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.