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

Unit Test failures now output a job summary, making it easier to see what the failures are. #5182

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

LikeLakers2
Copy link
Contributor

This change was requested by Absolucy, of catgirl fame.

About The Pull Request

The CI is very useful for figuring out if you made any errors with your changes. Among other things, it runs unit tests on a variety of maps and BYOND versions, making sure everything still works as expected.

Unfortunately, figuring out the specific errors you got in CI is a bit of a pain. You could scroll through the individual logs - but the logs are so long, it'd take forever.

You could also use the annotations section, but that's gets filled with generic error messages:

image

This PR helps on this matter, by adding a job summary whenever a failure is detected. Here's an example where I intentionally generated fails:

image

(If you want to see how it looks on your own screen, have a look: https://github.com/LikeLakers2/tgstation/actions/runs/13026959405)

As it is set up currently, each integration test job will generate its own summary; each summary contains one section per failed test; each section will contain the associated failure message(s) in a code block.

This system is pretty basic currently (for example, breaking something across all maps will cause many job summaries to be generated), but it's a step up from what we had previously. Ideally, I'll come back to this later, and improve on the job summary output - for example, by merging identical errors across maps, and having the failures generated as a singular job summary.

Why It's Good For The Game

Having all the failures summarized in one place, without generic failure messages splattered in-between, can make it easier to diagnose issues detected by CI.

@LikeLakers2 LikeLakers2 marked this pull request as draft January 30, 2025 01:02
@LikeLakers2
Copy link
Contributor Author

Converting to draft while I figure out why data/unit_tests.json isn't being output.

@LikeLakers2 LikeLakers2 force-pushed the project/ci/unit-test-step-summary-on-failure branch from df6a1f3 to 52deb4f Compare January 30, 2025 04:41
@LikeLakers2
Copy link
Contributor Author

I have no idea why my PR is getting stuck on the create_and_destroy test. But I know this PR doesn't touch game code, so it shouldn't matter.

@LikeLakers2 LikeLakers2 marked this pull request as ready for review January 30, 2025 05:22
…ta/unit_tests.json` not being emitted

work, dang you
@LikeLakers2 LikeLakers2 force-pushed the project/ci/unit-test-step-summary-on-failure branch from 52deb4f to 66f7324 Compare January 30, 2025 06:21
@LikeLakers2
Copy link
Contributor Author

Re-drafting this PR while more investigating goes on. I think I accidentally introduced more monkeys by fixing the unit test json being output.

@LikeLakers2 LikeLakers2 marked this pull request as draft January 30, 2025 06:24
@LikeLakers2
Copy link
Contributor Author

This PR is blocked on #5206, due to CI issues which that PR solves. When that PR is merged, this one will be rebased on top of master, and marked as ready for review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants