[LTP] [PATCH 1/2] lib: Add known-fail test tag

[LTP] [PATCH 1/2] lib: Add known-fail test tag

[Petr Vorel]

to save test reviewers time.

Suggested-by: Cyril Hrubis <chrubis@suse.cz>
Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
 doc/c-test-api.txt | 1 +
 lib/tst_test.c     | 8 ++++++--
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/doc/c-test-api.txt b/doc/c-test-api.txt
index 6481531cf2..f8ee4de0d1 100644
--- a/doc/c-test-api.txt
+++ b/doc/c-test-api.txt
@@ -2259,6 +2259,7 @@ struct tst_test test = {
 	.tags = (const struct tst_tag[]) {
 		{"linux-git", "9392a27d88b9"},
 		{"linux-git", "ff002b30181d"},
+		{"known-fail", "ustat() is known to fail with EINVAL on Btrfs"},
 		{"linux-stable-git", "c4a23c852e80"},
 		{"CVE", "2020-29373"},
 		{}
diff --git a/lib/tst_test.c b/lib/tst_test.c
index 23fc0ebf48..901b363704 100644
--- a/lib/tst_test.c
+++ b/lib/tst_test.c
@@ -762,10 +762,13 @@ static void print_failure_hint(const char *tag, const char *hint,
 				hint_printed = 1;
 				fprintf(stderr, "\n");
 				print_colored("HINT: ");
-				fprintf(stderr, "You _MAY_ be %s, see:\n\n", hint);
+				fprintf(stderr, "You _MAY_ be %s:\n\n", hint);
 			}
 
-			fprintf(stderr, "%s%s\n", url, tags[i].value);
+			if (url)
+				fprintf(stderr, "%s%s\n", url, tags[i].value);
+			else
+				fprintf(stderr, "%s\n", tags[i].value);
 		}
 	}
 }
@@ -778,6 +781,7 @@ static void print_failure_hints(void)
 					   LINUX_STABLE_GIT_URL);
 	print_failure_hint("glibc-git", "missing glibc fixes", GLIBC_GIT_URL);
 	print_failure_hint("CVE", "vulnerable to CVE(s)", CVE_DB_URL);
+	print_failure_hint("known-fail", "hit by known kernel failures", NULL);
 }
 
 static void do_exit(int ret)
-- 
2.34.1


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH 2/2] ustat0{1, 2}: Mark failure on Btrfs with known-fail tag

[Petr Vorel]

ustat() has been always broken on Btrfs and it's unlikely to be fixed
due ustat() being deprecated (glibc doesn't expose this system call
anymore).

Suggested-by: Cyril Hrubis <chrubis@suse.cz>
Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
 testcases/kernel/syscalls/ustat/ustat01.c | 4 ++++
 testcases/kernel/syscalls/ustat/ustat02.c | 4 ++++
 2 files changed, 8 insertions(+)

diff --git a/testcases/kernel/syscalls/ustat/ustat01.c b/testcases/kernel/syscalls/ustat/ustat01.c
index 0252858beb..e314649e8d 100644
--- a/testcases/kernel/syscalls/ustat/ustat01.c
+++ b/testcases/kernel/syscalls/ustat/ustat01.c
@@ -44,6 +44,10 @@ static void setup(void)
 static struct tst_test test = {
 	.test_all = run,
 	.setup = setup,
+	.tags = (const struct tst_tag[]) {
+		{"known-fail", "ustat() is known to fail with EINVAL on Btrfs"},
+		{}
+	}
 };
 #else
 TST_TEST_TCONF("testing ustat requires <sys/ustat.h> or <linux/types.h>");
diff --git a/testcases/kernel/syscalls/ustat/ustat02.c b/testcases/kernel/syscalls/ustat/ustat02.c
index d08446eaa1..67d2918f83 100644
--- a/testcases/kernel/syscalls/ustat/ustat02.c
+++ b/testcases/kernel/syscalls/ustat/ustat02.c
@@ -63,6 +63,10 @@ static struct tst_test test = {
 	.test = run,
 	.setup = setup,
 	.tcnt = ARRAY_SIZE(tc),
+	.tags = (const struct tst_tag[]) {
+		{"known-fail", "ustat() is known to fail with EINVAL on Btrfs"},
+		{}
+	}
 };
 #else
 TST_TEST_TCONF("testing ustat requires <sys/ustat.h> or <linux/types.h>");
-- 
2.34.1


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 2/2] ustat0{1, 2}: Mark failure on Btrfs with known-fail tag

[Cyril Hrubis]

Hi!
> --- a/testcases/kernel/syscalls/ustat/ustat01.c
> +++ b/testcases/kernel/syscalls/ustat/ustat01.c
> @@ -44,6 +44,10 @@ static void setup(void)
>  static struct tst_test test = {
>  	.test_all = run,
>  	.setup = setup,
> +	.tags = (const struct tst_tag[]) {
> +		{"known-fail", "ustat() is known to fail with EINVAL on Btrfs"},
                                   ^
				   Maybe we should just remove the
				   "known to fail" from these messages.

Since the code in the library will print:

---
You _MAY_ be hit by known kernel failures:

ustat() is known to fail with EINVAL on Btrfs
---

so the 'known to fail' part is redundant here.

What about:

---
You _MAY_ be hit by known kernel failures:

ustat() fails with EINVAL on Btrfs
---

Other than this I think that this is a good way how to put these kind of
hints into the testcases.


Also I do wonder if there is a good URL that would contain more verbose
description of the problem but I guess there isn't. Well maybe this one:

https://www.mail-archive.com/linux-btrfs@vger.kernel.org/msg95697.html

Not sure how to encode that, maybe just add it to the test top level
comment?

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 2/2] ustat0{1, 2}: Mark failure on Btrfs with known-fail tag

[Petr Vorel]

Hi Cyril,

> > +	.tags = (const struct tst_tag[]) {
> > +		{"known-fail", "ustat() is known to fail with EINVAL on Btrfs"},
>                                    ^
> 				   Maybe we should just remove the
> 				   "known to fail" from these messages.

> Since the code in the library will print:

> ---
> You _MAY_ be hit by known kernel failures:

> ustat() is known to fail with EINVAL on Btrfs
> ---

> so the 'known to fail' part is redundant here.

> What about:

> ---
> You _MAY_ be hit by known kernel failures:

> ustat() fails with EINVAL on Btrfs
> ---

+1

> Other than this I think that this is a good way how to put these kind of
> hints into the testcases.


> Also I do wonder if there is a good URL that would contain more verbose
> description of the problem but I guess there isn't. Well maybe this one:

> https://www.mail-archive.com/linux-btrfs@vger.kernel.org/msg95697.html
Let's use lore.
https://lore.kernel.org/linux-btrfs/CAB=NE6X2-mbZwVFnKUwjRmTGp3auZFHQXJ1h_YTJ2driUeoR+A@mail.gmail.com/

But I'd put Josef Bacik's reply as he's a maintainer:
https://lore.kernel.org/linux-btrfs/e7e867b8-b57a-7eb2-2432-1627bd3a88fb@toxicpanda.com/

> Not sure how to encode that, maybe just add it to the test top level
> comment?
I'd prefer to have it in metadata html/pdf and in known-bug section.
Thus I'd prefer:

	.tags = (const struct tst_tag[]) {
		{"known-fail", "ustat() is known to fail with EINVAL on Btrfs, see "
			"https://lore.kernel.org/linux-btrfs/e7e867b8-b57a-7eb2-2432-1627bd3a88fb@toxicpanda.com/"
		},
		{}

But while this is obviously ok for test output:

$ ./ustat01 -h
...
Tags
----
known-fail: ustat() is known to fail with EINVAL on Btrfs, see https://lore.kernel.org/linux-btrfs/e7e867b8-b57a-7eb2-2432-1627bd3a88fb@toxicpanda.com/

It gets parsed as new line:

  "ustat01": {
   "tags": [
     [
      "known-fail",
      "ustat() is known to fail with EINVAL on Btrfs, see ",
      "https://lore.kernel.org/linux-btrfs/e7e867b8-b57a-7eb2-2432-1627bd3a88fb@toxicpanda.com/"
     ]
    ],
   "fname": "testcases/kernel/syscalls/ustat/ustat01.c"
  }

Unless you think it's useful to expect that tag values can have longer input and
thus testinfo.pl should use all array items (except the first which is key),
I'd go the easy way and put the link only to top level docparse section as you
suggested.


Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 2/2] ustat0{1, 2}: Mark failure on Btrfs with known-fail tag

[Cyril Hrubis]

Hi!
> I'd prefer to have it in metadata html/pdf and in known-bug section.
> Thus I'd prefer:
> 
> 	.tags = (const struct tst_tag[]) {
> 		{"known-fail", "ustat() is known to fail with EINVAL on Btrfs, see "
> 			"https://lore.kernel.org/linux-btrfs/e7e867b8-b57a-7eb2-2432-1627bd3a88fb@toxicpanda.com/"
> 		},
> 		{}
> 
> But while this is obviously ok for test output:
> 
> $ ./ustat01 -h
> ...
> Tags
> ----
> known-fail: ustat() is known to fail with EINVAL on Btrfs, see https://lore.kernel.org/linux-btrfs/e7e867b8-b57a-7eb2-2432-1627bd3a88fb@toxicpanda.com/
> 
> It gets parsed as new line:
> 
>   "ustat01": {
>    "tags": [
>      [
>       "known-fail",
>       "ustat() is known to fail with EINVAL on Btrfs, see ",
>       "https://lore.kernel.org/linux-btrfs/e7e867b8-b57a-7eb2-2432-1627bd3a88fb@toxicpanda.com/"
>      ]
>     ],
>    "fname": "testcases/kernel/syscalls/ustat/ustat01.c"
>   }
> 
> Unless you think it's useful to expect that tag values can have longer input and
> thus testinfo.pl should use all array items (except the first which is key),
> I'd go the easy way and put the link only to top level docparse section as you
> suggested.

I guess that the URL in the tag field is okay too. It makes the line
slightly longer, but at the same time it's more descriptive.

Anyway, can we please still consider this for the release?

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 2/2] ustat0{1, 2}: Mark failure on Btrfs with known-fail tag

[Petr Vorel]

> Hi!
> > I'd prefer to have it in metadata html/pdf and in known-bug section.
> > Thus I'd prefer:

> > 	.tags = (const struct tst_tag[]) {
> > 		{"known-fail", "ustat() is known to fail with EINVAL on Btrfs, see "
> > 			"https://lore.kernel.org/linux-btrfs/e7e867b8-b57a-7eb2-2432-1627bd3a88fb@toxicpanda.com/"
> > 		},
> > 		{}

> > But while this is obviously ok for test output:

> > $ ./ustat01 -h
> > ...
> > Tags
> > ----
> > known-fail: ustat() is known to fail with EINVAL on Btrfs, see https://lore.kernel.org/linux-btrfs/e7e867b8-b57a-7eb2-2432-1627bd3a88fb@toxicpanda.com/

> > It gets parsed as new line:

> >   "ustat01": {
> >    "tags": [
> >      [
> >       "known-fail",
> >       "ustat() is known to fail with EINVAL on Btrfs, see ",
> >       "https://lore.kernel.org/linux-btrfs/e7e867b8-b57a-7eb2-2432-1627bd3a88fb@toxicpanda.com/"
> >      ]
> >     ],
> >    "fname": "testcases/kernel/syscalls/ustat/ustat01.c"
> >   }

> > Unless you think it's useful to expect that tag values can have longer input and
> > thus testinfo.pl should use all array items (except the first which is key),
> > I'd go the easy way and put the link only to top level docparse section as you
> > suggested.

> I guess that the URL in the tag field is okay too. It makes the line
> slightly longer, but at the same time it's more descriptive.

> Anyway, can we please still consider this for the release?
I'll try to post tonight or on Monday with fix for testinfo.pl which use all
array items for value (not just the second).

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp