Skip to content
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.

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

ci: add unit test to github action #1604

Merged
merged 7 commits into from
Jun 11, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
8 changes: 7 additions & 1 deletion .github/workflows/pika.yml
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,11 @@ jobs:
# See https://cmake.org/cmake/help/latest/manual/ctest.1.html for more detail
run: ctest -C ${{env.BUILD_TYPE}}

- name: Unit Test
working-directory: ${{github.workspace}}
run: |
./pikatests.sh all

build_on_centos:
# The CMake configure and build commands are platform agnostic and should work equally well on Windows or Mac.
# You can convert this to a matrix build if you need cross-platform coverage.
Expand Down Expand Up @@ -101,4 +106,5 @@ jobs:
run: |
cd ${{github.workspace}}/build
source /opt/rh/devtoolset-10/enable
ctest -C ${{env.BUILD_TYPE}}
ctest -C ${{env.BUILD_TYPE}}
machinly marked this conversation as resolved.
Show resolved Hide resolved

machinly marked this conversation as resolved.
Show resolved Hide resolved
81 changes: 72 additions & 9 deletions pikatests.sh
Original file line number Diff line number Diff line change
@@ -1,10 +1,73 @@
#!/bin/bash
rm -rf ./log
rm -rf .db
cp output/pika src/redis-server
cp conf/pika.conf tests/assets/default.conf

tclsh tests/test_helper.tcl --clients 1 --single unit/$1
rm src/redis-server
rm -rf ./log
rm -rf ./db

# clear the log file
function cleanup() {
rm -rf ./log
rm -rf db
rm -rf dbsync/
rm src/redis-server
}

# check if tcl is installed
function check_tcl {
if [ -z "$(which tclsh)" ]; then
echo "tclsh is not installed"
exit 1
fi
}

# handle different build directories.
function setup_build_dir {
BUILD_DIR=""
if [ -d "./build" ] && [ -d "./output" ]; then
echo "Both build and output directories exist. Please remove one of them."
exit 1
fi
if [ -d "./build" ]; then
BUILD_DIR="build"
fi

if [ -d "./output" ]; then
BUILD_DIR="output"
fi
if [ -z "$BUILD_DIR" ]; then
echo "No build directory found. Please build the project first."
exit 1
fi
echo "BUILD_DIR: $BUILD_DIR"
}

# setup pika bin and conf
function setup_pika_bin {
PIKA_BIN="./$BUILD_DIR/pika"
if [ ! -f "$PIKA_BIN" ]; then
echo "pika bin not found"
exit 1
fi
cp $PIKA_BIN src/redis-server
cp conf/pika.conf tests/assets/default.conf
}


cleanup

check_tcl

setup_build_dir

setup_pika_bin

echo "run pika tests $1"

if [ "$1" == "all" ]; then
tclsh tests/test_helper.tcl --clients 1
else
tclsh tests/test_helper.tcl --clients 1 --single unit/$1
fi

if [ $? -ne 0 ]; then
echo "pika tests failed"
cleanup
exit 1
machinly marked this conversation as resolved.
Show resolved Hide resolved
fi
cleanup
machinly marked this conversation as resolved.
Show resolved Hide resolved
Copy link

Choose a reason for hiding this comment

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

The code review of this patch is as follows:

  • A cleanup function is added to remove log, db and dbsync directories
  • Functions are defined to check if tcl is installed, setup build directory, and pika binary and configuration file
  • The removal of the "rm -rf ./db" command from the original script may potentially leave residual database files in the current directory and the new cleanup function does not remove these either.
  • Code seems to lack proper error handling mechanisms, i.e., it just exits with an error message but does not specify what caused the error or how to fix it.
  • It might be useful to add a logging mechanism to stream output to a separate file and ensure that users can review test results more easily.

Some possible improvements to the existing code may include:

  • Adding proper error handling to the code, such as specifying error types and potential solutions.
  • Instead of using multiple functions, it would be better to use a single function broken down into logical blocks for readability and simplicity.
  • Checking the permissions before removing files or directories to avoid accidental deletion of critical files that can cause issues later on.
  • Clearing the necessary directories more effectively by using more specialized commands like find instead of rm -rf.

71 changes: 38 additions & 33 deletions tests/test_helper.tcl
Original file line number Diff line number Diff line change
Expand Up @@ -12,43 +12,48 @@ source tests/support/test.tcl
source tests/support/util.tcl

set ::all_tests {
unit/printver
unit/auth
unit/protocol
unit/basic
unit/scan
unit/type/list
# unit/printver
wanghenshui marked this conversation as resolved.
Show resolved Hide resolved
# unit/auth
# unit/protocol
# unit/basic
# unit/scan
# unit/type/list
unit/type/list-2
unit/type/list-3
unit/type/set
# unit/type/set
wanghenshui marked this conversation as resolved.
Show resolved Hide resolved
unit/type/zset
unit/type/hash
unit/sort
unit/expire
unit/other
unit/multi
unit/quit
unit/aofrw
integration/replication
integration/replication-2
integration/replication-3
integration/replication-4
integration/replication-psync
integration/aof
integration/rdb
integration/convert-zipmap-hash-on-load
unit/pubsub
unit/slowlog
unit/scripting
unit/maxmemory
unit/introspection
unit/limits
unit/obuf-limits
unit/dump
unit/bitops
unit/memefficiency
unit/hyperloglog
# unit/type/hash
# unit/sort
# unit/expire
# unit/other
# unit/multi
# unit/quit
# unit/aofrw
# integration/replication
# integration/replication-2
# integration/replication-3
# integration/replication-4
# integration/replication-psync
# integration/aof
# integration/rdb
# integration/convert-zipmap-hash-on-load
# unit/pubsub
# unit/slowlog
# unit/scripting
# unit/maxmemory
# unit/introspection
# unit/limits
# unit/obuf-limits
# unit/dump
# unit/bitops
# unit/memefficiency
# unit/hyperloglog
}

# because the comment not works in tcl list, use regsub to ignore the item starting with '#'
regsub -all {#.*?\n} $::all_tests {} ::all_tests


# Index to the next test to run in the ::all_tests list.
set ::next_test 0

machinly marked this conversation as resolved.
Show resolved Hide resolved
machinly marked this conversation as resolved.
Show resolved Hide resolved
machinly marked this conversation as resolved.
Show resolved Hide resolved
wanghenshui marked this conversation as resolved.
Show resolved Hide resolved
Expand Down