Skip to content
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

Check Sphinx build log in CI #55

Merged
merged 1 commit into from
Apr 18, 2024
Merged

Check Sphinx build log in CI #55

merged 1 commit into from
Apr 18, 2024

Conversation

mwoehlke
Copy link
Member

@mwoehlke mwoehlke commented Mar 21, 2024

Add a script to parse a Sphinx build log and turn it into GitHub Actions formatted messages. Add ability to specify a build log output location to Makefile. Use these to scrape the Sphinx build log in order to report any warnings. Also, propagate the result to another job, which allows the 'build' job to pass with warnings, while the 'check' job will fail.

@mwoehlke mwoehlke force-pushed the scrape-sphinx-logs branch 5 times, most recently from e9a22e0 to 7e2705b Compare March 21, 2024 18:25
Add a script to parse a Sphinx build log and turn it into GitHub Actions
formatted messages. Add ability to specify a build log output location
to Makefile. Use these to scrape the Sphinx build log in order to report
any warnings. Also, propagate the result to another job, which allows
the 'build' job to pass with warnings, while the 'check' job will fail.
@mwoehlke mwoehlke changed the title WIP: Check Sphinx build log in CI Check Sphinx build log in CI Mar 21, 2024
@mwoehlke
Copy link
Member Author

Okay, as far as I can tell, this "works"... mostly. The "check" job fails if there are warnings, and warnings show up in the Files Changed" tab... but not in the Checks panel in the Conversation tab. I don't know if GitHub removed that feature, or what. In any case, having the "check" job that fails if there are warnings is an improvement.

Copy link
Collaborator

@bretbrownjr bretbrownjr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

The check-build-logs.py parsing logic is a little "magic" to me. It would be nice to break the parsing and output file creation into different functions. That would allow some self-contained testing like doctest to quickly make sure our parsing is both documented and behaving as expected.

I won't hold up this PR on that, though. Making sure Sphinx warnings are exposed to CI is important, but not core to the goals of a CPS spec as such.

@mwoehlke
Copy link
Member Author

The check-build-logs.py parsing logic is a little "magic" to me. It would be nice to break the parsing and output file creation into different functions.

Yes, it's a combination of log scraping and mangling diagnostics into the magic format used to report to GHA. I'm not sure how you'd split it; generate an intermediate file (e.g. JSON) from log scraping and have a separate script to turn that into what GHA wants?

@mwoehlke mwoehlke merged commit 454cda8 into master Apr 18, 2024
9 checks passed
@mwoehlke mwoehlke deleted the scrape-sphinx-logs branch April 18, 2024 17:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants