Modifed existing code to work on ubuntu environment too #114
Conversation
Abhishek Sharma < [email protected] >
| @@ -54,6 +54,8 @@ function tc_local_setup() | |||
| # Following paths are needed for running the tests | |||
| export libdir=$(dirname $Libpath) | |||
| export bindir=/usr/bin | |||
| # Chnage the shell to bash so that code can work on ubuntu too | |||
There was a problem hiding this comment.
s/Chnage/change
General comment for all the changes:
Though summary says the patch set is to modify code to work on ubuntu, observed that some tests are excluded from running on ubuntu. its better to break down the fixes module wise which allows to have proper commit history and reasons for the changes. Also group changes if they are common. Please address in next version
| @@ -35,7 +35,13 @@ TESTDIR=${LTPBIN%/shared}/net_tools | |||
|
|
|||
| iface=0 # system network interface | |||
|
|
|||
| COMMANDS="arp hostname ifconfig ipmaddr iptunnel netstat route traceroute traceroute6" | |||
| #traceroute6 is having an open bug for ubuntu | |||
| grep -i "ubuntu" /etc/*-release >/dev/null 2>&1 | |||
There was a problem hiding this comment.
Add the ubuntu bug number. Instead of grepping here for ubuntu, os check from tc_utils wrapper cant be used here ? Observed same in many places below as well.
| @@ -54,7 +54,7 @@ function tc_local_setup() | |||
| sed -i "/^abs_top_srcdir/ s|/builddir/build/BUILD/parted-2.1|$PARTED_DIR|" $TESTS_DIR/init.sh | |||
| sed -i '/^abs_top_srcdir/ a abs_srcdir="$abs_top_srcdir/tests"' $TESTS_DIR/init.sh | |||
|
|
|||
| sed -i "/^# along with this program/ a ENABLE_DEVICE_MAPPER=yes" $TESTS_DIR/t6000-dm.sh | |||
| #sed -i "/^# along with this program/ a ENABLE_DEVICE_MAPPER=yes" $TESTS_DIR/t6000-dm.sh | |||
|
|
|||
There was a problem hiding this comment.
Please add reason for these set of changes. So If patch is sent module wise, the reason gets covered in your commit.
| tc_pass_or_fail $? "Setting minimum password lifetime to 30 days failed" | ||
| tc_register "passwd --minimum" | ||
| passwd --minimum=30 $TC_TEMP_USER 1>$stdout 2>$stderr | ||
| tc_pass_or_fail $? "Setting minimum password lifetime to 30 days failed" |
|
|
||
|
|
||
| function tc_local_setup() | ||
| { | ||
| tc_check_package "patchutils" | ||
| tc_check_package "patchutils" | ||
| tc_break_if_bad $? "patchutils not installed" || return |
| @@ -72,6 +72,7 @@ function run_test() | |||
| for test in $TESTS; do | |||
| tc_register "Test $test" | |||
| perl $test >$stdout 2>$stderr | |||
| tc_ignore_warnings "Argument" | |||
| tc_pass_or_fail $? "$test failed" | |||
| done | |||
There was a problem hiding this comment.
tc_pass_or_fail $? "$test failed" is wrong . store return code of perl $test and use it here.
Argument is observed in stderr in only ubuntu ?
| if [ "$test" == "t/check_data_structure.t" ];then | ||
| TST_TOTAL=`expr $TST_TOTAL - 1` | ||
| continue | ||
| fi | ||
| tc_register "Test $test" |
There was a problem hiding this comment.
check_data_structure.t is ignored on all distros ?
| TST_TOTAL=`expr $TST_TOTAL - 1` | ||
| continue | ||
| fi | ||
| fi |
There was a problem hiding this comment.
Same here, reason on why these tests are not relevant !
| @@ -54,6 +54,7 @@ function run_test() | |||
| for test in $TESTS; do | |||
| tc_register "Test $test" | |||
| perl $test >$stdout 2>$stderr | |||
| tc_ignore_warnings "Evaluating Canadian English (ct='text/html')" | |||
| tc_pass_or_fail $? "$test failed" | |||
| done | |||
There was a problem hiding this comment.
Please use right return code from perl $test
Abhishek Sharma < [email protected] >