* [PATCH] gdb/testsuite: fix gdb.guile/scm-parameter.exp "wrong type argument" test pattern @ 2022-10-24 16:43 Simon Marchi 2022-10-24 23:22 ` Maciej W. Rozycki 0 siblings, 1 reply; 7+ messages in thread From: Simon Marchi @ 2022-10-24 16:43 UTC (permalink / raw) To: gdb-patches; +Cc: Maciej W . Rozycki, Simon Marchi Since commit 90319cefe3 ("GDB/Guile: Don't assert that an integer value is boolean"), I see: FAIL: gdb.guile/scm-parameter.exp: kind=PARAM_ZINTEGER: test-PARAM_ZINTEGER-param: guile (set-parameter-value! test-PARAM_ZINTEGER-param #:unlimited) FAIL: gdb.guile/scm-parameter.exp: kind=PARAM_ZUINTEGER: test-PARAM_ZUINTEGER-param: guile (set-parameter-value! test-PARAM_ZUINTEGER-param #:unlimited) This comes from the fact that GDB outputs this: ERROR: In procedure set-parameter-value!: In procedure gdbscm_set_parameter_value_x: Wrong type argument in position 2 (expecting integer): #:unlimited Error while executing Scheme code. while the test expects an additional "ERROR:" on the second line, something like this: ERROR: In procedure set-parameter-value!: ERROR: In procedure gdbscm_set_parameter_value_x: Wrong type argument in position 2 (expecting integer): #:unlimited Error while executing Scheme code. The patch below fixes the test for me. I believe that the first two lines are output by Guile itself, in the SCM_ASSERT_TYPE macro. I tried on different systems, different Guile versions (2.0, 2.2 and 3.0) and I always get the former output, never the output the test expects. I presume the patch below isn't right, as there is surely some systems that do print the latter output, otherwise Maciej (the original author) would have noticed it. I presume we'll need to accept both outputs. But I'd like we if could clarify when we get which. Change-Id: I9dc45e7492a4f08340cad974610242ed689de959 --- gdb/testsuite/gdb.guile/scm-parameter.exp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/gdb/testsuite/gdb.guile/scm-parameter.exp b/gdb/testsuite/gdb.guile/scm-parameter.exp index b9f2d825211..5d72da935bc 100644 --- a/gdb/testsuite/gdb.guile/scm-parameter.exp +++ b/gdb/testsuite/gdb.guile/scm-parameter.exp @@ -183,10 +183,10 @@ foreach_with_prefix kind { "end" set param_integer_error \ - "ERROR: In procedure set-parameter-value!:\r\nERROR: In procedure\ - gdbscm_set_parameter_value_x: Wrong type argument in position 2\ - \\(expecting integer\\): #:unlimited\r\nError while executing Scheme\ - code\\." + [multi_line \ + "ERROR: In procedure set-parameter-value!:" \ + "In procedure gdbscm_set_parameter_value_x: Wrong type argument in position 2 \\(expecting integer\\): #:unlimited" \ + "Error while executing Scheme code\\."] set param_minus_one_error "integer -1 out of range" set param_minus_two_range "integer -2 out of range" set param_minus_two_unlimited "only -1 is allowed to set as unlimited" -- 2.37.3 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] gdb/testsuite: fix gdb.guile/scm-parameter.exp "wrong type argument" test pattern 2022-10-24 16:43 [PATCH] gdb/testsuite: fix gdb.guile/scm-parameter.exp "wrong type argument" test pattern Simon Marchi @ 2022-10-24 23:22 ` Maciej W. Rozycki 2022-10-25 1:08 ` Simon Marchi 0 siblings, 1 reply; 7+ messages in thread From: Maciej W. Rozycki @ 2022-10-24 23:22 UTC (permalink / raw) To: Simon Marchi; +Cc: gdb-patches On Mon, 24 Oct 2022, Simon Marchi wrote: > I believe that the first two lines are output by Guile itself, in the > SCM_ASSERT_TYPE macro. I tried on different systems, different Guile > versions (2.0, 2.2 and 3.0) and I always get the former output, never > the output the test expects. I presume the patch below isn't right, as > there is surely some systems that do print the latter output, otherwise > Maciej (the original author) would have noticed it. I presume we'll > need to accept both outputs. But I'd like we if could clarify when we > get which. FTR I'm still looking into it and like you I have hesitated to just paper the issue over by allowing both outputs without first understanding what is really going on here. I cannot rule out a distribution-specific patch causing a discrepancy here, but I feel like tracking it down. NB guile 2.0.13 here, reporting as: guile (GNU Guile) 2.0.13 Packaged by Debian (2.0.13-deb+1-5.4) (and it seems like I have said version consistently throughout my relevant development machines except for different "-deb+..." suffixes). It must have had long distribution maintenance history. Maciej ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] gdb/testsuite: fix gdb.guile/scm-parameter.exp "wrong type argument" test pattern 2022-10-24 23:22 ` Maciej W. Rozycki @ 2022-10-25 1:08 ` Simon Marchi 2022-10-26 7:15 ` Tom de Vries 0 siblings, 1 reply; 7+ messages in thread From: Simon Marchi @ 2022-10-25 1:08 UTC (permalink / raw) To: Maciej W. Rozycki, Simon Marchi; +Cc: gdb-patches On 2022-10-24 19:22, Maciej W. Rozycki wrote: > On Mon, 24 Oct 2022, Simon Marchi wrote: > >> I believe that the first two lines are output by Guile itself, in the >> SCM_ASSERT_TYPE macro. I tried on different systems, different Guile >> versions (2.0, 2.2 and 3.0) and I always get the former output, never >> the output the test expects. I presume the patch below isn't right, as >> there is surely some systems that do print the latter output, otherwise >> Maciej (the original author) would have noticed it. I presume we'll >> need to accept both outputs. But I'd like we if could clarify when we >> get which. > > FTR I'm still looking into it and like you I have hesitated to just paper > the issue over by allowing both outputs without first understanding what > is really going on here. I cannot rule out a distribution-specific patch > causing a discrepancy here, but I feel like tracking it down. > > NB guile 2.0.13 here, reporting as: > > guile (GNU Guile) 2.0.13 > Packaged by Debian (2.0.13-deb+1-5.4) According to that version number, it looks like Ubuntu 20.04? https://packages.ubuntu.com/focal/guile-2.0 I tried building on Ubuntu 20.04 against guile-2.0, and I see the same result as you. And when I try guile2.0 on Arch Linux (this package [1]), I also see the same result as you. So I must have tested it wrong previously. You can dig further if you want, but I'd be fine just accepting both outputs and saying that guile-2.0 outputs the additional ERROR: while subsequent versions do not. Simon [1] https://aur.archlinux.org/packages/guile2.0 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] gdb/testsuite: fix gdb.guile/scm-parameter.exp "wrong type argument" test pattern 2022-10-25 1:08 ` Simon Marchi @ 2022-10-26 7:15 ` Tom de Vries 2022-10-26 15:31 ` Simon Marchi 0 siblings, 1 reply; 7+ messages in thread From: Tom de Vries @ 2022-10-26 7:15 UTC (permalink / raw) To: Simon Marchi, Maciej W. Rozycki, Simon Marchi; +Cc: gdb-patches On 10/25/22 03:08, Simon Marchi via Gdb-patches wrote: > > > On 2022-10-24 19:22, Maciej W. Rozycki wrote: >> On Mon, 24 Oct 2022, Simon Marchi wrote: >> >>> I believe that the first two lines are output by Guile itself, in the >>> SCM_ASSERT_TYPE macro. I tried on different systems, different Guile >>> versions (2.0, 2.2 and 3.0) and I always get the former output, never >>> the output the test expects. I presume the patch below isn't right, as >>> there is surely some systems that do print the latter output, otherwise >>> Maciej (the original author) would have noticed it. I presume we'll >>> need to accept both outputs. But I'd like we if could clarify when we >>> get which. >> >> FTR I'm still looking into it and like you I have hesitated to just paper >> the issue over by allowing both outputs without first understanding what >> is really going on here. I cannot rule out a distribution-specific patch >> causing a discrepancy here, but I feel like tracking it down. >> >> NB guile 2.0.13 here, reporting as: >> >> guile (GNU Guile) 2.0.13 >> Packaged by Debian (2.0.13-deb+1-5.4) > > According to that version number, it looks like Ubuntu 20.04? > > https://packages.ubuntu.com/focal/guile-2.0 > > I tried building on Ubuntu 20.04 against guile-2.0, and I see the same > result as you. And when I try guile2.0 on Arch Linux (this package > [1]), I also see the same result as you. So I must have tested it wrong > previously. > > You can dig further if you want, but I'd be fine just accepting both > outputs and saying that guile-2.0 outputs the additional ERROR: while > subsequent versions do not. > FWIW, I did the same here in commit 6bbe1a929c6 ("[gdb/testsuite] Fix gdb.guile/scm-breakpoint.exp with guile 3.0"). Thanks, - Tom ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] gdb/testsuite: fix gdb.guile/scm-parameter.exp "wrong type argument" test pattern 2022-10-26 7:15 ` Tom de Vries @ 2022-10-26 15:31 ` Simon Marchi 2022-10-29 13:56 ` [OB PATCH] gdb/testsuite: Wrap `param_integer_error' in gdb.guile/scm-parameter.exp Maciej W. Rozycki 0 siblings, 1 reply; 7+ messages in thread From: Simon Marchi @ 2022-10-26 15:31 UTC (permalink / raw) To: Tom de Vries, Maciej W. Rozycki, Simon Marchi; +Cc: gdb-patches On 10/26/22 03:15, Tom de Vries wrote: > On 10/25/22 03:08, Simon Marchi via Gdb-patches wrote: >> >> >> On 2022-10-24 19:22, Maciej W. Rozycki wrote: >>> On Mon, 24 Oct 2022, Simon Marchi wrote: >>> >>>> I believe that the first two lines are output by Guile itself, in the >>>> SCM_ASSERT_TYPE macro. I tried on different systems, different Guile >>>> versions (2.0, 2.2 and 3.0) and I always get the former output, never >>>> the output the test expects. I presume the patch below isn't right, as >>>> there is surely some systems that do print the latter output, otherwise >>>> Maciej (the original author) would have noticed it. I presume we'll >>>> need to accept both outputs. But I'd like we if could clarify when we >>>> get which. >>> >>> FTR I'm still looking into it and like you I have hesitated to just paper >>> the issue over by allowing both outputs without first understanding what >>> is really going on here. I cannot rule out a distribution-specific patch >>> causing a discrepancy here, but I feel like tracking it down. >>> >>> NB guile 2.0.13 here, reporting as: >>> >>> guile (GNU Guile) 2.0.13 >>> Packaged by Debian (2.0.13-deb+1-5.4) >> >> According to that version number, it looks like Ubuntu 20.04? >> >> https://packages.ubuntu.com/focal/guile-2.0 >> >> I tried building on Ubuntu 20.04 against guile-2.0, and I see the same >> result as you. And when I try guile2.0 on Arch Linux (this package >> [1]), I also see the same result as you. So I must have tested it wrong >> previously. >> >> You can dig further if you want, but I'd be fine just accepting both >> outputs and saying that guile-2.0 outputs the additional ERROR: while >> subsequent versions do not. >> > > FWIW, I did the same here in commit 6bbe1a929c6 ("[gdb/testsuite] Fix gdb.guile/scm-breakpoint.exp with guile 3.0"). Thanks, then I just went ahead and pushed this: From ee7f721ea2f51cd6cda301ce6a68e84f61c31e0c Mon Sep 17 00:00:00 2001 From: Simon Marchi <simon.marchi@efficios.com> Date: Mon, 24 Oct 2022 12:43:38 -0400 Subject: [PATCH] gdb/testsuite: fix gdb.guile/scm-parameter.exp "wrong type argument" test pattern for Guile >= 2.2 Since commit 90319cefe3 ("GDB/Guile: Don't assert that an integer value is boolean"), I see: FAIL: gdb.guile/scm-parameter.exp: kind=PARAM_ZINTEGER: test-PARAM_ZINTEGER-param: guile (set-parameter-value! test-PARAM_ZINTEGER-param #:unlimited) FAIL: gdb.guile/scm-parameter.exp: kind=PARAM_ZUINTEGER: test-PARAM_ZUINTEGER-param: guile (set-parameter-value! test-PARAM_ZUINTEGER-param #:unlimited) This comes from the fact that GDB outputs this: ERROR: In procedure set-parameter-value!: In procedure gdbscm_set_parameter_value_x: Wrong type argument in position 2 (expecting integer): #:unlimited Error while executing Scheme code. while the test expects an additional "ERROR:" on the second line, something like this: ERROR: In procedure set-parameter-value!: ERROR: In procedure gdbscm_set_parameter_value_x: Wrong type argument in position 2 (expecting integer): #:unlimited Error while executing Scheme code. Guile 2.0 outputs the `ERROR:` on the second line, while later versions do not. Change the pattern to accept both outputs. This is similar to commit 6bbe1a929c6 ("[gdb/testsuite] Fix gdb.guile/scm-breakpoint.exp with guile 3.0"). Change-Id: I9dc45e7492a4f08340cad974610242ed689de959 --- gdb/testsuite/gdb.guile/scm-parameter.exp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/gdb/testsuite/gdb.guile/scm-parameter.exp b/gdb/testsuite/gdb.guile/scm-parameter.exp index b9f2d8252117..0b2076c40576 100644 --- a/gdb/testsuite/gdb.guile/scm-parameter.exp +++ b/gdb/testsuite/gdb.guile/scm-parameter.exp @@ -183,10 +183,10 @@ foreach_with_prefix kind { "end" set param_integer_error \ - "ERROR: In procedure set-parameter-value!:\r\nERROR: In procedure\ - gdbscm_set_parameter_value_x: Wrong type argument in position 2\ - \\(expecting integer\\): #:unlimited\r\nError while executing Scheme\ - code\\." + [multi_line \ + "ERROR: In procedure set-parameter-value!:" \ + "(ERROR: )?In procedure gdbscm_set_parameter_value_x: Wrong type argument in position 2 \\(expecting integer\\): #:unlimited" \ + "Error while executing Scheme code\\."] set param_minus_one_error "integer -1 out of range" set param_minus_two_range "integer -2 out of range" set param_minus_two_unlimited "only -1 is allowed to set as unlimited" -- 2.38.1 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [OB PATCH] gdb/testsuite: Wrap `param_integer_error' in gdb.guile/scm-parameter.exp 2022-10-26 15:31 ` Simon Marchi @ 2022-10-29 13:56 ` Maciej W. Rozycki 2022-10-31 12:57 ` Simon Marchi 0 siblings, 1 reply; 7+ messages in thread From: Maciej W. Rozycki @ 2022-10-29 13:56 UTC (permalink / raw) To: Simon Marchi; +Cc: Tom de Vries, Simon Marchi, gdb-patches Wrap an overlong line in the definition of `param_integer_error' in gdb.guile/scm-parameter.exp. No functional change. --- On Wed, 26 Oct 2022, Simon Marchi wrote: > >>> FTR I'm still looking into it and like you I have hesitated to just paper > >>> the issue over by allowing both outputs without first understanding what > >>> is really going on here. I cannot rule out a distribution-specific patch > >>> causing a discrepancy here, but I feel like tracking it down. > >>> > >>> NB guile 2.0.13 here, reporting as: > >>> > >>> guile (GNU Guile) 2.0.13 > >>> Packaged by Debian (2.0.13-deb+1-5.4) > >> > >> According to that version number, it looks like Ubuntu 20.04? > >> > >> https://packages.ubuntu.com/focal/guile-2.0 > >> > >> I tried building on Ubuntu 20.04 against guile-2.0, and I see the same > >> result as you. And when I try guile2.0 on Arch Linux (this package > >> [1]), I also see the same result as you. So I must have tested it wrong > >> previously. > >> > >> You can dig further if you want, but I'd be fine just accepting both > >> outputs and saying that guile-2.0 outputs the additional ERROR: while > >> subsequent versions do not. > >> > > > > FWIW, I did the same here in commit 6bbe1a929c6 ("[gdb/testsuite] Fix gdb.guile/scm-breakpoint.exp with guile 3.0"). > > Thanks, then I just went ahead and pushed this: Thanks, though why such a rush to fix a benign test failure while the review took months in the first place? FWIW I have been struggling to get multiple versions of Guile compiled and installed locally (easier) and then GDB built against each of them (tougher). As it turns out our documentation is misleading. We have: `--with-guile[=GUILE]' Build GDB with GNU Guile scripting support. (Done by default if libguile is present and found at configure time.) If your host does not have Guile installed, you can find it at `https://www.gnu.org/software/guile/'. The optional argument GUILE can be a version number, which will cause `configure' to try to use that version of Guile; or the file name of a `pkg-config' executable, which will be queried to find the information needed to compile and link against Guile. (and similarly in `./configure --help'), so one could have thought `--with-guile=2.0' will work. Well, not. You need to specify both the name and the version, e.g.: `--with-guile=guile-2.0', which I find kind of awkward: why would one want to have a Guile package under a name that is not "guile"? I think documentation is sensible and it's implementation that ought to be fixed. But that is not enough. Still I got: checking whether to use guile... guile-2.0 checking for pkg-config... /usr/bin/pkg-config checking for usable guile from /usr/bin/pkg-config... checking for scm_set_automatic_finalization_enabled... no configure: error: in `.../gdb': configure: error: linking guile version guile-2.0 test program failed See `config.log' for more details make[1]: *** [Makefile:12496: configure-gdb] Error 1 This is because I have built local Guile as static libraries only and dependencies are not pulled with the (incorrect) `pkg-config' invocation we have in our configury. I got these issues sorted now and will be posting fixes soon. With these in place I have figured out that the message changed between Guile 2.0 and 2.2. Last but not least here's a fix I committed as obvious to correct an overlong line you have introduced with your change. While we do have an exemption for TCL code, there's no need to make use of it here and these long lines hit clarity of code badly. I have verified this test case to still pass after my change. Maciej --- gdb/testsuite/gdb.guile/scm-parameter.exp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) gdb-test-scm-parameter-error-wrap.diff Index: src/gdb/testsuite/gdb.guile/scm-parameter.exp =================================================================== --- src.orig/gdb/testsuite/gdb.guile/scm-parameter.exp +++ src/gdb/testsuite/gdb.guile/scm-parameter.exp @@ -185,7 +185,9 @@ foreach_with_prefix kind { set param_integer_error \ [multi_line \ "ERROR: In procedure set-parameter-value!:" \ - "(ERROR: )?In procedure gdbscm_set_parameter_value_x: Wrong type argument in position 2 \\(expecting integer\\): #:unlimited" \ + "(ERROR: )?In procedure gdbscm_set_parameter_value_x:\ + Wrong type argument in position 2 \\(expecting integer\\):\ + #:unlimited" \ "Error while executing Scheme code\\."] set param_minus_one_error "integer -1 out of range" set param_minus_two_range "integer -2 out of range" ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [OB PATCH] gdb/testsuite: Wrap `param_integer_error' in gdb.guile/scm-parameter.exp 2022-10-29 13:56 ` [OB PATCH] gdb/testsuite: Wrap `param_integer_error' in gdb.guile/scm-parameter.exp Maciej W. Rozycki @ 2022-10-31 12:57 ` Simon Marchi 0 siblings, 0 replies; 7+ messages in thread From: Simon Marchi @ 2022-10-31 12:57 UTC (permalink / raw) To: Maciej W. Rozycki; +Cc: Tom de Vries, Simon Marchi, gdb-patches On 10/29/22 09:56, Maciej W. Rozycki wrote: > Wrap an overlong line in the definition of `param_integer_error' in > gdb.guile/scm-parameter.exp. No functional change. > --- > On Wed, 26 Oct 2022, Simon Marchi wrote: > >>>>> FTR I'm still looking into it and like you I have hesitated to just paper >>>>> the issue over by allowing both outputs without first understanding what >>>>> is really going on here. I cannot rule out a distribution-specific patch >>>>> causing a discrepancy here, but I feel like tracking it down. >>>>> >>>>> NB guile 2.0.13 here, reporting as: >>>>> >>>>> guile (GNU Guile) 2.0.13 >>>>> Packaged by Debian (2.0.13-deb+1-5.4) >>>> >>>> According to that version number, it looks like Ubuntu 20.04? >>>> >>>> https://packages.ubuntu.com/focal/guile-2.0 >>>> >>>> I tried building on Ubuntu 20.04 against guile-2.0, and I see the same >>>> result as you. And when I try guile2.0 on Arch Linux (this package >>>> [1]), I also see the same result as you. So I must have tested it wrong >>>> previously. >>>> >>>> You can dig further if you want, but I'd be fine just accepting both >>>> outputs and saying that guile-2.0 outputs the additional ERROR: while >>>> subsequent versions do not. >>>> >>> >>> FWIW, I did the same here in commit 6bbe1a929c6 ("[gdb/testsuite] Fix gdb.guile/scm-breakpoint.exp with guile 3.0"). >> >> Thanks, then I just went ahead and pushed this: > > Thanks, though why such a rush to fix a benign test failure while the > review took months in the first place? The fix seemed relatively obvious, given we already had one instance of this. > FWIW I have been struggling to get multiple versions of Guile compiled > and installed locally (easier) and then GDB built against each of them > (tougher). As it turns out our documentation is misleading. We have: > > `--with-guile[=GUILE]' > Build GDB with GNU Guile scripting support. (Done by default if > libguile is present and found at configure time.) If your host > does not have Guile installed, you can find it at > `https://www.gnu.org/software/guile/'. The optional argument > GUILE can be a version number, which will cause `configure' to > try to use that version of Guile; or the file name of a > `pkg-config' executable, which will be queried to find the > information needed to compile and link against Guile. > > (and similarly in `./configure --help'), so one could have thought > `--with-guile=2.0' will work. Well, not. You need to specify both the > name and the version, e.g.: `--with-guile=guile-2.0', which I find kind of > awkward: why would one want to have a Guile package under a name that is > not "guile"? I think documentation is sensible and it's implementation > that ought to be fixed. Ack. > > But that is not enough. Still I got: > > checking whether to use guile... guile-2.0 > checking for pkg-config... /usr/bin/pkg-config > checking for usable guile from /usr/bin/pkg-config... checking for scm_set_automatic_finalization_enabled... no > configure: error: in `.../gdb': > configure: error: linking guile version guile-2.0 test program failed > See `config.log' for more details > make[1]: *** [Makefile:12496: configure-gdb] Error 1 > > This is because I have built local Guile as static libraries only and > dependencies are not pulled with the (incorrect) `pkg-config' invocation > we have in our configury. > > I got these issues sorted now and will be posting fixes soon. With these > in place I have figured out that the message changed between Guile 2.0 and > 2.2. Thanks. > Last but not least here's a fix I committed as obvious to correct an > overlong line you have introduced with your change. While we do have an > exemption for TCL code, there's no need to make use of it here and these > long lines hit clarity of code badly. I have verified this test case to > still pass after my change. This is subjective. For expected output patterns, I prefer to format it as one expected line per source line. But how you formatted it is fine with me as well. Simon ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2022-10-31 12:57 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-10-24 16:43 [PATCH] gdb/testsuite: fix gdb.guile/scm-parameter.exp "wrong type argument" test pattern Simon Marchi 2022-10-24 23:22 ` Maciej W. Rozycki 2022-10-25 1:08 ` Simon Marchi 2022-10-26 7:15 ` Tom de Vries 2022-10-26 15:31 ` Simon Marchi 2022-10-29 13:56 ` [OB PATCH] gdb/testsuite: Wrap `param_integer_error' in gdb.guile/scm-parameter.exp Maciej W. Rozycki 2022-10-31 12:57 ` Simon Marchi
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).