diff mbox series

[2/2] network/in6_02: Don't use default value for LHOST_IFACES

Message ID 20180417124207.4561-2-pvorel@suse.cz
State Superseded
Delegated to: Alexey Kodanev
Headers show
Series None | expand

Commit Message

Petr Vorel April 17, 2018, 12:42 p.m. UTC
in6_02 can be test without help of network.sh. In that case LHOST_IFACES
is not defined and "eth0" is used as default value. This can cause
failures.

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
I'm not sure if we can make all C network tests make independent on
loading variables via tst_net.sh, but here it does not make much sense
to me to fail when it's not done.
---
 testcases/network/lib6/in6_02.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

Comments

Alexey Kodanev April 18, 2018, 2 p.m. UTC | #1
On 04/17/2018 03:42 PM, Petr Vorel wrote:
> in6_02 can be test without help of network.sh. In that case LHOST_IFACES
> is not defined and "eth0" is used as default value. This can cause
> failures.
>
I think the fix should be before the conversion to the new API, otherwise
it unnecessary depends on it.

> Signed-off-by: Petr Vorel <pvorel@suse.cz>
> ---
> I'm not sure if we can make all C network tests make independent on
> loading variables via tst_net.sh, but here it does not make much sense
> to me to fail when it's not done.
> ---
>  testcases/network/lib6/in6_02.c | 11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/testcases/network/lib6/in6_02.c b/testcases/network/lib6/in6_02.c
> index e3deb7b7b..c00e5dc0c 100644
> --- a/testcases/network/lib6/in6_02.c
> +++ b/testcases/network/lib6/in6_02.c
> @@ -33,14 +33,13 @@
>  
>  #define I2N_RNDCOUNT	10	/* random ints */
>  #define I2N_LOWCOUNT	10	/* sequential from 0 */
> -#define DEFAULT_IFACE	"eth0"
>  
>  static struct {
>  	char *name;
>  	int nonzero;
>  } test_case[] = {
>  	{ "lo", 1 },
> -	{ DEFAULT_IFACE, 1 },
> +	{ NULL, 1 },
>  	{ "hoser75", 0 },
>  	{ "6", 0 },
>  };
> @@ -61,6 +60,8 @@ void if_nametoindex_test(void)
>  	tst_res(TINFO, "IPv6 if_nametoindex() test");
>  
>  	for (i = 0; i < ARRAY_SIZE(test_case); ++i) {
> +		if (test_case[i].name == NULL)
> +			continue;


What about using TCONF if it skips the test-case because
LHOST_IFACE is not defined?

Thanks,
Alexey
Petr Vorel April 18, 2018, 2:13 p.m. UTC | #2
Hi Alexey,

> On 04/17/2018 03:42 PM, Petr Vorel wrote:
> > in6_02 can be test without help of network.sh. In that case LHOST_IFACES
> > is not defined and "eth0" is used as default value. This can cause
> > failures.

> I think the fix should be before the conversion to the new API, otherwise
> it unnecessary depends on it.
Agree, I'll fix it.

...
> >  	for (i = 0; i < ARRAY_SIZE(test_case); ++i) {
> > +		if (test_case[i].name == NULL)
> > +			continue;

> What about using TCONF if it skips the test-case because
> LHOST_IFACE is not defined?
I wanted to keep at least testing of if_nametoindex("lo").
Would be overkill in case of TCONF to create in6_02.sh which would just load tst_net.sh?
I don't like the fact, that network C tests depend on being run via network.sh (unlike
shell network tests).

> Thanks,
> Alexey


Kind regards,
Petr
Alexey Kodanev April 18, 2018, 2:56 p.m. UTC | #3
On 04/18/2018 05:13 PM, Petr Vorel wrote:
> Hi Alexey,
> 
>> On 04/17/2018 03:42 PM, Petr Vorel wrote:
>>> in6_02 can be test without help of network.sh. In that case LHOST_IFACES
>>> is not defined and "eth0" is used as default value. This can cause
>>> failures.
> 
>> I think the fix should be before the conversion to the new API, otherwise
>> it unnecessary depends on it.
> Agree, I'll fix it.
> 
> ...
>>>  	for (i = 0; i < ARRAY_SIZE(test_case); ++i) {
>>> +		if (test_case[i].name == NULL)
>>> +			continue;
> 
>> What about using TCONF if it skips the test-case because
>> LHOST_IFACE is not defined?
> I wanted to keep at least testing of if_nametoindex("lo").
> Would be overkill in case of TCONF to create in6_02.sh which would just load tst_net.sh?
> I don't like the fact, that network C tests depend on being run via network.sh (unlike
> shell network tests).

Why it depends on network.sh? LHOST_IFACE is just an environment variable.
It can be defined on the test machine already before the test is started,
or with tst_net.sh.

I mean, using TCONF for the LHOST_IFACE test-case, when the test in6_02,
doesn't find the variable and skips it, then proceeds with the rest of
test-cases.
Petr Vorel April 20, 2018, 8:15 a.m. UTC | #4
Hi Alexey,

> >> What about using TCONF if it skips the test-case because
> >> LHOST_IFACE is not defined?
> > I wanted to keep at least testing of if_nametoindex("lo").
> > Would be overkill in case of TCONF to create in6_02.sh which would just load tst_net.sh?
> > I don't like the fact, that network C tests depend on being run via network.sh (unlike
> > shell network tests).

> Why it depends on network.sh? LHOST_IFACE is just an environment variable.
> It can be defined on the test machine already before the test is started,
> or with tst_net.sh.
Yes, but that's what I doesn't like: you need to source tst_net.sh (either with network.sh
or manually). Not a big issue, it's just you shouldn't run C tests like in6_02 without
variables. This is different from other non-networking tests, which you can run as they
are (without any preparation, setup).
This is the reason I was thinking about wrapping any C test which need variables with
shell script which source tst_net.sh for it.

Kind regards,
Petr
diff mbox series

Patch

diff --git a/testcases/network/lib6/in6_02.c b/testcases/network/lib6/in6_02.c
index e3deb7b7b..c00e5dc0c 100644
--- a/testcases/network/lib6/in6_02.c
+++ b/testcases/network/lib6/in6_02.c
@@ -33,14 +33,13 @@ 
 
 #define I2N_RNDCOUNT	10	/* random ints */
 #define I2N_LOWCOUNT	10	/* sequential from 0 */
-#define DEFAULT_IFACE	"eth0"
 
 static struct {
 	char *name;
 	int nonzero;
 } test_case[] = {
 	{ "lo", 1 },
-	{ DEFAULT_IFACE, 1 },
+	{ NULL, 1 },
 	{ "hoser75", 0 },
 	{ "6", 0 },
 };
@@ -61,6 +60,8 @@  void if_nametoindex_test(void)
 	tst_res(TINFO, "IPv6 if_nametoindex() test");
 
 	for (i = 0; i < ARRAY_SIZE(test_case); ++i) {
+		if (test_case[i].name == NULL)
+			continue;
 		TEST(if_nametoindex(test_case[i].name));
 		if (!TEST_RETURN != !test_case[i].nonzero) {
 			tst_res(TFAIL, "if_nametoindex(\"%s\") %ld [should be %szero]",
@@ -242,16 +243,16 @@  void setup(void)
 	char *ifnames = getenv("LHOST_IFACES");
 
 	if (!ifnames) {
-		tst_res(TWARN, "LHOST_IFACES not defined, default to %s", DEFAULT_IFACE);
+		tst_res(TINFO, "LHOST_IFACES not defined, it will not be tested");
 		return;
 	}
 
 	static char name[256];
 	sscanf(ifnames, "%255s", name);
-	if (!strcmp(name, test_case[1].name))
+	if (!strcmp(name, test_case[0].name))
 		return;
 
-	tst_res(TINFO, "change default '%s' name to '%s'", DEFAULT_IFACE, name);
+	tst_res(TINFO, "LHOST_IFACES \"%s\"", name);
 	test_case[1].name = name;
 }