[Bug testsuite/29778] New: [gdb/testsuite] Revise untested usage

[Bug testsuite/29778] New: [gdb/testsuite] Revise untested usage

[vries at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=29778

            Bug ID: 29778
           Summary: [gdb/testsuite] Revise untested usage
           Product: gdb
           Version: HEAD
            Status: NEW
          Severity: enhancement
          Priority: P2
         Component: testsuite
          Assignee: unassigned at sourceware dot org
          Reporter: vries at gcc dot gnu.org
  Target Milestone: ---

The definitions/explanations for UNTESTED in the dejagnu manual are:
...
A test case is not yet complete, and in particular cannot yet produce a PASS or
FAIL. You can also use this outcome in dummy “tests” that note explicitly the
absence of a real test case for a particular property.
...
and:
...
untested
prints a message for an test case that isn’t run for some technical reason. 
...
and:
...
untested
prints a message for an test case that isn’t run for some reason.
...
and:
...
UNTESTED
A test was not run. This is a placeholder used when there is no real test case
yet.
...
and:
...
For example, you might use this in a dummy test whose only role is to record
that a test does not yet exist for some feature.
...

I don't think the definition is very clear, but as I understand it the valid
uses of UNTESTED are:
...
gdb.base/features.exp: PASS: feature a
gdb.base/features.exp: UNTESTED: feature b
...
or:
...
gdb.base/feature-b: UNTESTED: <feature b in more detail>
...

In the gdb testsuite there's an interpretation of UNTESTED as "not having
progressed sufficiently in executing an otherwise complete test-case to
generate a PASS or FAIL", which explains why it's used for say failure to
compile.

I suspect this usage is wrong, but again given the vague definition I'm not
sure.

Anyway, perhaps given the vague definition, I think we should eliminate the
usage of untested, by redefining it to:
...
proc untested { message } {
  error "untested: $message"
}
...
and fixing the errors.

Note that out of a total of:
...
$ egrep -c UNTESTED: gdb.sum 
35
...
I have:
...
$ egrep -c UNTESTED:.*(not supported|unsupported) gdb.sum 
18
...
which probably should use unsupported instead of untested.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug testsuite/29778] [gdb/testsuite] Revise untested usage

[vries at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=29778

--- Comment #1 from Tom de Vries <vries at gcc dot gnu.org> ---
Hmm, reading the definition of UNSUPPORTED:
...
There is no support for the tested case. This may mean that a conditional
feature of an operating system, or of a compiler, is not implemented. DejaGnu
also uses this message when a testing environment (often a “bare board” target)
lacks basic support for compiling or running the test case. For example, a test
for the system subroutine gethostname would never work on a target board
running only a boot monitor.
...
and seeing how like XFAIL it's focused on the environment, I start to wonder if
the role of UNTESTED was intended to be that it's to UNSUPPORTED what KFAIL is
to XFAIL.

In other words:
- xfail: fail due to problem in environment
- kfail: fail due to problem in gdb
- unsupported: not run due to missing optional feature in environment
- untested: not run due to missing optional feature in gdb

That way, each one implies one type of action to achieve a passing test-case:
- xfail: fix environment
- kfail: fix gdb
- unsupported: add missing feature to environment
- untested: build gdb with missing feature enabled

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug testsuite/29778] [gdb/testsuite] Revise untested usage

[vries at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=29778

--- Comment #2 from Tom de Vries <vries at gcc dot gnu.org> ---
And then there are the test-cases that use proc skip_ variants which largely
seem happy to skip only doing "verbose $msg" or just silently.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug testsuite/29778] [gdb/testsuite] Revise untested usage

[simon.marchi at polymtl dot ca]

https://sourceware.org/bugzilla/show_bug.cgi?id=29778

Simon Marchi <simon.marchi at polymtl dot ca> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |simon.marchi at polymtl dot ca

--- Comment #3 from Simon Marchi <simon.marchi at polymtl dot ca> ---
(In reply to Tom de Vries from comment #1)
> Hmm, reading the definition of UNSUPPORTED:
> ...
> There is no support for the tested case. This may mean that a conditional
> feature of an operating system, or of a compiler, is not implemented. DejaGnu
> also uses this message when a testing environment (often a “bare board”
> target)
> lacks basic support for compiling or running the test case. For example, a
> test
> for the system subroutine gethostname would never work on a target board
> running only a boot monitor.
> ...
> and seeing how like XFAIL it's focused on the environment, I start to wonder
> if the role of UNTESTED was intended to be that it's to UNSUPPORTED what
> KFAIL is to XFAIL.
> 
> In other words:
> - xfail: fail due to problem in environment
> - kfail: fail due to problem in gdb
> - unsupported: not run due to missing optional feature in environment
> - untested: not run due to missing optional feature in gdb

I'm struggling a bit with the difference between unsupported and untested,
especially because unsupported sounds like a specific case of untested.  And
except because of what the Dejagnu documentation says, unsupported really
sounds like it could be used for both "missing feature in environment" and
"missing feature in gdb".  Using unsupported just for the former one seems
arbitrary.

> And then there are the test-cases that use proc skip_ variants which largely seem happy to skip only doing "verbose $msg" or just silently.

I think all tests should at least record at least one untested or unsupported,
so that there's a trace in gdb.sum.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug testsuite/29778] [gdb/testsuite] Revise untested usage

[vries at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=29778

--- Comment #4 from Tom de Vries <vries at gcc dot gnu.org> ---
(In reply to Simon Marchi from comment #3)
> > And then there are the test-cases that use proc skip_ variants which largely seem happy to skip only doing "verbose $msg" or just silently.
> 
> I think all tests should at least record at least one untested or
> unsupported, so that there's a trace in gdb.sum.

I think that's an ok principle, but once the volume grows the usefulness is
reduced.

I think it's fine to avoid generating noise.  For instance, I would consider it
noise to record that a ppc64-specific test is not run on x86_64.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug testsuite/29778] [gdb/testsuite] Revise untested usage

[vries at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=29778

--- Comment #5 from Tom de Vries <vries at gcc dot gnu.org> ---
(In reply to Simon Marchi from comment #3)
> > and seeing how like XFAIL it's focused on the environment, I start to wonder
> > if the role of UNTESTED was intended to be that it's to UNSUPPORTED what
> > KFAIL is to XFAIL.
> > 
> > In other words:
> > - xfail: fail due to problem in environment
> > - kfail: fail due to problem in gdb
> > - unsupported: not run due to missing optional feature in environment
> > - untested: not run due to missing optional feature in gdb
> 
> I'm struggling a bit with the difference between unsupported and untested,
> especially because unsupported sounds like a specific case of untested.  And
> except because of what the Dejagnu documentation says, unsupported really
> sounds like it could be used for both "missing feature in environment" and
> "missing feature in gdb".  Using unsupported just for the former one seems
> arbitrary.
>  

I know some struggle with the difference between kfail and xfail and feel it's
arbitrary.  For me the difference is clear: for a kfail you need to record a
gdb PR.  For an xfail, you don't.

Between unsupported and untested, I agree what I proposed here can bee seen as
arbitrary, it's just an attempt to make sense of the available incomplete
information.

All I care about is not having to wonder each time whether to use untested or
unsupported, either by:
- having clear rules when to use them, or
- eliminated the use of one of them by generating an error, or
- simply mapping one onto the other.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug testsuite/29778] [gdb/testsuite] Revise untested usage

[vries at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=29778

--- Comment #6 from Tom de Vries <vries at gcc dot gnu.org> ---
(In reply to Tom de Vries from comment #5)
> - simply mapping one onto the other.

Say:
...
proc unsupported { message } {
   untested "unsupported: $message"
}
...

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug testsuite/29778] [gdb/testsuite] Revise untested usage

[vries at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=29778

--- Comment #7 from Tom de Vries <vries at gcc dot gnu.org> ---
(In reply to Tom de Vries from comment #4)
> (In reply to Simon Marchi from comment #3)
> > > And then there are the test-cases that use proc skip_ variants which largely seem happy to skip only doing "verbose $msg" or just silently.
> > 
> > I think all tests should at least record at least one untested or
> > unsupported, so that there's a trace in gdb.sum.
> 
> I think that's an ok principle, but once the volume grows the usefulness is
> reduced.
> 
> I think it's fine to avoid generating noise.  For instance, I would consider
> it noise to record that a ppc64-specific test is not run on x86_64.

And we could also avoid noise by emitting one untested/unsupported per skip
group.  I don't need to know that gdb.server/*.exp is skipped for each
test-case, if gdbserver is unavailable.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug testsuite/29778] [gdb/testsuite] Revise untested usage

[tromey at sourceware dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=29778

Tom Tromey <tromey at sourceware dot org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |tromey at sourceware dot org

--- Comment #8 from Tom Tromey <tromey at sourceware dot org> ---
I wasn't aware of this bug when I wrote the "require" series.

I also stumbled over the weird text in dejagnu that made me
that "untested" was essentially useless.  So, I changed require
to use unsupported.

On reflection though, I think what dejagnu says isn't super important.
We can just pick the one we like for gdb.

I tend to think the noise is fine.  My reasoning is that these
procs give us a way to see not just that a test didn't run, but
also why it didn't run.  Occasionally this reason could be surprising
and lead to some kind of bug fix.  So my view is that the silent
"return"s in the test cases should be converted.

Centralizing these decisions with "require" or other "do task and
return on failure" procs makes this simpler to achieve.
(Here I was thinking of prepare_for_testing but it looks like
build_executable can call untested.)

One other way centralizing is nice is that if we decide to remove
the noise, we can just remove these calls and be done.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug testsuite/29778] [gdb/testsuite] Revise untested usage

[sam at gentoo dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=29778

Sam James <sam at gentoo dot org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |sam at gentoo dot org

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug testsuite/29778] [gdb/testsuite] Revise untested usage

[tromey at sourceware dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=29778

--- Comment #9 from Tom Tromey <tromey at sourceware dot org> ---
I found a case where the noisier output would be actively helpful.
For me, dw2-ranges.exp started silently doing nothing.
Investigating it:

/home/tromey/gdb/build/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.dwarf2/dw2-ranges-3.c:
warning: STABS debugging information is obsolete and not supported anymore

So, a new compiler warning means we started skipping a test.

If every early return had some annotation that showed up in the .sum,
we could detect this scenario... not saying there's anything to do
about it, but it seems better to flag it an intentionally ignore
it than to have some tests simply silently stop running and never
really find out.

-- 
You are receiving this mail because:
You are on the CC list for the bug.