Re: [PATCH net-next 5/5] selftests/net: Adding test cases of replacing routes and route advertisements.

[Kui-Feng Lee <sinquersw@gmail.com>]

On 2/1/24 00:46, Hangbin Liu wrote:
> Hi,
> 
> On Tue, Jan 30, 2024 at 10:40:41PM -0800, thinker.li@gmail.com wrote:
>> +# Create a new dummy_10 to remove all associated routes.
>> +reset_dummy_10()
>> +{
>> +	$IP link del dev dummy_10
>> +
>> +	$IP link add dummy_10 type dummy
>> +	$IP link set dev dummy_10 up
>> +	$IP -6 address add 2001:10::1/64 dev dummy_10
>> +}
>> +
>>   fib6_gc_test()
>>   {
>>   	setup
>> @@ -768,15 +778,19 @@ fib6_gc_test()
>>   	    $IP -6 route add 2001:20::$i \
>>   		via 2001:10::2 dev dummy_10 expires $EXPIRE
>>   	done
>> -	sleep $(($EXPIRE * 2))
>> -	N_EXP_SLEEP=$($IP -6 route list |grep expires|wc -l)
>> -	if [ $N_EXP_SLEEP -ne 0 ]; then
>> -	    echo "FAIL: expected 0 routes with expires, got $N_EXP_SLEEP"
>> +	sleep $(($EXPIRE * 2 + 1))
>> +	N_EXP=$($IP -6 route list |grep expires|wc -l)
>> +	if [ $N_EXP -ne 0 ]; then
>> +	    echo "FAIL: expected 0 routes with expires, got $N_EXP"
>>   	    ret=1
>>   	else
>>   	    ret=0
>>   	fi
>>   
>> +	log_test $ret 0 "ipv6 route garbage collection"
>> +
>> +	reset_dummy_10
> 
> Since you reset the dummy device and will not affect the later tests. Maybe
> you can log the test directly, e.g.
> 
> 	if [ "$($IP -6 route list |grep expires|wc -l)" -ne 0 ]; then
> 		log_test $ret 0 "ipv6 route garbage collection"
> 	fi
> 
> Or, if you want to keep ret and also report passed log, you can wrapper the
> number checking like
> 
> check_exp_number()
> {
> 	local exp=$1
> 	local n_exp=$($IP -6 route list |grep expires|wc -l)
> 	if [ "$n_exp" -ne "$exp" ]; then
> 		echo "FAIL: expected $exp routes with expires, got $n_exp"
> 		ret=1
> 	else
> 		ret=0
> 	fi
> }
> 
> Then we can call it without repeating the if/else lines
> 
> 	check_exp_number 0
> 	log_test $ret 0 "ipv6 route garbage collection"

If I read it correctly, the point here is too many boilerplate checks,
and you prefer to reduce them. Right?
No problem! I will do it.


> 
> Thanks
> Hangbin