fix: forgejo-runner validate exit with error when validation fails #1009

Merged
earl-warren merged 2 commits from earl-warren/runner:wip-validate into main 2025-09-18 06:52:20 +00:00
Contributor
  • bug fixes
    • PR: fix: forgejo-runner validate exit with error when validation fails
<!--start release-notes-assistant--> <!--URL:https://code.forgejo.org/forgejo/runner--> - bug fixes - [PR](https://code.forgejo.org/forgejo/runner/pulls/1009): <!--number 1009 --><!--line 0 --><!--description Zml4OiBmb3JnZWpvLXJ1bm5lciB2YWxpZGF0ZSBleGl0IHdpdGggZXJyb3Igd2hlbiB2YWxpZGF0aW9uIGZhaWxz-->fix: forgejo-runner validate exit with error when validation fails<!--description--> <!--end release-notes-assistant-->
earl-warren changed title from fix: forgejo-runner validate exit on error when validation fails to WIP: fix: forgejo-runner validate exit on error when validation fails 2025-09-17 13:43:50 +00:00
Contributor

Maybe “exit with error” instead of “on error”? It’s not very clear that this implements error codes for exits rather than exiting when validation fails (which it already does, once it’s done validating). :)

I glanced over the code too and didn’t spot any obvious flaws—but my Go-fu is not very strong and I didn’t look in‐depth to try and decipher what was going on in more detail. It doesn’t seem like any tests were added to check for non-zero exit codes though? Maybe I’m missing something.

Maybe “exit _with_ error” instead of “_on_ error”? It’s not very clear that this implements error codes for exits rather than exiting when validation fails (which it already does, once it’s done validating). :) I glanced over the code too and didn’t spot any obvious flaws—but my Go-fu is not very strong and I didn’t look in‐depth to try and decipher what was going on in more detail. It doesn’t seem like any tests were added to check for non-zero exit codes though? Maybe I’m missing something.
earl-warren changed title from WIP: fix: forgejo-runner validate exit on error when validation fails to WIP: fix: forgejo-runner validate exit with error when validation fails 2025-09-18 06:33:13 +00:00
earl-warren force-pushed wip-validate from 2e2e084613
All checks were successful
cascade / forgejo (pull_request_target) Has been skipped
cascade / debug (pull_request_target) Has been skipped
cascade / end-to-end (pull_request_target) Has been skipped
checks / validate mocks (pull_request) Successful in 1m9s
checks / build and test (pull_request) Successful in 1m23s
checks / runner exec tests (pull_request) Successful in 36s
checks / runner integration tests (pull_request) Successful in 6m27s
checks / integration tests (pull_request) Successful in 16m17s
issue-labels / release-notes (pull_request_target) Successful in 6s
to 2e83c0662b
Some checks failed
cascade / forgejo (pull_request_target) Has been skipped
cascade / debug (pull_request_target) Has been skipped
cascade / end-to-end (pull_request_target) Has been skipped
issue-labels / release-notes (pull_request_target) Successful in 6s
checks / validate mocks (pull_request) Successful in 53s
checks / build and test (pull_request) Successful in 2m37s
checks / runner integration tests (pull_request) Has been cancelled
checks / runner exec tests (pull_request) Has been cancelled
checks / integration tests (pull_request) Has been cancelled
2025-09-18 06:38:08 +00:00
Compare
earl-warren changed title from WIP: fix: forgejo-runner validate exit with error when validation fails to fix: forgejo-runner validate exit with error when validation fails 2025-09-18 06:38:29 +00:00
Author
Contributor

It doesn’t seem like any tests were added to check for non-zero exit codes though? Maybe I’m missing something.

The tests at #1009/files now have a message field set which is verified to be contained in the error returned by validation. It was previously expected to be empty, meaning no error returned.

The CLI library will exit on error if a non nil error is returned.

Maybe “exit with error” instead of “on error”?

Done.

> It doesn’t seem like any tests were added to check for non-zero exit codes though? Maybe I’m missing something. The tests at https://code.forgejo.org/forgejo/runner/pulls/1009/files#diff-3496cc8ea4728e33a9e503f5bf7db0201d1a37f8 now have a message field set which is verified to be contained in the error returned by validation. It was previously expected to be empty, meaning no error returned. The CLI library will exit on error if a non nil error is returned. > Maybe “exit with error” instead of “on error”? Done.
earl-warren force-pushed wip-validate from 2e83c0662b
Some checks failed
cascade / forgejo (pull_request_target) Has been skipped
cascade / debug (pull_request_target) Has been skipped
cascade / end-to-end (pull_request_target) Has been skipped
issue-labels / release-notes (pull_request_target) Successful in 6s
checks / validate mocks (pull_request) Successful in 53s
checks / build and test (pull_request) Successful in 2m37s
checks / runner integration tests (pull_request) Has been cancelled
checks / runner exec tests (pull_request) Has been cancelled
checks / integration tests (pull_request) Has been cancelled
to 8c6c089a65
All checks were successful
issue-labels / release-notes (pull_request_target) Successful in 5s
checks / validate mocks (pull_request) Successful in 36s
checks / build and test (pull_request) Successful in 48s
checks / runner exec tests (pull_request) Successful in 30s
checks / runner integration tests (pull_request) Successful in 4m20s
checks / integration tests (pull_request) Successful in 10m19s
cascade / debug (pull_request_target) Has been skipped
cascade / end-to-end (pull_request_target) Successful in 4s
cascade / forgejo (pull_request_target) Successful in 1m39s
2025-09-18 06:40:49 +00:00
Compare
viceice approved these changes 2025-09-18 06:48:15 +00:00
earl-warren deleted branch wip-validate 2025-09-18 06:52:20 +00:00
Sign in to join this conversation.
No reviewers
No milestone
No assignees
3 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Reference
forgejo/runner!1009
No description provided.