Skip redis test if redis-server not available #24641
Conversation
If the `redis-server` command is not available then we should just skip the test.
GiteaBot
added
the
lgtm/need 2
modules/queue/base_redis_test.go
Outdated
| @@ -56,7 +56,7 @@ func TestBaseRedis(t *testing.T) { | |||
| }() | |||
| if !waitRedisReady("redis://127.0.0.1:6379/0", 0) { | |||
| redisServer = redisServerCmd(t) | |||
| if redisServer == nil && os.Getenv("CI") != "" { | |||
| if redisServer == nil || os.Getenv("CI") != "" { | |||
There was a problem hiding this comment.
I would suggest not doing so.
Keep the old logic will remind developers that "please remember to test the redis queue"
Since in most cases, developers only run single test, this test doesn't affect them.
And there is always a chance for developers to bypass it: CI=local make test-xxxx
I think it's better to clarify the failure message: please use "CI=local make test" if you want to run the full test but do not have a redis-server
But if most people prefer to skip the redis queue test, I have no objection.
There was a problem hiding this comment.
Maybe I'm mis-understanding the use of the CI env variable. The problem with the existing logic on main is that it will skip the test with CI=1 (or anything else).
The problem with the logic suggested here is that it will just always skip the test :)
Please see my suggestion below. I think the default should be local (CI='') and we set the CI env flag in CI.
There was a problem hiding this comment.
It is not reasonable to force developers to install redis in order to run tests.
Co-authored-by: Kyle D. <kdumontnu@gmail.com>
GiteaBot
added
lgtm/need 1
|
Hmm, looks like builds are failing now because we don't have minio set up for unit tests. We can add gitea/.github/workflows/pull-db_test.yml Line 26 in 23ae939 |
GiteaBot
added
lgtm/done
GiteaBot
added
lgtm/blocked
Replaces #24641 Currently, unit tests fail when run locally (unless users have minio instance running). This PR only requires redis unit tests if in CI. - Only run redis unit tests when `CI` env variable is set - Add minio as a service in unit tests actions
|
replaced by #24650 |
If the
redis-servercommand is not available then we should just skip the test.