From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx3-rdu2.redhat.com ([66.187.233.73]:36454 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751681AbeCPUD0 (ORCPT ); Fri, 16 Mar 2018 16:03:26 -0400 Date: Fri, 16 Mar 2018 21:03:20 +0100 From: Stefano Brivio To: David Ahern Cc: "David S . Miller" , Sabrina Dubroca , Steffen Klassert , netdev@vger.kernel.org Subject: Re: [PATCH net-next 3/9] selftests: pmtu: Introduce support for multiple tests Message-ID: <20180316210320.48a1b17e@epycfail> In-Reply-To: <626b0eb0-846a-b00e-58a2-d5c2e3b24a95@gmail.com> References: <2870b8f3ab9ce40d3dbcfc1f308aba580f59197f.1521129192.git.sbrivio@redhat.com> <20180316202920.3dc280be@epycfail> <626b0eb0-846a-b00e-58a2-d5c2e3b24a95@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: netdev-owner@vger.kernel.org List-ID: On Fri, 16 Mar 2018 12:53:23 -0700 David Ahern 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