GDB version as convenience variable

  On 2019-03-25 3:18 p.m., Eli Zaretskii wrote:
>> Date: Mon, 25 Mar 2019 14:51:36 -0400
>> From: Simon Marchi <simark@simark.ca>
>> Cc: gdb-patches@sourceware.org, brobecker@adacore.com
>>
>> comparing the output with gdb --version, or some other automated
>> mean, would be perfect.
> 
> Well, I never wrote any tests for GDB, and I'm not sure I have a
> system on which to run such a test.  So I hope someone else will add
> this test.
> 

Apparently, the test default.exp needs to be changed in any case, because
it adds new convenience variables.  Without modification, the
"show convenience" test fails because there is a new entry it doesn't know
about.

So for now, I would suggest adding the modification below to your patch to keep
the test passing.  It is not obvious to change the test to match the convenience
variable value against a variable or even against a regexp, because it uses
gdb_test_list_exact, which does straight string comparison.

Also, I am not sure if reading the output of "show version" and testing against
that would be a really good test, because the implementation and the test would
work essentially work the same way.  So if the version string erroneously becomes
0.0.0 because of some bug in some script, the test would still pass, even though
the values are wrong.

At least, the following is simple and robust.




	
Simon
  

> Apparently, the test default.exp needs to be changed in any case,
> because it adds new convenience variables.  Without modification, the
> "show convenience" test fails because there is a new entry it doesn't
> know about.
> 
> So for now, I would suggest adding the modification below to your
> patch to keep the test passing.  It is not obvious to change the test
> to match the convenience variable value against a variable or even
> against a regexp, because it uses gdb_test_list_exact, which does
> straight string comparison.
> 
> Also, I am not sure if reading the output of "show version" and
> testing against that would be a really good test, because the
> implementation and the test would work essentially work the same way.
> So if the version string erroneously becomes 0.0.0 because of some bug
> in some script, the test would still pass, even though the values are
> wrong.
> 
> At least, the following is simple and robust.

Hmmm, the direct string comparison thing is a bit annoying.

This is going to be one more manual test to do after each branch,
and I'm a bit concerned about that. We can start with that, as we want
the test to pass. But I'm wondering if, instead of getting the output
from "show version" to determine the expected values, we could parse
gdb/version.in instead. If needed, we could so the parsing as part of
the build process (ig do it in the makefile, and store the result in
a couple of files). Something like that.

> diff --git a/gdb/testsuite/gdb.base/default.exp b/gdb/testsuite/gdb.base/default.exp
> index ece1428e617e..9ff5144448d8 100644
> --- a/gdb/testsuite/gdb.base/default.exp
> +++ b/gdb/testsuite/gdb.base/default.exp
> @@ -602,6 +602,8 @@ set show_conv_list \
>         {$_probe_arg10 = <error: No frame selected>} \
>         {$_probe_arg11 = <error: No frame selected>} \
>         {$_isvoid = <internal function _isvoid>} \
> +       {$_gdb_major = 8} \
> +       {$_gdb_minor = 4} \
>      }
>  if ![skip_python_tests] {
>      append show_conv_list \
> 
> 
> 	
> Simon
  

> Date: Tue, 26 Mar 2019 13:57:16 -0700
> From: Joel Brobecker <brobecker@adacore.com>
> Cc: Eli Zaretskii <eliz@gnu.org>, gdb-patches@sourceware.org
> 
> This is going to be one more manual test to do after each branch,
> and I'm a bit concerned about that. We can start with that, as we want
> the test to pass. But I'm wondering if, instead of getting the output
> from "show version" to determine the expected values, we could parse
> gdb/version.in instead. If needed, we could so the parsing as part of
> the build process (ig do it in the makefile, and store the result in
> a couple of files). Something like that.

Yes, if version.in is updated by some automated method, the same
method could update the test suite.
  

> > This is going to be one more manual test to do after each branch,
> > and I'm a bit concerned about that. We can start with that, as we want
> > the test to pass. But I'm wondering if, instead of getting the output
> > from "show version" to determine the expected values, we could parse
> > gdb/version.in instead. If needed, we could so the parsing as part of
> > the build process (ig do it in the makefile, and store the result in
> > a couple of files). Something like that.
> 
> Yes, if version.in is updated by some automated method, the same
> method could update the test suite.

We're actually thinking of getting rid of that daily bump, and doing
this would make it more difficult. There has to be a way to parse
version.in during the build and make that information available to
the testsuite, somehow. If not, I'd rather adjust the testing to
allow regexps, and just verify that it prints a positive number for
the major version, and a non-zero number for the minor version.
  

> From: Simon Marchi <simark@simark.ca>
> Cc: gdb-patches@sourceware.org, brobecker@adacore.com
> Date: Tue, 26 Mar 2019 10:46:56 -0400
> 
> Apparently, the test default.exp needs to be changed in any case, because
> it adds new convenience variables.  Without modification, the
> "show convenience" test fails because there is a new entry it doesn't know
> about.
> 
> So for now, I would suggest adding the modification below to your patch to keep
> the test passing.  It is not obvious to change the test to match the convenience
> variable value against a variable or even against a regexp, because it uses
> gdb_test_list_exact, which does straight string comparison.
> 
> Also, I am not sure if reading the output of "show version" and testing against
> that would be a really good test, because the implementation and the test would
> work essentially work the same way.  So if the version string erroneously becomes
> 0.0.0 because of some bug in some script, the test would still pass, even though
> the values are wrong.
> 
> At least, the following is simple and robust.

Thanks, I used that, and pushed the changes including this one.
  

On 2019-03-27 8:56 a.m., Joel Brobecker wrote:
>>> This is going to be one more manual test to do after each branch,
>>> and I'm a bit concerned about that. We can start with that, as we want
>>> the test to pass. But I'm wondering if, instead of getting the output
>>> from "show version" to determine the expected values, we could parse
>>> gdb/version.in instead. If needed, we could so the parsing as part of
>>> the build process (ig do it in the makefile, and store the result in
>>> a couple of files). Something like that.
>>
>> Yes, if version.in is updated by some automated method, the same
>> method could update the test suite.
> 
> We're actually thinking of getting rid of that daily bump, and doing
> this would make it more difficult. There has to be a way to parse
> version.in during the build and make that information available to
> the testsuite, somehow. If not, I'd rather adjust the testing to
> allow regexps, and just verify that it prints a positive number for
> the major version, and a non-zero number for the minor version.
> 

I wrote a patch to make the test read the version number from version.in
directly, see here:

https://sourceware.org/ml/gdb-patches/2019-03/msg00762.html

Simon