[00/46] Rewrite "require" test procedure and use it more often

  A recent patch by Enze Li made me revisit the "require" procedure.
For his patch, see:

https://sourceware.org/pipermail/gdb-patches/2022-December/194586.html

This series rewrites "require" to be a bit more 'static' and then
changes a lot of the test suite to use it when possible.  I looked at
a large number of tests, but not every single one, so it's possible
some more work here could be done.

The new 'require' just invokes simple predicates and decides what to
do based on them.  It will issue an "unsupported" message if the
requirements are not satisfied.  See patch #3.

Currently the requirements should only be invoked when gdb is not
running.  At least, that's what I've done, I don't think there's a
real barrier to doing this, other than the fact that the resulting
code might be overly subtle.

More changes here could be done, for example allowing uses like:

    require {is_remote host}

or the like.  I haven't tried this.

The main benefit of this entire approach is that it removes
boilerplate and decisions about whether to use verbose, unsupport,
untested, or even no output at all.

Regression tested on x86-64 Fedora 36, though TBH that's hardly
sufficient.

Let me know what you think.

Tom
  

On 12/17/22 00:07, Tom Tromey wrote:
> A recent patch by Enze Li made me revisit the "require" procedure.
> For his patch, see:
> 
> https://sourceware.org/pipermail/gdb-patches/2022-December/194586.html
> 
> This series rewrites "require" to be a bit more 'static' and then
> changes a lot of the test suite to use it when possible.  I looked at
> a large number of tests, but not every single one, so it's possible
> some more work here could be done.
> 
> The new 'require' just invokes simple predicates and decides what to
> do based on them.  It will issue an "unsupported" message if the
> requirements are not satisfied.  See patch #3.
> 
> Currently the requirements should only be invoked when gdb is not
> running.  At least, that's what I've done, I don't think there's a
> real barrier to doing this, other than the fact that the resulting
> code might be overly subtle.

That has happened in the past. Of course once you realize that's how it should work, you don't do it anymore.

I wonder if there is a way to assert if the requirement is being used with a running gdb.

> 
> More changes here could be done, for example allowing uses like:
> 
>      require {is_remote host}
> 
> or the like.  I haven't tried this.
> 
> The main benefit of this entire approach is that it removes
> boilerplate and decisions about whether to use verbose, unsupport,
> untested, or even no output at all.
> 
> Regression tested on x86-64 Fedora 36, though TBH that's hardly
> sufficient.
> 
> Let me know what you think.
> 
> Tom
> 
>
  

On Fri, 16 Dec 2022 17:07:32 -0700
Tom Tromey <tom@tromey.com> wrote:

> Let me know what you think.

Overall, I like this series.

One thing that I find a little odd (due to a sort of double negation) is the
construct:

    require !skip_something

...which replaces something like:

    if {[skip_something]} {
        unsupported "target does not support something"
	return 0
    }

I see that Markus Metzger also commented on this and also that you
noted that this could be improved over time.  (Which is fine with me.)

I looked at each patch in the series, though I didn't read all of them
closely.  After a while, I just did some spot checks to make sure that
they were doing what I expected.

So... LGTM.

Kevin
  

>> Currently the requirements should only be invoked when gdb is not
>> running.  At least, that's what I've done, I don't think there's a
>> real barrier to doing this, other than the fact that the resulting
>> code might be overly subtle.

Luis> I wonder if there is a way to assert if the requirement is being
Luis> used with a running gdb.

We could make it work.

Also I think now that gdb has --config, we can make skip_python_tests
without needing a running gdb.  I don't know if there are others that
could be improved this way.

Tom
  

Kevin> One thing that I find a little odd (due to a sort of double negation) is the
Kevin> construct:
Kevin>     require !skip_something
...
Kevin> I see that Markus Metzger also commented on this and also that you
Kevin> noted that this could be improved over time.  (Which is fine with me.)

It would be pretty easy to just convert them all.

What non-negative name would be good?
"test_"?  "allow_"?  "enable_"?

Kevin> So... LGTM.

Thanks.  I will put it in reasonably soon, though if we pick a good name
I will do that first and send a v2.

Tom
  

On Thu, 05 Jan 2023 19:47:14 -0700
Tom Tromey <tom@tromey.com> wrote:

> Kevin> One thing that I find a little odd (due to a sort of double negation) is the
> Kevin> construct:
> Kevin>     require !skip_something  
> ...
> Kevin> I see that Markus Metzger also commented on this and also that you
> Kevin> noted that this could be improved over time.  (Which is fine with me.)  
> 
> It would be pretty easy to just convert them all.
> 
> What non-negative name would be good?
> "test_"?  "allow_"?  "enable_"?

Of the choices you mention, I like "allow_" the best.

Kevin