obs-status-service: add --test-run using interface-based Redis mock (fixes #113) #123

schrodinger · 2026-01-27T15:01:44+01:00

schrodinger commented

2026-01-27 15:01:44 +01:00

First-time contributor

This PR adds a --test-run mode to obs-status-service to allow local development without requiring access to OBS Redis.

The implementation follows an interface-based design for the Redis client, allowing a clean separation between the real Redis client and a mock implementation (redismock.go), while keeping production logic unchanged.

This approach avoids conditional logic in the core code paths and aligns with the design discussed in #113.

Fixes #113.

This PR adds a --test-run mode to obs-status-service to allow local development without requiring access to OBS Redis. The implementation follows an interface-based design for the Redis client, allowing a clean separation between the real Redis client and a mock implementation (redismock.go), while keeping production logic unchanged. This approach avoids conditional logic in the core code paths and aligns with the design discussed in #113. Fixes #113.

schrodinger added 1 commit 2026-01-27 15:01:44 +01:00

obs-status-service: add --test-run using interface-based Redis mock (fixes #113 )

go-generate-check / go-generate-check (pull_request) Has been cancelled

Details

bb37ea8718

adamm reviewed 2026-01-27 15:41:56 +01:00

obs-status-service/redis.go Outdated

						
				@@ -21,2 +26,3 @@

					if err != nil {

						panic(err)

						common.LogError("Invalid Redis URL", err)

						return

adamm commented

2026-01-27 15:41:56 +01:00

Why no more panic()?

schrodinger commented

2026-01-27 17:53:59 +01:00

First-time contributor

Why no more panic()?

I removed panic because I think that this code runs in a long-lived service so like any invalid Redis URL might give a runtime error, so logging the error and exiting cleanly felt more appropriate than crashing with a stack trace.

This also keeps the behavior consistent with the --test-run mode, where we want predictable startup failures instead of panics.

If you prefer keeping panic here for consistency with the rest of the codebase, I’m happy to switch it back.

> Why no more panic()? I removed panic because I think that this code runs in a long-lived service so like any invalid Redis URL might give a runtime error, so logging the error and exiting cleanly felt more appropriate than crashing with a stack trace. This also keeps the behavior consistent with the --test-run mode, where we want predictable startup failures instead of panics. If you prefer keeping panic here for consistency with the rest of the codebase, I’m happy to switch it back.

adamm commented

2026-02-02 11:54:39 +01:00

Invalid Redis URL means we cannot start so it's fine to panic(). in *testRun you're already not calling this function anyway. Maybe have an error log here too and then panic.

It's long running only when it has an actual URL to try.

Invalid Redis URL means we cannot start so it's fine to panic(). in *testRun you're already not calling this function anyway. Maybe have an error log here too and then panic. It's long running only when it has an actual URL to try.

schrodinger commented

2026-02-02 15:21:23 +01:00

First-time contributor

Ok I understand, thanks for the clarification!
I’ve updated RedisConnect to panic on an invalid Redis URL.

Ok I understand, thanks for the clarification! I’ve updated RedisConnect to panic on an invalid Redis URL.

schrodinger requested review from adamm 2026-01-27 17:54:10 +01:00

schrodinger force-pushed issue-113 from bb37ea8718 to ac19fecf01

2026-01-29 16:31:18 +01:00

Compare

schrodinger force-pushed issue-113 from ac19fecf01 to d563763f63

2026-02-02 15:19:58 +01:00

Compare

go-generate-check / go-generate-check (pull_request) Has been cancelled

Details

This pull request can be merged automatically.

This branch is out-of-date with the base branch

You are not authorized to merge this pull request.

View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.

git fetch -u issue-113:schrodinger-issue-113

git checkout schrodinger-issue-113

Sign in to join this conversation.

2 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: git-workflow/autogits#123

Allow edits from maintainers