This looks like a nice upgrade, but I have a couple of comments.
> -----Original Message-----
> From: Wang Mingyu
>
> Signed-off-by: Wang Mingyu <[email protected]>
> ---
> tests/Functional.samba/data/smb04.conf | 4 +-
> tests/Functional.samba/data/smb05.conf | 19 ++++
> tests/Functional.samba/data/smb06.conf | 18 +++
> tests/Functional.samba/fuego_test.sh | 20 +++-
> tests/Functional.samba/samba_test.sh | 8 +-
> tests/Functional.samba/test.yaml | 10 +-
> .../Functional.samba/tests/restore_target.sh | 24 ++++
> tests/Functional.samba/tests/test_samba04.sh | 16 +--
> tests/Functional.samba/tests/test_samba05.sh | 103
> ++++++++++++++++++
> tests/Functional.samba/tests/test_samba06.sh | 58 ++++++++++
> .../tests/test_samba06_host.sh | 23 ++++
> 11 files changed, 278 insertions(+), 25 deletions(-)
> create mode 100644 tests/Functional.samba/data/smb05.conf
> create mode 100644 tests/Functional.samba/data/smb06.conf
> mode change 100644 => 100755 tests/Functional.samba/samba_test.sh
> create mode 100644 tests/Functional.samba/tests/restore_target.sh
> create mode 100644 tests/Functional.samba/tests/test_samba05.sh
> create mode 100644 tests/Functional.samba/tests/test_samba06.sh
> create mode 100644 tests/Functional.samba/tests/test_samba06_host.sh
>
> diff --git a/tests/Functional.samba/data/smb04.conf
> b/tests/Functional.samba/data/smb04.conf
> index 94acbbc..c836021 100644
> --- a/tests/Functional.samba/data/smb04.conf
> +++ b/tests/Functional.samba/data/smb04.conf
> @@ -6,14 +6,12 @@
> load printers = yes
> log file = /var/log/samba/log.%m
> max log size = 50
> - max protocol = SMB3_10
> - min protocol = SMB2
> interfaces = 127.0.0.1 test_target
> dns proxy = no
>
> [test]
> comment = samba server
> - path = /home/test_for_fuego/samba
> + path = /home/test/samba
> writable = yes
> browseable = yes
> guest ok = yes
> diff --git a/tests/Functional.samba/data/smb05.conf
> b/tests/Functional.samba/data/smb05.conf
> new file mode 100644
> index 0000000..63d3d02
> --- /dev/null
> +++ b/tests/Functional.samba/data/smb05.conf
> @@ -0,0 +1,19 @@
> +[global]
> + workgroup = MYGROUP
> + server string = Samba Server
> + security = user
> + passdb backend = tdbsam
> + hosts allow = 127.0.0.1 test_target
> + load printers = no
> + log file = /var/log/samba/log.%m
> + max log size = 50
> + max protocol = SMB3_10
> + min protocol = SMB2
> + interfaces = 127.0.0.1 test_target
> + dns proxy = no
> +
> +[test]
> + comment = samba server
> + path = /home/test/samba
> + writable = yes
> + browseable = yes
> diff --git a/tests/Functional.samba/data/smb06.conf
> b/tests/Functional.samba/data/smb06.conf
> new file mode 100644
> index 0000000..1bbacd4
> --- /dev/null
> +++ b/tests/Functional.samba/data/smb06.conf
> @@ -0,0 +1,18 @@
> +[global]
> + workgroup = MYGROUP
> + server string = Samba Server
> + security = user
> + hosts allow = 127.0.0.1 test_host
> + load printers = yes
> + log file = /var/log/samba/log.%m
> + max log size = 50
> + interfaces = 127.0.0.1 test_target
> + dns proxy = no
> +
> +[test]
> + comment = samba server
> + path = /home/test/samba
> + writable = yes
> + browseable = yes
> + guest ok = yes
> + public = yes
> diff --git a/tests/Functional.samba/fuego_test.sh
> b/tests/Functional.samba/fuego_test.sh
> index 1f694ef..63e0cef 100644
> --- a/tests/Functional.samba/fuego_test.sh
> +++ b/tests/Functional.samba/fuego_test.sh
> @@ -1,7 +1,9 @@
> +NEED_ROOT=1
> +
> function test_pre_check {
> + assert_has_program samba
> assert_has_program expect
> assert_has_program smbclient
> - assert_has_program sed
You use 'sed' below. I'm not sure why you are removing this assertion.
'sed' is not one of the programs required by Fuego, so you should check
for it.
> }
>
> function test_deploy {
> @@ -12,9 +14,19 @@ function test_deploy {
> }
>
> function test_run {
> - report "cd $BOARD_TESTDIR/fuego.$TESTDIR; \
> - export test_target=$IPADDR; \
> - ./samba_test.sh"
> + report "cd $BOARD_TESTDIR/fuego.$TESTDIR;\
> + export test_target=$IPADDR;\
> + export test_host=$SRV_IP;\
> + ./samba_test.sh"
> +
> + service_file=/tmp/test_for_fuego
> + report_append "cd $BOARD_TESTDIR/fuego.$TESTDIR;\
> + sh tests/test_samba06.sh $SRV_IP $IPADDR $service_file"
> +
> + log_this "cd $TEST_HOME; sh tests/test_samba06_host.sh $IPADDR"
> +
> + report_append "cd $BOARD_TESTDIR/fuego.$TESTDIR;\
> + sh tests/restore_target.sh $service_file"
Does this need to be a 'report_append'? Is there something from restore_target.sh
that needs to go into the test log? I can't see anything.
Please convert this to just 'cmd'.
> }
>
> function test_processing {
> diff --git a/tests/Functional.samba/samba_test.sh
> b/tests/Functional.samba/samba_test.sh
> old mode 100644
> new mode 100755
> index 95c34db..93b0740
> --- a/tests/Functional.samba/samba_test.sh
> +++ b/tests/Functional.samba/samba_test.sh
> @@ -2,6 +2,8 @@
> . ./fuego_board_function_lib.sh
> set_init_manager
>
> -for i in tests/*.sh; do
> - source $i
> -done
> +source tests/test_samba01.sh
> +source tests/test_samba02.sh
> +source tests/test_samba03.sh
> +source tests/test_samba04.sh
> +source tests/test_samba05.sh
> diff --git a/tests/Functional.samba/test.yaml
> b/tests/Functional.samba/test.yaml
> index bd61de6..f38dcf9 100644
> --- a/tests/Functional.samba/test.yaml
> +++ b/tests/Functional.samba/test.yaml
> @@ -1,11 +1,9 @@
> fuego_package_version: 1
> name: Functional.samba
> description: |
> - The Samba software suite is a collection of programs that implements
> - the Server Message Block protocol for UNIX systems.
> - This is a simple test to check the ps/pid/syslog of samba and use
> - smbclient to implement a simple ftp-like client.
> -license: BSD-3-Clause
> + The Samba software suite is a collection of programs that implements
> the Server Message Block protocol for UNIX systems.
> + This is a simple test to check the ps/pid/syslog of samba and use
> smbclient to implement a simple ftp-like client.
Please keep descriptions to 80 columns.
> +license: BSD-3-Clause
> author: Wang Mingyu <[email protected]>
> maintainer: Wang Mingyu <[email protected]>
> version: 1.00
> @@ -15,6 +13,8 @@ tags: ['samba']
> params:
> IPADDR:
> description: the IP address of your target board - Defined in board file
> + SRV_IP:
> + description: the IP address of your host - Defined in board file
> data_files:
> - samba_test.sh
> - fuego_test.sh
> diff --git a/tests/Functional.samba/tests/restore_target.sh
> b/tests/Functional.samba/tests/restore_target.sh
> new file mode 100644
> index 0000000..6610923
> --- /dev/null
> +++ b/tests/Functional.samba/tests/restore_target.sh
> @@ -0,0 +1,24 @@
> +#!/bin/sh
> +
> +# In the target start samba, and confirm the process condition by command
> ps.
> +# check the keyword "smbd".
This comment appears to be a copy-paste bug. It does not describe
this file. Please update it.
> +
> +. ./fuego_board_function_lib.sh
> +set_init_manager
> +
> +if [ -f /etc/samba/smb.conf_bak ]
> +then
> + mv /etc/samba/smb.conf_bak /etc/samba/smb.conf
> +fi
> +mv data/smb06.conf_bak data/smb06.conf
> +
> +if [ "$(tail -n 1 $1)" = "active" -o "$(tail -n 1 $1)" = "unknown" ]
> +then
> + exec_service_on_target iptables start
> +fi
> +if [ "$(head -n 1 $1)" = "inactive" ]
> +then
> + exec_service_on_target smb stop
> +fi
> +
> +rm -fr $1
> diff --git a/tests/Functional.samba/tests/test_samba04.sh
> b/tests/Functional.samba/tests/test_samba04.sh
> index 2ad6e59..c1dac10 100644
> --- a/tests/Functional.samba/tests/test_samba04.sh
> +++ b/tests/Functional.samba/tests/test_samba04.sh
> @@ -10,14 +10,12 @@ iptables_status=$(get_service_status iptables)
> exec_service_on_target smb stop
> exec_service_on_target iptables stop
>
> -useradd test_for_fuego
> +mkdir -p /home/test/samba
> +mkdir -p /home/test/samba_test
>
> -mkdir -p /home/test_for_fuego/samba
> -mkdir -p /home/test_for_fuego/samba_test
> +cp data/test1 /home/test/samba/
>
> -cp data/test1 /home/test_for_fuego/samba/
> -
> -chown nobody:nobody -R /home/test_for_fuego/samba
> +chown nobody:nobody -R /home/test/samba
>
> if [ -f /etc/samba/smb.conf ]
> then
> @@ -30,8 +28,6 @@ restore_target(){
> mv /etc/samba/smb.conf_bak /etc/samba/smb.conf
> fi
> mv data/smb04.conf_bak data/smb04.conf
> - rm -fr /home/test_for_fuego
> - userdel test_for_fuego
> if [ "$iptables_status" = "active" -o "$iptables_status" = "unknown" ]
> then
> exec_service_on_target iptables start
> @@ -62,7 +58,7 @@ expect <<-EOF
> spawn smbclient //$test_target/test -N
> expect {
> -re ".*smb: .*" {
> - send "get test1 /home/test_for_fuego/samba_test/test1\n"
> + send "get test1 /home/test/samba_test/test1\n"
> }
> default { send_user "Can not log into the board.\n"} }
> expect {
> @@ -74,7 +70,7 @@ send "exit\n"
> expect eof
> EOF
>
> -if ls /home/test_for_fuego/samba_test/test1
> +if ls /home/test/samba_test/test1
> then
> echo " -> $test: TEST-PASS"
> else
> diff --git a/tests/Functional.samba/tests/test_samba05.sh
> b/tests/Functional.samba/tests/test_samba05.sh
> new file mode 100644
> index 0000000..f66a92f
> --- /dev/null
> +++ b/tests/Functional.samba/tests/test_samba05.sh
> @@ -0,0 +1,103 @@
> +#!/bin/sh
> +
> +# In the target start samba,
> +# Set the server to access the target having password from the server, and
> make sure the server can access the target.
Please keep comments (and code where possible) to 80 columns width.
Also - just to clarify - is this test (samba05) doing all operations on the
board. The description is a bit confusing.
> +
> +test="samba05"
> +
> +. ./fuego_board_function_lib.sh
> +
> +set_init_manager
> +
> +useradd test_for_fuego
> +
> +expect <<-EOF
> +spawn smbpasswd -a test_for_fuego
> +expect "SMB password:"
> +send "goodluck\r"
> +expect "Retype new SMB password:"
> +send "goodluck\r"
> +expect "Added user root."
> +EOF
> +
> +smb_status=$(get_service_status smb)
> +iptables_status=$(get_service_status iptables)
> +
> +exec_service_on_target smb stop
> +exec_service_on_target iptables stop
> +
> +mkdir -p /home/test/samba
> +
> +cp data/test1 /home/test/samba/
> +
> +chown nobody:nobody -R /home/test/samba
> +
> +if [ -f /etc/samba/smb.conf ]
> +then
> + mv /etc/samba/smb.conf /etc/samba/smb.conf_bak
> +fi
> +
> +restore_target(){
> + if [ -f /etc/samba/smb.conf_bak ]
> + then
> + mv /etc/samba/smb.conf_bak /etc/samba/smb.conf
> + fi
> + smbpasswd -x test_for_fuego
> + rm -fr /home/test_for_fuego
> + mv data/smb05.conf_bak data/smb05.conf
Normally, the entire fuego.Functional.samba directory is removed
after the test. I don't think it's necessary to save off the smb05.bak
and restore it here. (Unless it's used more than once per test run,
but I don't see that anywhere.)
> + userdel test_for_fuego
> + if [ "$iptables_status" = "active" -o "$iptables_status" = "unknown" ]
> + then
> + exec_service_on_target iptables start
> + fi
> + if [ "$smb_status" = "inactive" ]
> + then
> + exec_service_on_target smb stop
> + fi
> +}
> +
> +cp data/smb05.conf data/smb05.conf_bak
See above. I don't think you need to save a copy of this file.
> +sed -i 's/test_target/'"$test_target"'/g' data/smb05.conf
Is there some reason this can't use 'localhost' or '127.0.0.1'
instead of $test_target here?
> +cp data/smb05.conf /etc/samba/smb.conf
> +
> +if exec_service_on_target smb start
> +then
> + echo " -> start of smb succeeded."
> +else
> + echo " -> start of smb failed."
> + echo " -> $test: TEST-FAIL"
> + restore_target
> + exit
> +fi
> +
> +sleep 5
> +
> +expect <<-EOF
> +spawn sh
> +expect ".*"
> +send "smbclient //$test_target/test --user=test_for_fuego\r"
Same thing here. Is $test_target needed, or could it be 'localhost'
or '127.0.0.1'? (Maybe it's needed. I'm not an smb expert. But this could
be simplified you could use a generic address for the local machine.)
> +expect {
> + -re ".*Enter.*password:.*" {
> + send "goodluck\r"
> + }
> + default { send_user "Can not access the board.\n"} }
> +expect {
> + -re ".*Password:.*" {
> + send "goodluck\r"
> + }
> + default { send_user "Password is wrong.\n"} }
> +expect {
> + -re ".*smb:.*" {
> + send "ls\r"
> + }
> + default { send_user "Can not log into the board.\n"} }
> +expect {
> + -re ".*test1.*" {
> + send_user " -> $test: TEST-PASS\n"
> + }
> + default { send_user " -> $test: TEST-FAIL\n"} }
> +send "exit\n"
> +expect eof
> +EOF
> +
> +restore_target
> diff --git a/tests/Functional.samba/tests/test_samba06.sh
> b/tests/Functional.samba/tests/test_samba06.sh
> new file mode 100644
> index 0000000..29c1dd1
> --- /dev/null
> +++ b/tests/Functional.samba/tests/test_samba06.sh
> @@ -0,0 +1,58 @@
> +#!/bin/sh
> +
> +# In the target start samba, and confirm the process condition by command
> ps.
> +# check the keyword "smbd".
> +
> +
> +. ./fuego_board_function_lib.sh
> +set_init_manager
> +
> +echo $(get_service_status smb) > $3
> +echo $(get_service_status iptables) >> $3
> +
> +exec_service_on_target smb stop
> +exec_service_on_target iptables stop
> +
> +mkdir -p /home/test/samba
> +
> +cp data/test1 /home/test/samba/
> +
> +chown nobody:nobody -R /home/test/samba
> +
> +if [ -f /etc/samba/smb.conf ]
> +then
> + mv /etc/samba/smb.conf /etc/samba/smb.conf_bak
> +fi
> +
> +restore_target(){
> + if [ -f /etc/samba/smb.conf_bak ]
> + then
> + mv /etc/samba/smb.conf_bak /etc/samba/smb.conf
> + fi
> + mv data/smb06.conf_bak data/smb06.conf
I don't think smb06.conf needs to be backed up or restored.
> + if [ "$iptables_status" = "active" -o "$iptables_status" = "unknown" ]
> + then
> + exec_service_on_target iptables start
> + fi
> + if [ "$smb_status" = "inactive" ]
> + then
> + exec_service_on_target smb stop
> + fi
> +}
> +
> +
> +cp data/smb06.conf data/smb06.conf_bak
I don't think smb06.conf needs to be backed up or restored.
This could be remove, IMHO.
Or are you trying to make it so this test can be run manually multiple
times (outside of Fuego's CI context?)
> +sed -i 's/test_target/'"$2"'/g' data/smb06.conf
> +sed -i 's/test_host/'"$1"'/g' data/smb06.conf
> +cp data/smb06.conf /etc/samba/smb.conf
> +
> +if exec_service_on_target smb start
> +then
> + echo " -> start of smb succeeded."
> +else
> + echo " -> start of smb failed."
> + echo " -> $test: TEST-FAIL"
> + exit
> +fi
> +
> +sleep 5
> diff --git a/tests/Functional.samba/tests/test_samba06_host.sh
> b/tests/Functional.samba/tests/test_samba06_host.sh
> new file mode 100644
> index 0000000..77ff7a5
> --- /dev/null
> +++ b/tests/Functional.samba/tests/test_samba06_host.sh
> @@ -0,0 +1,23 @@
> +#!/bin/sh
> +
> +# In the target start samba, and confirm the process condition by command
> ps.
> +# check the keyword "smbd".
This description doesn't seem right. isn't this a test on the host against
the server running on the target?
Please update this comment, or explain why it's accurate.
> +
> +test="samba06"
> +expect <<-EOF
> +spawn sh
> +expect ".*"
> +send "smbclient //$1/test -N\r"
> +expect {
> + -re ".*smb:.*" {
> + send "ls\r"
> + }
> + default { send_user "Can't connect to the board"\n"} }
> +expect {
> + -re ".*test1.*" {
> + send_user " -> $test: TEST-PASS\n"
> + }
> + default { send_user " -> $test: TEST-FAIL\n"} }
> +send "exit\n"
> +expect eof
> +EOF
> --
> 2.17.1
This is pretty nice. It's very nice to see a test that uses Fuego's inherent
host/target capabilities to perform a test. And it's nice to see a test
of actual samba functionality.
Please fix the items mentioned, and/or respond to comments
and resubmit.
Thanks,
-- Tim
lmpx.com only provides a reader for public news (NNTP) servers. It is not
affiliated with the servers or forums shown here and is not responsible for
the content of articles, which is written by their respective authors.