Feature/GitHub summary #257
Conversation
mrVanDalo
force-pushed
the
feature/github-summary
branch
from
7621fbb to
74b278d
Compare
|
|
||
| def test_github_summary_without_env() -> None: | ||
| env = {k: v for k, v in os.environ.items() if k != "GITHUB_STEP_SUMMARY"} | ||
| with patch.dict(os.environ, env, clear=True): |
There was a problem hiding this comment.
There is a pytest fixture in "monkeypatch" that does that.
| await asyncio.to_thread(write_result_file) | ||
|
|
||
| if opts.github_summary: | ||
| github_summary_path_str = os.environ.get("GITHUB_STEP_SUMMARY") |
There was a problem hiding this comment.
Don't think we need a flag, we can just test for the presents for GITHUB_STEP_SUMMARY.
| results, # type: ignore[arg-type] | ||
| ) | ||
|
|
||
| await asyncio.to_thread(write_result_file) |
There was a problem hiding this comment.
Don't get why this was moved to a thread.
There was a problem hiding this comment.
claude fixed that. Because of some linting problems. I don't really know.
| nonlocal rc | ||
| rc = 1 | ||
|
|
||
| await asyncio.to_thread(write_github_summary) |
There was a problem hiding this comment.
Same here. Also this is blocking io, it happens after we are done with everything. So we are blocking anyway.
| def test_github_summary_with_env() -> None: | ||
| with TemporaryDirectory() as d: | ||
| path = Path(d) / "summary.md" | ||
| with patch.dict(os.environ, {"GITHUB_STEP_SUMMARY": str(path)}): |
There was a problem hiding this comment.
Same here. We also only need this test and not the negative one, because we can just unset the environment variable in one of the existing tests.
There was a problem hiding this comment.
So we should default to GITHUB_STEP_SUMMARY present? and add a parameter to overwrite this environment variable, and look for this environment variable instead if present, and than write output to this file?
e.g.: --step-summary-variable FORGEJO_STEP_SUMMARY will check if FORGEJO_STEP_SUMMARY is set and a path to a file , ... ?
There was a problem hiding this comment.
Mhm. To make it simpler for the user, can we not also detect gitea/forgeo as well?
If we really need to make this configurable we can do something like --ci-summary auto|off.
|
Added in #239 |
2 participants
Closes #222
Mostly Vibe coded using claude
This would be the contents of $GITHUB_STEP_SUMMARY when running
./result/bin/nix-fast-build --github-summaryin this repository