Copilot commented on code in PR #3323:
URL: https://github.com/apache/brpc/pull/3323#discussion_r3338275795
##########
.github/workflows/ci-linux.yml:
##########
@@ -196,6 +196,12 @@ jobs:
cat config.mk
cd test
make -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
Review Comment:
The repository appears to have no MySQL-related code/tests (a full-repo
search for `mysql`/`MySQL`/`MYSQL` returns no matches), so installing
`mysql-server` here adds significant CI time and potential apt/service
flakiness without exercising anything. Consider installing only `redis-server`
(which is required for `test/brpc_redis_unittest.cpp` to stop skipping).
##########
.github/workflows/ci-linux.yml:
##########
@@ -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
- 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
Review Comment:
`GTEST_FILTER='-RedisTest.*'` disables the entire `RedisTest` suite,
including tests that don't connect to the forked redis-server (e.g.
`command_parser`, `redis_reply_codec`, `memory_allocation_limits`). If the
flakiness is limited to the client tests that immediately connect after the
fixed 50ms sleep, consider excluding only those specific tests so ASan still
covers the pure parsing/service logic.
##########
.github/workflows/ci-linux.yml:
##########
@@ -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
Review Comment:
Same as above: there don't appear to be any MySQL-related tests in this
repo, so installing `mysql-server` is unnecessary overhead in this job as well.
If the intent is only to avoid skipping Redis tests, install `redis-server`
only.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
