netdev - Re: [PATCH net-next 3/9] selftests: pmtu: Introduce support for multiple tests

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20180316210320.48a1b17e@epycfail>
Date:   Fri, 16 Mar 2018 21:03:20 +0100
From:   Stefano Brivio <sbrivio@...hat.com>
To:     David Ahern <dsahern@...il.com>
Cc:     "David S . Miller" <davem@...emloft.net>,
        Sabrina Dubroca <sd@...asysnail.net>,
        Steffen Klassert <steffen.klassert@...unet.com>,
        netdev@...r.kernel.org
Subject: Re: [PATCH net-next 3/9] selftests: pmtu: Introduce support for
 multiple tests

On Fri, 16 Mar 2018 12:53:23 -0700
David Ahern <dsahern@...il.com> wrote:

> On 3/16/18 12:29 PM, Stefano Brivio wrote:
> > This might be equally counter-intuitive for somebody. If one does a lot
> > of explicit error checking with early returns, my return convention is
> > also rather practical. E.g. in the setup() function I can do:
> > 
> > 	eval setup_${arg} && echo "  ${arg} not supported" && return 0
> > 
> > instead of:
> > 
> > 	eval setup_${arg} || { echo "  ${arg} not supported" && return 0; }  
> 
> I think it is weird to have 'a && b' where b is done when a fails as
> opposed to succeeds hence the comment. I think a common convention
> across scripts is important but having the tests is more so. Just a
> suggestion.

Yeah, also true. I'll change this.

> Look at fib_tests and fib-onlink-tests. As the number of tests grows,
> output consistency makes your life easier. With printf:
> 
> ...
> TEST: IPv4 linkdown flag set                                  [ OK ]
> TEST: IPv6 linkdown flag set                                  [ OK ]
> TEST: Directly connected nexthop, unicast address             [ OK ]
> TEST: Directly connected nexthop, unicast address with device [ OK ]
> TEST: Gateway is linklocal address                            [ OK ]
> TEST: Gateway is linklocal address, no device                 [ OK ]
> TEST: Gateway can not be local unicast address                [ OK ]
> TEST: Gateway can not be local unicast address, with device   [ OK ]
> TEST: Gateway can not be a local linklocal address            [ OK ]
> TEST: Gateway can be local address in a VRF                   [FAIL]
> TEST: Gateway can be local address in a VRF, with device      [FAIL]
> TEST: Gateway can be local linklocal address in a VRF         [ OK ]
> TEST: Redirect to VRF lookup                                  [ OK ]
> ...
> 
> the FAIL cases jump out versus echo
> 
> ...
> TEST: IPv4 linkdown flag set [ OK ]
> TEST: IPv6 linkdown flag set [ OK ]
> TEST: Directly connected nexthop, unicast address [ OK ]
> TEST: Directly connected nexthop, unicast address with device [ OK ]
> TEST: Gateway is linklocal address [ OK ]
> TEST: Gateway is linklocal address, no device [ OK ]
> TEST: Gateway can not be local unicast address [ OK ]
> TEST: Gateway can not be local unicast address, with device [ OK ]
> TEST: Gateway can not be a local linklocal address [ OK ]
> TEST: Gateway can be local address in a VRF [FAIL]
> TEST: Gateway can be local address in a VRF, with device [FAIL]
> TEST: Gateway can be local linklocal address in a VRF [ OK ]
> TEST: Redirect to VRF lookup [ OK ]
> ...
> 
> where your mind has a lot more work to do to find the tests broken by a
> change.
> 
> That is also why I chose OK versus PASS -- ok at 2 letters, fail at 4
> the failures really stand out.

I see your point. I'll use printf and make it a bit prettier in v2.

-- 
Stefano