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

Add and improve tests #92

Merged
merged 5 commits into from
Nov 15, 2024
Merged

Add and improve tests #92

merged 5 commits into from
Nov 15, 2024

Conversation

abetomo
Copy link
Contributor

@abetomo abetomo commented Nov 14, 2024

  • Execute queries with commands in the container
    • Remove -p 33061:3306 from container run
  • Change the query result to check as it is
  • Add test for mroonga_command('status')

Copy link

@otegami otegami left a comment

Choose a reason for hiding this comment

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

LGTM

* Execute queries with commands in the container
  * Remove `-p 33061:3306` from `container run`
* Change the query result to check as it is
* Add test for `mroonga_command('status')`
CI may take longer than 10 seconds.
test.sh Outdated
@@ -19,29 +19,54 @@ container_name="mroonga_build_test_${timestamp}"
eval $(grep -E -o '[a-z]+_version=[0-9.]+' $context/Dockerfile)
mysql_version=$(head -n1 $context/Dockerfile | grep -E -o '[0-9.]{2,}')

function run_sql() {
sql=$1
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
sql=$1
sql="$1"

test.sh Outdated
success=$?

set -e
sudo docker container stop $container_name
Copy link
Member

Choose a reason for hiding this comment

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

こいつらをtrap EXITでやるようにしたら↑の&&はなくせる?

function cleanup() {
  sudo docker container stop $container_name
  sudo docker container logs $container_name
  sudo docker container rm $container_name
}
trap cleanup EXIT

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ぉぉ、これ探してたんですよ!試してみます。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

あざます。できました。

test.sh Outdated
set +e
diff -u /tmp/expected.txt /tmp/actual.txt
echo -e "$(run_sql "SELECT JSON_PRETTY(mroonga_command('status'))")" && \
Copy link
Member

Choose a reason for hiding this comment

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

echo -eってなんで必要なの?\nを改行にしたいとか?

Suggested change
echo -e "$(run_sql "SELECT JSON_PRETTY(mroonga_command('status'))")" && \
run_sql "SELECT JSON_PRETTY(mroonga_command('status'))" && \

Copy link
Contributor Author

Choose a reason for hiding this comment

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

そうですね。
echo しないと {\n "os": "Linux",\n "cpu": "x86_64",\n ... という感じだったので付けました。
そもそもこの表示がいらないか。

test.sh Outdated
Comment on lines 28 to 29
expected=$1
actual=$2
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
expected=$1
actual=$2
expected="$1"
actual="$2"

test.sh Outdated
function assert() {
expected=$1
actual=$2
if [ "$(echo ${expected})" = "$(echo ${actual})" ]; then
Copy link
Member

Choose a reason for hiding this comment

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

なんでechoいるの?

Suggested change
if [ "$(echo ${expected})" = "$(echo ${actual})" ]; then
if [ "${expected}" = "${actual}" ]; then

Copy link
Contributor Author

Choose a reason for hiding this comment

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

echo いらないです。見た目が同じ文字列が = 判定されなくて色々検証していた残骸です…。
= 判定されなかったのは \t じゃなくて (ただのスペース)だったからでした。)

test.sh Outdated Show resolved Hide resolved
test.sh Outdated Show resolved Hide resolved
@kou kou merged commit b2d27c9 into mroonga:master Nov 15, 2024
1 check passed
@abetomo abetomo deleted the add-and-improve-tests branch November 15, 2024 01:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants