Skip to content

ci(linux): bring up redis-server and mysql-server for unittests #3323

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.

Sign up for GitHub

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

Merged
chenBright merged 4 commits into from
Jun 5, 2026
Merged

ci(linux): bring up redis-server and mysql-server for unittests #3323

Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a6e885b
ci(linux): bring up redis-server and mysql-server for unittests
rajvarun77 Jun 1, 2026
c6a03f5
ci(linux): skip flaky redis client tests under ASan
rajvarun77 Jun 2, 2026
4ed6c48
ci(linux): install redis-server and mysql-server in install-essential…
rajvarun77 Jun 3, 2026
815d086
ci(linux): run forked-server integration tests under bazel
rajvarun77 Jun 3, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 16 additions & 1 deletion .github/workflows/ci-linux.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,12 @@ jobs:
cat config.mk
cd test
make -j ${{env.proc_num}}
- name: install redis-server and mysql-server

@chenBright chenBright Jun 2, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@rajvarun77 rajvarun77 Jun 3, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

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

@rajvarun77 rajvarun77 Jun 3, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

- name: run tests
run: |
cd test
Expand All @@ -214,10 +220,19 @@ jobs:
cat config.mk
cd test
make NEED_GPERFTOOLS=0 -j ${{env.proc_num}}
- 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

@rajvarun77 rajvarun77 Jun 3, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

- name: run tests
run: |
cd test
sh ./run_tests.sh
# 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

@rajvarun77 rajvarun77 Jun 3, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


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

@chenBright chenBright Jun 2, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

@rajvarun77 rajvarun77 Jun 3, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@chenBright chenBright Jun 3, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's necessary.

@rajvarun77 rajvarun77 Jun 4, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done — addressed in the latest commit. gcc-unittest-with-bazel, clang-unittest-with-bazel, and clang-unittest-bazel-with-babylon-and-new-pb now pull in the install-essential-dependencies action, and brpc_redis_unittest is tagged external + local so it runs unsandboxed (where the PATH-located redis-server is visible and loopback works) and re-runs rather than serving a cached pass.

The Redis integration tests now genuinely fork redis-server and run under Bazel — confirmed from the logs of the latest green Build and Test on Linux run: the forked redis-server reaches Ready to accept connections, and RedisTest.sanity / keys_with_spaces / incr_and_decr / by_components / auth all pass (not skipped, not cached).

runs-on: ubuntu-22.04
Expand Down
Loading