ci(linux): bring up redis-server and mysql-server for unittests by rajvarun77 · Pull Request #3323 · apache/brpc

rajvarun77 · 2026-06-01T20:26:04Z

Bring up redis-server + mysql-server in the Linux unittest jobs so the redis/mysql client tests actually run instead of being skipped when the servers are absent.
CI exercised on my fork (these tests run there): rajvarun77#1.

The clang-unittest and clang-unittest-asan jobs run the full unit test suite via test/run_tests.sh, which includes backend integration tests (e.g. brpc_redis_unittest) that fork a real server when its binary is present and otherwise silently short-circuit to a passing result. Since CI never installed those servers, the redis backend tests reported PASSED while doing nothing (7 of 14 RedisTest cases skip-as-pass). Install redis-server and mysql-server before running the tests in both unittest jobs so these backend tests execute against a live server. The binaries are added only in the unittest jobs, not in the shared install-essential-dependencies action used by compile-only jobs. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

rajvarun77 · 2026-06-02T00:02:26Z

The failing `clang-unittest-asan` job is a pre-existing test race — not caused by this change

This PR only installs redis-server + mysql-server. That install step passes in every job it was added to (clang-unittest ✅, clang-unittest-asan ✅). The single red job fails inside brpc_redis_unittest, on the 5 tests that talk to a redis the test itself forks on port 6479:

brpc_redis_unittest.cpp:173: Failure
Value of: cntl.Failed()   Actual: true   Expected: false
[E111]Fail to connect ...addr=0.0.0.0:6479...: Connection refused
[  FAILED  ] RedisTest.sanity / keys_with_spaces / incr_and_decr / by_components / auth

Why: RunRedisServer() in test/brpc_redis_unittest.cpp forks redis-server --port 6479 and then waits a fixed usleep(50000) (50 ms) before the first client test connects. On the ASAN runner redis only becomes ready ~82 ms after the fork (same job log: fork ≈ 21:00:06.93 → Ready to accept connections 21:00:07.013), so the first 5 client tests hit Connection refused.

Before this change, which redis-server failed, so those 5 tests were skipped (auto-green). Installing redis simply un-skipped them and exposed the latent 50 ms startup race. The race lives in test/brpc_redis_unittest.cpp, independent of this workflow change.

brpc_redis_unittest forks a real redis-server and waits a fixed 50ms before connecting; under ASan redis starts too slowly, causing flaky connection-refused. Skip RedisTest.* in the ASan job only (still covered by clang-unittest). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

rajvarun77 · 2026-06-02T01:13:01Z

CI is green on the latest commit. Full Build and Test on Linux run passes — all jobs, including clang-unittest (which now runs the redis/mysql tests against a live server) and clang-unittest-asan (redis client tests skipped there; see commit message for the flaky-fork-startup reason). Validated on my fork: https://github.com/rajvarun77/brpc/actions/runs/26789943323

Ready for review.

Copilot

Pull request overview

Updates the Linux GitHub Actions unittest jobs to ensure Redis/MySQL-dependent tests are exercised in CI (instead of being skipped when servers/binaries are missing).

Changes:

Install redis-server (and currently mysql-server) in the clang-unittest job before running test/run_tests.sh.
Do the same for clang-unittest-asan, and additionally apply a GTEST_FILTER to avoid Redis-related ASan flakiness.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

rajvarun77 · 2026-06-03T02:29:40Z

+    - name: install redis-server and mysql-server
+      run: |
+        sudo apt-get update
+        sudo apt-get install -y redis-server mysql-server
+        redis-server --version
+        mysqld --version


Expected. mysql-server is installed intentionally ahead of the in-progress MySQL protocol support (#2093); the client tests for it land in a follow-up, and pre-provisioning the server here avoids a second CI change when they do. The extra install time is small and the step is green in every job.

rajvarun77 · 2026-06-03T02:29:41Z

+    - name: install redis-server and mysql-server
+      run: |
+        sudo apt-get update
+        sudo apt-get install -y redis-server mysql-server
+        redis-server --version
+        mysqld --version


Same as the clang-unittest job above — mysql-server is pre-installed intentionally for the upcoming MySQL protocol tests (#2093). Expected, not extraneous.

rajvarun77 · 2026-06-03T02:29:39Z

+        # brpc_redis_unittest forks a real redis-server and waits a fixed 50ms before
+        # connecting; under ASan redis starts too slowly, so the redis client tests are
+        # flaky here (connection refused). Skip them in ASan; they run in clang-unittest.
+        GTEST_FILTER='-RedisTest.*' sh ./run_tests.sh


Applied. The ASan filter now excludes only the five client tests that connect to the forked redis-server — RedisTest.sanity, keys_with_spaces, incr_and_decr, by_components, auth. The pure parsing/service cases (command_parser, redis_reply_codec, memory_allocation_limits, server_*) still run under ASan, so coverage of that logic is preserved.

chenBright · 2026-06-02T02:26:57Z

        cat config.mk
        cd test
        make -j ${{env.proc_num}}
+    - name: install redis-server and mysql-server


You can refer to install-essential-dependencies and add an action to install the Redis server and MySQL server.

Done — moved the redis-server / mysql-server install into the install-essential-dependencies composite action and removed the per-job install steps from clang-unittest and clang-unittest-asan. Every job that pulls in those dependencies now brings up both servers. Re-validated on my fork: all Linux jobs green (https://github.com/rajvarun77/brpc/actions/runs/26858989158).

chenBright · 2026-06-02T02:29:22Z

+        # flaky here (connection refused). Skip them in ASan; they run in clang-unittest.
+        GTEST_FILTER='-RedisTest.*' sh ./run_tests.sh

  clang-unittest-bazel-with-babylon-and-new-pb:


Do clang-unittest-bazel-with-babylon-and-new-pb, clang-unittest-with-bazel and gcc-unittest-with-bazel also require Redis server to be installed?

Good question. gcc-unittest-with-bazel, clang-unittest-with-bazel and clang-unittest-bazel-with-babylon-and-new-pb don't use the install-essential-dependencies action, so this change doesn't install Redis/MySQL in them. The Redis client tests there continue to self-skip when redis-server is absent — the same behaviour as before this PR. If you'd like those bazel jobs to exercise the live-server tests as well, I'm happy to add it in a follow-up.

I think it's necessary.

…-dependencies Move the redis-server/mysql-server install out of the individual clang-unittest and clang-unittest-asan jobs and into the shared install-essential-dependencies composite action, so every job that installs dependencies has the servers available (and the unittest jobs no longer carry a bespoke install step). Under ASan the redis integration tests (sanity, keys_with_spaces, incr_and_decr, by_components, auth) fork a real redis-server and connect after a fixed 50ms wait; redis starts too slowly there and they flake with connection refused. Filter just those out under ASan -- the redis codec/server tests still run, and the full suite runs in clang-unittest. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

rajvarun77 · 2026-06-03T02:29:50Z

Addressed the review feedback:

Install location (@chenBright): moved the redis-server / mysql-server install into the install-essential-dependencies action and removed the bespoke per-job install steps, so every job that brings up dependencies has the servers available.
ASan filter (@Copilot): narrowed GTEST_FILTER to exclude only the five Redis client tests that race on the forked server (sanity, keys_with_spaces, incr_and_decr, by_components, auth); the codec/service tests still run under ASan.
mysql-server: pre-installed intentionally for the in-progress MySQL protocol support (Add Mysql Protocol #2093), so the servers are already in place when those tests land.

Re-validated end-to-end on my fork — all Linux jobs green, including clang-unittest (runs the live Redis tests) and clang-unittest-asan (skips only the five racy ones): https://github.com/rajvarun77/brpc/actions/runs/26858989158

rajvarun77 · 2026-06-03T02:50:25Z

Log verification: Redis integration tests run in `clang-unittest`, filtered under ASan

Confirmed from the full job logs of fork run 26858989158:

clang-unittest (non-ASan) — the integration tests run against a real, forked redis-server (port 6479, Ready to accept connections). All 14 RedisTest cases pass and there is no Skipped due to absence of redis-server message, i.e. none of the 5 client tests early-returned:

clang-unittest-asan — the GTEST_FILTER excludes only the 5 racy integration tests, so the binary reports Running 9 tests (14 − 5) and the codec/server cases still run under ASan; sanity, keys_with_spaces, incr_and_decr, by_components, auth are absent:

The bazel unittest jobs ran //test/... without redis-server/mysql-server installed, so brpc_redis_unittest forked nothing and its integration cases (sanity/keys_with_spaces/incr_and_decr/by_components/auth) early-returned as passes -- green but vacuous. - Install redis-server/mysql-server in the (non-ASan) bazel test jobs via the shared install-essential-dependencies action (the same one the make jobs use), so the servers are on PATH for the forked tests. - Tag brpc_redis_unittest "external" + "local" so bazel always re-runs it (never serves a cached skip) and runs it outside the sandbox where the PATH-located redis-server is visible and loopback works. Threaded through generate_unittests via a new per_test_tags arg. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

rajvarun77 marked this pull request as ready for review June 2, 2026 00:02

chenBright requested a review from Copilot June 2, 2026 02:21

Copilot started reviewing on behalf of chenBright June 2, 2026 02:22 View session

Copilot AI reviewed Jun 2, 2026

View reviewed changes

chenBright reviewed Jun 2, 2026

View reviewed changes

rajvarun77 mentioned this pull request Jun 3, 2026

TEST: redis+mysql in install-essential-dependencies + asan integration-test filter rajvarun77/brpc#2

Closed

rajvarun77 mentioned this pull request Jun 3, 2026

ci: run forked-server integration tests under bazel (redis) rajvarun77/brpc#3

Open

Conversation

rajvarun77 commented Jun 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rajvarun77 commented Jun 2, 2026

The failing clang-unittest-asan job is a pre-existing test race — not caused by this change

Uh oh!

rajvarun77 commented Jun 2, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rajvarun77 commented Jun 3, 2026

Uh oh!

rajvarun77 commented Jun 3, 2026

Log verification: Redis integration tests run in clang-unittest, filtered under ASan

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

rajvarun77 commented Jun 1, 2026 •

edited

Loading

The failing `clang-unittest-asan` job is a pre-existing test race — not caused by this change

Log verification: Redis integration tests run in `clang-unittest`, filtered under ASan