* [PATCH] gdb/testsuite: add untested message to skip_debuginfod_tests
@ 2022-12-09 11:56 Enze Li
2022-12-12 12:44 ` Bruno Larsen
2022-12-13 16:14 ` Tom Tromey
0 siblings, 2 replies; 5+ messages in thread
From: Enze Li @ 2022-12-09 11:56 UTC (permalink / raw)
To: gdb-patches; +Cc: enze.li
When running the fetch_src_and_symbols.exp, I see the following output
which looks weird. I even don't know whether the execution was successful.
======
Running /home/lee/dev/binutils-gdb/gdb/testsuite/gdb.debuginfod/fetch_src_and_symbols.exp ...
testcase /home/lee/dev/binutils-gdb/gdb/testsuite/gdb.debuginfod/fetch_src_and_symbols.exp
completed in 0 seconds
=== gdb Summary ===
======
It is the fact that there's not enough feedback provided during the
skip_debuginfod_tests procedure. Fix this by adding some untested
messages to the skip_debuginfod_tests procedure to clarify the output.
With this patch applied, I get:
======
Running /home/lee/dev/binutils-gdb/gdb/testsuite/gdb.debuginfod/fetch_src_and_symbols.exp ...
UNTESTED: gdb.debuginfod/fetch_src_and_symbols.exp: cannot find debuginfo
testcase /home/lee/dev/binutils-gdb/gdb/testsuite/gdb.debuginfod/fetch_src_and_symbols.exp
completed in 0 seconds
=== gdb Summary ===
# of untested testcases 1
======
I also modernized the form of 'if' with 'if {} {}' in
skip_debuginfod_tests.
Tested on x86_64-linux.
---
gdb/testsuite/lib/debuginfod-support.exp | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/gdb/testsuite/lib/debuginfod-support.exp b/gdb/testsuite/lib/debuginfod-support.exp
index ceecf9071110..c1a2bb900dce 100644
--- a/gdb/testsuite/lib/debuginfod-support.exp
+++ b/gdb/testsuite/lib/debuginfod-support.exp
@@ -18,11 +18,13 @@
# Return true if the debuginfod tests should be skipped, otherwise, return
# false.
proc skip_debuginfod_tests {} {
- if [is_remote host] {
+ if { [is_remote host] } {
+ untested "does not work on remote host"
return true
}
if { [which debuginfod] == 0 } {
+ untested "cannot find debuginfo"
return true
}
@@ -39,6 +41,7 @@ proc skip_debuginfod_tests {} {
if { [string first "with-debuginfod" \
[eval exec $::GDB --quiet $::INTERNAL_GDBFLAGS \
--configuration]] == -1 } {
+ untested "GDB was not configured with debuginfod"
return true
}
base-commit: cd3866b6d07b37258eb840443537baa163877e24
--
2.30.2
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] gdb/testsuite: add untested message to skip_debuginfod_tests
2022-12-09 11:56 [PATCH] gdb/testsuite: add untested message to skip_debuginfod_tests Enze Li
@ 2022-12-12 12:44 ` Bruno Larsen
2022-12-13 16:14 ` Tom Tromey
1 sibling, 0 replies; 5+ messages in thread
From: Bruno Larsen @ 2022-12-12 12:44 UTC (permalink / raw)
To: Enze Li, gdb-patches; +Cc: enze.li
On 09/12/2022 12:56, Enze Li via Gdb-patches wrote:
> When running the fetch_src_and_symbols.exp, I see the following output
> which looks weird. I even don't know whether the execution was successful.
> ======
> Running /home/lee/dev/binutils-gdb/gdb/testsuite/gdb.debuginfod/fetch_src_and_symbols.exp ...
> testcase /home/lee/dev/binutils-gdb/gdb/testsuite/gdb.debuginfod/fetch_src_and_symbols.exp
> completed in 0 seconds
>
> === gdb Summary ===
>
> ======
>
> It is the fact that there's not enough feedback provided during the
> skip_debuginfod_tests procedure. Fix this by adding some untested
> messages to the skip_debuginfod_tests procedure to clarify the output.
>
> With this patch applied, I get:
> ======
> Running /home/lee/dev/binutils-gdb/gdb/testsuite/gdb.debuginfod/fetch_src_and_symbols.exp ...
> UNTESTED: gdb.debuginfod/fetch_src_and_symbols.exp: cannot find debuginfo
> testcase /home/lee/dev/binutils-gdb/gdb/testsuite/gdb.debuginfod/fetch_src_and_symbols.exp
> completed in 0 seconds
>
> === gdb Summary ===
>
> # of untested testcases 1
> ======
>
> I also modernized the form of 'if' with 'if {} {}' in
> skip_debuginfod_tests.
Good catch!
Reviewed-By: Bruno Larsen <blarsen@redhat.com>
--
Cheers,
Bruno
> Tested on x86_64-linux.
> ---
> gdb/testsuite/lib/debuginfod-support.exp | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/gdb/testsuite/lib/debuginfod-support.exp b/gdb/testsuite/lib/debuginfod-support.exp
> index ceecf9071110..c1a2bb900dce 100644
> --- a/gdb/testsuite/lib/debuginfod-support.exp
> +++ b/gdb/testsuite/lib/debuginfod-support.exp
> @@ -18,11 +18,13 @@
> # Return true if the debuginfod tests should be skipped, otherwise, return
> # false.
> proc skip_debuginfod_tests {} {
> - if [is_remote host] {
> + if { [is_remote host] } {
> + untested "does not work on remote host"
> return true
> }
>
> if { [which debuginfod] == 0 } {
> + untested "cannot find debuginfo"
> return true
> }
>
> @@ -39,6 +41,7 @@ proc skip_debuginfod_tests {} {
> if { [string first "with-debuginfod" \
> [eval exec $::GDB --quiet $::INTERNAL_GDBFLAGS \
> --configuration]] == -1 } {
> + untested "GDB was not configured with debuginfod"
> return true
> }
>
>
> base-commit: cd3866b6d07b37258eb840443537baa163877e24
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] gdb/testsuite: add untested message to skip_debuginfod_tests
2022-12-09 11:56 [PATCH] gdb/testsuite: add untested message to skip_debuginfod_tests Enze Li
2022-12-12 12:44 ` Bruno Larsen
@ 2022-12-13 16:14 ` Tom Tromey
2022-12-14 7:43 ` Enze Li
1 sibling, 1 reply; 5+ messages in thread
From: Tom Tromey @ 2022-12-13 16:14 UTC (permalink / raw)
To: Enze Li via Gdb-patches; +Cc: Enze Li, enze.li
>>>>> "Enze" == Enze Li via Gdb-patches <gdb-patches@sourceware.org> writes:
Enze> proc skip_debuginfod_tests {} {
Enze> - if [is_remote host] {
Enze> + if { [is_remote host] } {
Enze> + untested "does not work on remote host"
Enze> return true
Enze> }
Do any of the other skip_ procs call untested?
I suspect this should be done somewhere else, and/or it's a pervasive
problem when a prerequisite is not met.
Tom
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] gdb/testsuite: add untested message to skip_debuginfod_tests
2022-12-13 16:14 ` Tom Tromey
@ 2022-12-14 7:43 ` Enze Li
2022-12-14 17:19 ` Tom Tromey
0 siblings, 1 reply; 5+ messages in thread
From: Enze Li @ 2022-12-14 7:43 UTC (permalink / raw)
To: Tom Tromey; +Cc: Enze Li via Gdb-patches, enze.li
On Tue, Dec 13 2022 at 09:14:47 AM -0700, Tom Tromey wrote:
>>>>>> "Enze" == Enze Li via Gdb-patches <gdb-patches@sourceware.org> writes:
>
> Enze> proc skip_debuginfod_tests {} {
> Enze> - if [is_remote host] {
> Enze> + if { [is_remote host] } {
> Enze> + untested "does not work on remote host"
> Enze> return true
> Enze> }
Thanks for your swift replay. :)
>
> Do any of the other skip_ procs call untested?
Hmmm...Not many. Actually, from the skip_debuginfod_tests, I see an
untested message "cannot find curl". It provides detailed information
when running the testcase. Then I thought other spots in the same
procedure may benifit from it.
lib/debuginfod-support.exp:proc skip_debuginfod_tests {} {
lib/debuginfod-support.exp- if [is_remote host] {
lib/debuginfod-support.exp- return true
lib/debuginfod-support.exp- }
lib/debuginfod-support.exp-
lib/debuginfod-support.exp- if { [which debuginfod] == 0 } {
lib/debuginfod-support.exp- return true
lib/debuginfod-support.exp- }
lib/debuginfod-support.exp-
lib/debuginfod-support.exp- if { [which curl] == 0 } {
lib/debuginfod-support.exp- untested "cannot find curl"
lib/debuginfod-support.exp- return true
lib/debuginfod-support.exp- }
Except this, three unsupported messages were found in skip_* procedure.
> I suspect this should be done somewhere else, and/or it's a pervasive
> problem when a prerequisite is not met.
>
> Tom
If the way of my previous patch is unconventional, how about this?
=================================================================
--- a/gdb/testsuite/gdb.debuginfod/fetch_src_and_symbols.exp
+++ b/gdb/testsuite/gdb.debuginfod/fetch_src_and_symbols.exp
@@ -20,7 +20,24 @@ standard_testfile main.c
load_lib dwarf.exp
load_lib debuginfod-support.exp
-if { [skip_debuginfod_tests] } { return -1 }
+set skip_ret [skip_debuginfod_tests]
+if { $skip_ret < 0 } {
+ switch $skip_ret {
+ -1 {
+ untested "does not work on remote host"
+ }
+ -2 {
+ untested "cannot find debuginfo"
+ }
+ -3 {
+ untested "cannot find curl"
+ }
+ -4 {
+ untested "GDB was not configured with debuginfod"
+ }
+ }
+ return -1
+}
set sourcetmp [standard_output_file tmp-${srcfile}]
set outputdir [standard_output_file {}]
--- a/gdb/testsuite/lib/debuginfod-support.exp
+++ b/gdb/testsuite/lib/debuginfod-support.exp
@@ -18,17 +18,16 @@
# Return true if the debuginfod tests should be skipped, otherwise, return
# false.
proc skip_debuginfod_tests {} {
- if [is_remote host] {
- return true
+ if { [is_remote host] } {
+ return -1
}
if { [which debuginfod] == 0 } {
- return true
+ return -2
}
if { [which curl] == 0 } {
- untested "cannot find curl"
- return true
+ return -3
}
# Skip testing if gdb was not configured with debuginfod.
@@ -39,10 +38,10 @@ proc skip_debuginfod_tests {} {
if { [string first "with-debuginfod" \
[eval exec $::GDB --quiet $::INTERNAL_GDBFLAGS \
--configuration]] == -1 } {
- return true
+ return -4
}
- return false
+ return 0
}
# Create two directories within the current output directory. One directory
=================================================================
Best Regards,
Enze
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] gdb/testsuite: add untested message to skip_debuginfod_tests
2022-12-14 7:43 ` Enze Li
@ 2022-12-14 17:19 ` Tom Tromey
0 siblings, 0 replies; 5+ messages in thread
From: Tom Tromey @ 2022-12-14 17:19 UTC (permalink / raw)
To: Enze Li via Gdb-patches; +Cc: Tom Tromey, Enze Li, enze.li
>>>>> "Enze" == Enze Li via Gdb-patches <gdb-patches@sourceware.org> writes:
Enze> Hmmm...Not many. Actually, from the skip_debuginfod_tests, I see an
Enze> untested message "cannot find curl". It provides detailed information
Enze> when running the testcase. Then I thought other spots in the same
Enze> procedure may benifit from it.
Personally I think these predicates should just return the result. If
a message would be helpful, they can use 'verbose'.
Enze> If the way of my previous patch is unconventional, how about this?
I think it's kind of overkill. "verbose" in the proc would work just as
well, and I suppose my feeling (unsupported by fact really) is that
normally an "untested" is just ignored anyway, and if you want to find
out what's happening you would have to dig a bit anyway.
Also I question if "untested" is correct due to what the dejagnu manual
says:
UNTESTED
A test was not run. This is a placeholder used when there is no
real test case yet.
I think "unsupported" is what we ought to be using.
If you want to see how I think it should generally work, look at my
branch "rewrite-require". This changes "require" to be simpler and then
tests can express their static requirements very simply:
require dwarf2 !isnative mumble bleah
Then the main difficulty is converting all the code to use this.
I made a stab at it there.
Tom
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-12-14 17:19 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-12-09 11:56 [PATCH] gdb/testsuite: add untested message to skip_debuginfod_tests Enze Li
2022-12-12 12:44 ` Bruno Larsen
2022-12-13 16:14 ` Tom Tromey
2022-12-14 7:43 ` Enze Li
2022-12-14 17:19 ` Tom Tromey
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).