public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] Fix gdb.fortran/array-element.exp failures.
@ 2014-07-04  5:58 Gabriel Krisman Bertazi
  2014-07-04 15:35 ` Sergio Durigan Junior
  0 siblings, 1 reply; 18+ messages in thread
From: Gabriel Krisman Bertazi @ 2014-07-04  5:58 UTC (permalink / raw)
  To: gdb-patches; +Cc: Gabriel Krisman Bertazi

This fixes two FAIL results when running this test file due to a
misplaced "continue" command, which caused the inferior to end execution
prematurely.

Before, we had:

FAIL: gdb.fortran/array-element.exp: continue to breakpoint once again
(the program exited)
FAIL: gdb.fortran/array-element.exp: print the second element of array a

                === gdb Summary ===

                # of expected passes            3
                # of unexpected failures        2

This gives us:

                === gdb Summary ===

                # of expected passes            4

2014-07-04  Gabriel Krisman Bertazi  <gabriel@krisman.be>

	* gdb.fortran/array-element.exp: Remove wrong "continue" command
	that caused test to fail.

---
 gdb/testsuite/gdb.fortran/array-element.exp | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/gdb/testsuite/gdb.fortran/array-element.exp b/gdb/testsuite/gdb.fortran/array-element.exp
index 579db03..3c87790 100644
--- a/gdb/testsuite/gdb.fortran/array-element.exp
+++ b/gdb/testsuite/gdb.fortran/array-element.exp
@@ -41,8 +41,5 @@ gdb_test "continue" \
     "continue to breakpoint"
 gdb_test "print a(1)" ".*1 = 1.*" "print the first element of array a"
 
-gdb_test "continue" \
-    "Continuing\\..*Breakpoint.*" \
-    "continue to breakpoint once again"
 gdb_test "print a(2)" ".*2 = 2.*" "print the second element of array a"
 
-- 
2.0.1

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] Fix gdb.fortran/array-element.exp failures.
  2014-07-04  5:58 [PATCH] Fix gdb.fortran/array-element.exp failures Gabriel Krisman Bertazi
@ 2014-07-04 15:35 ` Sergio Durigan Junior
  2014-07-04 23:20   ` Gabriel Krisman Bertazi
  0 siblings, 1 reply; 18+ messages in thread
From: Sergio Durigan Junior @ 2014-07-04 15:35 UTC (permalink / raw)
  To: Gabriel Krisman Bertazi; +Cc: gdb-patches

On Friday, July 04 2014, Gabriel Krisman Bertazi wrote:

> This fixes two FAIL results when running this test file due to a
> misplaced "continue" command, which caused the inferior to end execution
> prematurely.

Thanks, this is great :-).

> 2014-07-04  Gabriel Krisman Bertazi  <gabriel@krisman.be>
>
> 	* gdb.fortran/array-element.exp: Remove wrong "continue" command
> 	that caused test to fail.
>
> ---
>  gdb/testsuite/gdb.fortran/array-element.exp | 3 ---
>  1 file changed, 3 deletions(-)
>
> diff --git a/gdb/testsuite/gdb.fortran/array-element.exp b/gdb/testsuite/gdb.fortran/array-element.exp
> index 579db03..3c87790 100644
> --- a/gdb/testsuite/gdb.fortran/array-element.exp
> +++ b/gdb/testsuite/gdb.fortran/array-element.exp
> @@ -41,8 +41,5 @@ gdb_test "continue" \
>      "continue to breakpoint"
>  gdb_test "print a(1)" ".*1 = 1.*" "print the first element of array a"
>  
> -gdb_test "continue" \
> -    "Continuing\\..*Breakpoint.*" \
> -    "continue to breakpoint once again"
>  gdb_test "print a(2)" ".*2 = 2.*" "print the second element of array a"

I am not a Fortran expert, but after looking at the testcase it seems
this is the right fix indeed.

BTW, I recommend that this testcase be simplified.  Instead of:

  set bp_location [gdb_get_line_number "continue"]
  gdb_test "break $bp_location" \
      "Breakpoint.*at.* file .*$srcfile, line $bp_location\\." \
      "breakpoint at continue"

  gdb_test "continue" \
      "Continuing\\..*Breakpoint.*" \
      "continue to breakpoint"
  gdb_test "print a(1)" ".*1 = 1.*" "print the first element of array a"

  gdb_test "print a(2)" ".*2 = 2.*" "print the second element of array a"

It could be written as:

  gdb_breakpoint [gdb_get_line_number "continue"]
  gdb_continue_to_breakpoint "continue"
  gdb_test "print a(1)" ".*1 = 1.*" "print the first element of array a"
  gdb_test "print a(2)" ".*2 = 2.*" "print the second element of array a"

Would you like to do that?

Otherwise, it looks good to me (this is not an approval).

Thanks,

-- 
Sergio
GPG key ID: 0x65FC5E36
Please send encrypted e-mail if possible
http://sergiodj.net/

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] Fix gdb.fortran/array-element.exp failures.
  2014-07-04 15:35 ` Sergio Durigan Junior
@ 2014-07-04 23:20   ` Gabriel Krisman Bertazi
  2014-07-05 12:14     ` Sergio Durigan Junior
  0 siblings, 1 reply; 18+ messages in thread
From: Gabriel Krisman Bertazi @ 2014-07-04 23:20 UTC (permalink / raw)
  To: Sergio Durigan Junior; +Cc: gdb-patches

[-- Attachment #1: Type: text/plain, Size: 271 bytes --]

Sergio Durigan Junior <sergiodj@redhat.com> writes:

> Thanks, this is great :-).

Thanks for your review.

> BTW, I recommend that this testcase be simplified.  Instead of:

I applied your suggestions.  Below is a new version of the patch that
simplifies the testcase.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: fortran_test.patch --]
[-- Type: text/x-diff, Size: 974 bytes --]

diff --git a/gdb/testsuite/gdb.fortran/array-element.exp b/gdb/testsuite/gdb.fortran/array-element.exp
index 579db03..1ac3623 100644
--- a/gdb/testsuite/gdb.fortran/array-element.exp
+++ b/gdb/testsuite/gdb.fortran/array-element.exp
@@ -31,18 +31,9 @@ if ![runto sub_] then {
     continue
 }
 
-set bp_location [gdb_get_line_number "continue"]
-gdb_test "break $bp_location" \
-    "Breakpoint.*at.* file .*$srcfile, line $bp_location\\." \
-    "breakpoint at continue"
-
-gdb_test "continue" \
-    "Continuing\\..*Breakpoint.*" \
-    "continue to breakpoint"
-gdb_test "print a(1)" ".*1 = 1.*" "print the first element of array a"
+gdb_breakpoint [gdb_get_line_number "continue"]
+gdb_continue_to_breakpoint "continue"
 
-gdb_test "continue" \
-    "Continuing\\..*Breakpoint.*" \
-    "continue to breakpoint once again"
+gdb_test "print a(1)" ".*1 = 1.*" "print the first element of array a"
 gdb_test "print a(2)" ".*2 = 2.*" "print the second element of array a"
 

[-- Attachment #3: Type: text/plain, Size: 29 bytes --]


-- 
Gabriel Krisman Bertazi

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] Fix gdb.fortran/array-element.exp failures.
  2014-07-04 23:20   ` Gabriel Krisman Bertazi
@ 2014-07-05 12:14     ` Sergio Durigan Junior
  2014-07-05 17:17       ` Gabriel Krisman Bertazi
  0 siblings, 1 reply; 18+ messages in thread
From: Sergio Durigan Junior @ 2014-07-05 12:14 UTC (permalink / raw)
  To: Gabriel Krisman Bertazi; +Cc: gdb-patches

On Friday, July 04 2014, Gabriel Krisman Bertazi wrote:

>> BTW, I recommend that this testcase be simplified.  Instead of:
>
> I applied your suggestions.  Below is a new version of the patch that
> simplifies the testcase.

Thanks!  It is missing a (new) ChangeLog entry; other than that, I
believe it is fine to be applied.

Now it's just a matter of waiting for a maintainer's review/approval :-).

> diff --git a/gdb/testsuite/gdb.fortran/array-element.exp b/gdb/testsuite/gdb.fortran/array-element.exp
> index 579db03..1ac3623 100644
> --- a/gdb/testsuite/gdb.fortran/array-element.exp
> +++ b/gdb/testsuite/gdb.fortran/array-element.exp
> @@ -31,18 +31,9 @@ if ![runto sub_] then {
>      continue
>  }
>  
> -set bp_location [gdb_get_line_number "continue"]
> -gdb_test "break $bp_location" \
> -    "Breakpoint.*at.* file .*$srcfile, line $bp_location\\." \
> -    "breakpoint at continue"
> -
> -gdb_test "continue" \
> -    "Continuing\\..*Breakpoint.*" \
> -    "continue to breakpoint"
> -gdb_test "print a(1)" ".*1 = 1.*" "print the first element of array a"
> +gdb_breakpoint [gdb_get_line_number "continue"]
> +gdb_continue_to_breakpoint "continue"
>  
> -gdb_test "continue" \
> -    "Continuing\\..*Breakpoint.*" \
> -    "continue to breakpoint once again"
> +gdb_test "print a(1)" ".*1 = 1.*" "print the first element of array a"
>  gdb_test "print a(2)" ".*2 = 2.*" "print the second element of array a"

-- 
Sergio
GPG key ID: 0x65FC5E36
Please send encrypted e-mail if possible
http://sergiodj.net/

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] Fix gdb.fortran/array-element.exp failures.
  2014-07-05 12:14     ` Sergio Durigan Junior
@ 2014-07-05 17:17       ` Gabriel Krisman Bertazi
  2014-07-06 19:03         ` Sergio Durigan Junior
  0 siblings, 1 reply; 18+ messages in thread
From: Gabriel Krisman Bertazi @ 2014-07-05 17:17 UTC (permalink / raw)
  To: Sergio Durigan Junior; +Cc: gdb-patches

[-- Attachment #1: Type: text/plain, Size: 398 bytes --]

Sergio Durigan Junior <sergiodj@redhat.com> writes:

> Thanks!  It is missing a (new) ChangeLog entry; other than that, I
> believe it is fine to be applied.
>

Sorry, I missed it. Here follows the patch and the updated changelog.

2014-07-05  Gabriel Krisman Bertazi  <gabriel@krisman.be>

	* array-element.exp: Remove wrong "continue" command that caused
	test to fail and simplifies test case.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: fortran_test.patch --]
[-- Type: text/x-diff, Size: 974 bytes --]

diff --git a/gdb/testsuite/gdb.fortran/array-element.exp b/gdb/testsuite/gdb.fortran/array-element.exp
index 579db03..1ac3623 100644
--- a/gdb/testsuite/gdb.fortran/array-element.exp
+++ b/gdb/testsuite/gdb.fortran/array-element.exp
@@ -31,18 +31,9 @@ if ![runto sub_] then {
     continue
 }
 
-set bp_location [gdb_get_line_number "continue"]
-gdb_test "break $bp_location" \
-    "Breakpoint.*at.* file .*$srcfile, line $bp_location\\." \
-    "breakpoint at continue"
-
-gdb_test "continue" \
-    "Continuing\\..*Breakpoint.*" \
-    "continue to breakpoint"
-gdb_test "print a(1)" ".*1 = 1.*" "print the first element of array a"
+gdb_breakpoint [gdb_get_line_number "continue"]
+gdb_continue_to_breakpoint "continue"
 
-gdb_test "continue" \
-    "Continuing\\..*Breakpoint.*" \
-    "continue to breakpoint once again"
+gdb_test "print a(1)" ".*1 = 1.*" "print the first element of array a"
 gdb_test "print a(2)" ".*2 = 2.*" "print the second element of array a"
 

[-- Attachment #3: Type: text/plain, Size: 29 bytes --]


-- 
Gabriel Krisman Bertazi

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] Fix gdb.fortran/array-element.exp failures.
  2014-07-05 17:17       ` Gabriel Krisman Bertazi
@ 2014-07-06 19:03         ` Sergio Durigan Junior
  2014-07-14 23:28           ` Gabriel Krisman Bertazi
  0 siblings, 1 reply; 18+ messages in thread
From: Sergio Durigan Junior @ 2014-07-06 19:03 UTC (permalink / raw)
  To: Gabriel Krisman Bertazi; +Cc: gdb-patches

On Saturday, July 05 2014, Gabriel Krisman Bertazi wrote:

> Sorry, I missed it. Here follows the patch and the updated changelog.
>
> 2014-07-05  Gabriel Krisman Bertazi  <gabriel@krisman.be>
>
> 	* array-element.exp: Remove wrong "continue" command that caused
> 	test to fail and simplifies test case.

It should be:

	* gdb.fortran/array-element.exp

Also, and sorry for the nitpicking, but we write ChangeLog entries in
the imperative mode, so I'd write it as:

	* gdb.fortran/array-element.exp: Remove wrong "continue"
	  command.  Simplify test case.

Anyway, just minor comments, no need to resubmit the patch.  Let's wait
for some maintainer to take a look.

Thanks,

-- 
Sergio
GPG key ID: 0x65FC5E36
Please send encrypted e-mail if possible
http://sergiodj.net/

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] Fix gdb.fortran/array-element.exp failures.
  2014-07-06 19:03         ` Sergio Durigan Junior
@ 2014-07-14 23:28           ` Gabriel Krisman Bertazi
  2014-08-17  4:07             ` Gabriel Krisman Bertazi
  0 siblings, 1 reply; 18+ messages in thread
From: Gabriel Krisman Bertazi @ 2014-07-14 23:28 UTC (permalink / raw)
  To: gdb-patches

[-- Attachment #1: Type: text/plain, Size: 175 bytes --]


> Anyway, just minor comments, no need to resubmit the patch.  Let's wait
> for some maintainer to take a look.
>

Ping for this patch.

-- 
Gabriel Krisman Bertazi

[-- Attachment #2: Type: application/pgp-signature, Size: 818 bytes --]

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] Fix gdb.fortran/array-element.exp failures.
  2014-07-14 23:28           ` Gabriel Krisman Bertazi
@ 2014-08-17  4:07             ` Gabriel Krisman Bertazi
  2014-08-26  0:30               ` [PING] " Gabriel Krisman Bertazi
  0 siblings, 1 reply; 18+ messages in thread
From: Gabriel Krisman Bertazi @ 2014-08-17  4:07 UTC (permalink / raw)
  To: gdb-patches

[-- Attachment #1: Type: text/plain, Size: 438 bytes --]

Gabriel Krisman Bertazi <gabriel@krisman.be> writes:

>> Anyway, just minor comments, no need to resubmit the patch.  Let's wait
>> for some maintainer to take a look.

Sorry, I went on vacation and missed a few weekly pings.  Resending ping
and updated patch.  Is this ok for commit?

2014-08-17  Gabriel Krisman Bertazi  <gabriel@krisman.be>

	* gdb.fortran/array-element.exp: Remove wrong "continue"
	  command.  Simplify test case.



[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: fortran.patch --]
[-- Type: text/x-patch, Size: 974 bytes --]

diff --git a/gdb/testsuite/gdb.fortran/array-element.exp b/gdb/testsuite/gdb.fortran/array-element.exp
index 579db03..1ac3623 100644
--- a/gdb/testsuite/gdb.fortran/array-element.exp
+++ b/gdb/testsuite/gdb.fortran/array-element.exp
@@ -31,18 +31,9 @@ if ![runto sub_] then {
     continue
 }
 
-set bp_location [gdb_get_line_number "continue"]
-gdb_test "break $bp_location" \
-    "Breakpoint.*at.* file .*$srcfile, line $bp_location\\." \
-    "breakpoint at continue"
-
-gdb_test "continue" \
-    "Continuing\\..*Breakpoint.*" \
-    "continue to breakpoint"
-gdb_test "print a(1)" ".*1 = 1.*" "print the first element of array a"
+gdb_breakpoint [gdb_get_line_number "continue"]
+gdb_continue_to_breakpoint "continue"
 
-gdb_test "continue" \
-    "Continuing\\..*Breakpoint.*" \
-    "continue to breakpoint once again"
+gdb_test "print a(1)" ".*1 = 1.*" "print the first element of array a"
 gdb_test "print a(2)" ".*2 = 2.*" "print the second element of array a"
 

[-- Attachment #3: Type: text/plain, Size: 29 bytes --]


-- 
Gabriel Krisman Bertazi

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PING] [PATCH] Fix gdb.fortran/array-element.exp failures.
  2014-08-17  4:07             ` Gabriel Krisman Bertazi
@ 2014-08-26  0:30               ` Gabriel Krisman Bertazi
  2014-09-04 19:31                 ` Gabriel Krisman Bertazi
  2014-09-09 13:09                 ` Joel Brobecker
  0 siblings, 2 replies; 18+ messages in thread
From: Gabriel Krisman Bertazi @ 2014-08-26  0:30 UTC (permalink / raw)
  To: gdb-patches

[-- Attachment #1: Type: text/plain, Size: 161 bytes --]

Ping.

 2014-08-17  Gabriel Krisman Bertazi  <gabriel@krisman.be>

 	* gdb.fortran/array-element.exp: Remove wrong "continue"
 	  command.  Simplify test case.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: fortran.patch --]
[-- Type: text/x-patch, Size: 974 bytes --]

diff --git a/gdb/testsuite/gdb.fortran/array-element.exp b/gdb/testsuite/gdb.fortran/array-element.exp
index 579db03..1ac3623 100644
--- a/gdb/testsuite/gdb.fortran/array-element.exp
+++ b/gdb/testsuite/gdb.fortran/array-element.exp
@@ -31,18 +31,9 @@ if ![runto sub_] then {
     continue
 }
 
-set bp_location [gdb_get_line_number "continue"]
-gdb_test "break $bp_location" \
-    "Breakpoint.*at.* file .*$srcfile, line $bp_location\\." \
-    "breakpoint at continue"
-
-gdb_test "continue" \
-    "Continuing\\..*Breakpoint.*" \
-    "continue to breakpoint"
-gdb_test "print a(1)" ".*1 = 1.*" "print the first element of array a"
+gdb_breakpoint [gdb_get_line_number "continue"]
+gdb_continue_to_breakpoint "continue"
 
-gdb_test "continue" \
-    "Continuing\\..*Breakpoint.*" \
-    "continue to breakpoint once again"
+gdb_test "print a(1)" ".*1 = 1.*" "print the first element of array a"
 gdb_test "print a(2)" ".*2 = 2.*" "print the second element of array a"
 

[-- Attachment #3: Type: text/plain, Size: 29 bytes --]


-- 
Gabriel Krisman Bertazi

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PING] [PATCH] Fix gdb.fortran/array-element.exp failures.
  2014-08-26  0:30               ` [PING] " Gabriel Krisman Bertazi
@ 2014-09-04 19:31                 ` Gabriel Krisman Bertazi
  2014-09-09 13:09                 ` Joel Brobecker
  1 sibling, 0 replies; 18+ messages in thread
From: Gabriel Krisman Bertazi @ 2014-09-04 19:31 UTC (permalink / raw)
  To: gdb-patches

[-- Attachment #1: Type: text/plain, Size: 1283 bytes --]


Ping.  Is this ok for approval?

>  2014-08-17  Gabriel Krisman Bertazi  <gabriel@krisman.be>
>
>  	* gdb.fortran/array-element.exp: Remove wrong "continue"
>  	  command.  Simplify test case.
>
> diff --git a/gdb/testsuite/gdb.fortran/array-element.exp b/gdb/testsuite/gdb.fortran/array-element.exp
> index 579db03..1ac3623 100644
> --- a/gdb/testsuite/gdb.fortran/array-element.exp
> +++ b/gdb/testsuite/gdb.fortran/array-element.exp
> @@ -31,18 +31,9 @@ if ![runto sub_] then {
>      continue
>  }
>  
> -set bp_location [gdb_get_line_number "continue"]
> -gdb_test "break $bp_location" \
> -    "Breakpoint.*at.* file .*$srcfile, line $bp_location\\." \
> -    "breakpoint at continue"
> -
> -gdb_test "continue" \
> -    "Continuing\\..*Breakpoint.*" \
> -    "continue to breakpoint"
> -gdb_test "print a(1)" ".*1 = 1.*" "print the first element of array a"
> +gdb_breakpoint [gdb_get_line_number "continue"]
> +gdb_continue_to_breakpoint "continue"
>  
> -gdb_test "continue" \
> -    "Continuing\\..*Breakpoint.*" \
> -    "continue to breakpoint once again"
> +gdb_test "print a(1)" ".*1 = 1.*" "print the first element of array a"
>  gdb_test "print a(2)" ".*2 = 2.*" "print the second element of array a"

-- 
Gabriel Krisman Bertazi

[-- Attachment #2: Type: application/pgp-signature, Size: 818 bytes --]

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PING] [PATCH] Fix gdb.fortran/array-element.exp failures.
  2014-08-26  0:30               ` [PING] " Gabriel Krisman Bertazi
  2014-09-04 19:31                 ` Gabriel Krisman Bertazi
@ 2014-09-09 13:09                 ` Joel Brobecker
  2014-09-09 14:55                   ` Gabriel Krisman Bertazi
  2014-09-09 15:08                   ` Sergio Durigan Junior
  1 sibling, 2 replies; 18+ messages in thread
From: Joel Brobecker @ 2014-09-09 13:09 UTC (permalink / raw)
  To: Gabriel Krisman Bertazi; +Cc: gdb-patches

Gabriel,

>  2014-08-17  Gabriel Krisman Bertazi  <gabriel@krisman.be>
> 
>  	* gdb.fortran/array-element.exp: Remove wrong "continue"
>  	  command.  Simplify test case.
> 

Sorry it's taking so much time to review your patch.

To help us out, it's useful to show what the patch does in practice.
In this case, I'd like to do what's not working before the patch
gets applied, and how things change once your patch is applied.
GDB transcripts before and after usually help with that.

-- 
Joel

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PING] [PATCH] Fix gdb.fortran/array-element.exp failures.
  2014-09-09 13:09                 ` Joel Brobecker
@ 2014-09-09 14:55                   ` Gabriel Krisman Bertazi
  2014-09-09 15:45                     ` Joel Brobecker
  2014-09-09 15:08                   ` Sergio Durigan Junior
  1 sibling, 1 reply; 18+ messages in thread
From: Gabriel Krisman Bertazi @ 2014-09-09 14:55 UTC (permalink / raw)
  To: Joel Brobecker; +Cc: gdb-patches

[-- Attachment #1: Type: text/plain, Size: 1087 bytes --]

Joel,

> To help us out, it's useful to show what the patch does in practice.
> In this case, I'd like to do what's not working before the patch
> gets applied, and how things change once your patch is applied.
> GDB transcripts before and after usually help with that.

Sorry I didn't resend the original message with my pings.  Here it goes.

This fixes two FAIL results when running this test file due to a
misplaced "continue" command, which caused the inferior to end execution
prematurely.

On trunk, we have:

FAIL: gdb.fortran/array-element.exp: continue to breakpoint once again
(the program exited)
FAIL: gdb.fortran/array-element.exp: print the second element of array a

                === gdb Summary ===

                # of expected passes            3
                # of unexpected failures        2

And now, we get:
                === gdb Summary ===
                # of expected passes            3

Is this ok?

2014-07-04  Gabriel Krisman Bertazi  <gabriel@krisman.be>

	* gdb.fortran/array-element.exp: Remove wrong "continue"
	command.  Simplify test case.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: fortran_patch.patch --]
[-- Type: text/x-patch, Size: 974 bytes --]

diff --git a/gdb/testsuite/gdb.fortran/array-element.exp b/gdb/testsuite/gdb.fortran/array-element.exp
index 579db03..1ac3623 100644
--- a/gdb/testsuite/gdb.fortran/array-element.exp
+++ b/gdb/testsuite/gdb.fortran/array-element.exp
@@ -31,18 +31,9 @@ if ![runto sub_] then {
     continue
 }
 
-set bp_location [gdb_get_line_number "continue"]
-gdb_test "break $bp_location" \
-    "Breakpoint.*at.* file .*$srcfile, line $bp_location\\." \
-    "breakpoint at continue"
-
-gdb_test "continue" \
-    "Continuing\\..*Breakpoint.*" \
-    "continue to breakpoint"
-gdb_test "print a(1)" ".*1 = 1.*" "print the first element of array a"
+gdb_breakpoint [gdb_get_line_number "continue"]
+gdb_continue_to_breakpoint "continue"
 
-gdb_test "continue" \
-    "Continuing\\..*Breakpoint.*" \
-    "continue to breakpoint once again"
+gdb_test "print a(1)" ".*1 = 1.*" "print the first element of array a"
 gdb_test "print a(2)" ".*2 = 2.*" "print the second element of array a"
 

[-- Attachment #3: Type: text/plain, Size: 29 bytes --]


-- 
Gabriel Krisman Bertazi

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PING] [PATCH] Fix gdb.fortran/array-element.exp failures.
  2014-09-09 13:09                 ` Joel Brobecker
  2014-09-09 14:55                   ` Gabriel Krisman Bertazi
@ 2014-09-09 15:08                   ` Sergio Durigan Junior
  1 sibling, 0 replies; 18+ messages in thread
From: Sergio Durigan Junior @ 2014-09-09 15:08 UTC (permalink / raw)
  To: Joel Brobecker; +Cc: Gabriel Krisman Bertazi, gdb-patches

On Tuesday, September 09 2014, Joel Brobecker wrote:

> To help us out, it's useful to show what the patch does in practice.
> In this case, I'd like to do what's not working before the patch
> gets applied, and how things change once your patch is applied.
> GDB transcripts before and after usually help with that.

BTW, and I am not invalidating your request, I applied and tested his
patch locally before my review, which showed me that the issue was
probably a thinko from the original author.

Nevertheless, it's always good to have more data :-).

-- 
Sergio
GPG key ID: 0x65FC5E36
Please send encrypted e-mail if possible
http://sergiodj.net/

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PING] [PATCH] Fix gdb.fortran/array-element.exp failures.
  2014-09-09 14:55                   ` Gabriel Krisman Bertazi
@ 2014-09-09 15:45                     ` Joel Brobecker
  2014-09-09 18:17                       ` Gabriel Krisman Bertazi
  0 siblings, 1 reply; 18+ messages in thread
From: Joel Brobecker @ 2014-09-09 15:45 UTC (permalink / raw)
  To: Gabriel Krisman Bertazi; +Cc: gdb-patches

> Sorry I didn't resend the original message with my pings.  Here it goes.
> 
> This fixes two FAIL results when running this test file due to a
> misplaced "continue" command, which caused the inferior to end execution
> prematurely.
> 
> On trunk, we have:
> 
> FAIL: gdb.fortran/array-element.exp: continue to breakpoint once again
> (the program exited)
> FAIL: gdb.fortran/array-element.exp: print the second element of array a
> 
>                 === gdb Summary ===
> 
>                 # of expected passes            3
>                 # of unexpected failures        2
> 
> And now, we get:
>                 === gdb Summary ===
>                 # of expected passes            3
> 
> Is this ok?

The problem is that this does not tell me what was wrong. Just that
some tests did not pass. Although your change seems OK at first glance,
I'd like to do know what it was that did not match, and why it's OK
stop trying to match it now.

-- 
Joel

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PING] [PATCH] Fix gdb.fortran/array-element.exp failures.
  2014-09-09 15:45                     ` Joel Brobecker
@ 2014-09-09 18:17                       ` Gabriel Krisman Bertazi
  2014-09-10 12:50                         ` Joel Brobecker
  0 siblings, 1 reply; 18+ messages in thread
From: Gabriel Krisman Bertazi @ 2014-09-09 18:17 UTC (permalink / raw)
  To: Joel Brobecker; +Cc: gdb-patches

Joel Brobecker <brobecker@adacore.com> writes:

> The problem is that this does not tell me what was wrong. Just that
> some tests did not pass. Although your change seems OK at first glance,
> I'd like to do know what it was that did not match, and why it's OK
> stop trying to match it now.

Joel, thanks for your clarification.

Ok, so we got this situation:

The original testcase sets a breakpoint at the label continue and
resumes execution until we reach it.  On the Fortran file, this means
the inferior has iterated over the whole loop before reaching the
breakpoint for the first time.  Then, the original testcase issues
another continue command, causing the inferior to finish the execution
earlier than expected, since we still want to make a final test on
whether we print the second element.  This causes the two test failures.

My guess is that the original author meant to break after each loop
iteration, instead of going all the way until the continue label.

Nevertheless, stepping over a single iteration or stopping after the
entire loop has no impact on the test results.  So, what my patch does
is simply remove the second "continue" command that would prematurely
end inferior's execution, so we can actually test whether both elements
are printed correctly after executing the loop.

Other than that, when I first submitted this patch, Sergio asked me to
simplify the testcase, because it felt unusual.  That is what the other
modifications are about.

I'll make sure to update the commit message to include part of this
explanation to better clarify what this patch is really about.

Thanks,

-- 
Gabriel Krisman Bertazi

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PING] [PATCH] Fix gdb.fortran/array-element.exp failures.
  2014-09-09 18:17                       ` Gabriel Krisman Bertazi
@ 2014-09-10 12:50                         ` Joel Brobecker
  2014-09-10 16:11                           ` Sergio Durigan Junior
  2014-09-11  3:25                           ` Gabriel Krisman Bertazi
  0 siblings, 2 replies; 18+ messages in thread
From: Joel Brobecker @ 2014-09-10 12:50 UTC (permalink / raw)
  To: Gabriel Krisman Bertazi; +Cc: gdb-patches

> 
> Joel, thanks for your clarification.
> 
> Ok, so we got this situation:
> 
> The original testcase sets a breakpoint at the label continue and
> resumes execution until we reach it.  On the Fortran file, this means
> the inferior has iterated over the whole loop before reaching the
> breakpoint for the first time.  Then, the original testcase issues
> another continue command, causing the inferior to finish the execution
> earlier than expected, since we still want to make a final test on
> whether we print the second element.  This causes the two test failures.
> 
> My guess is that the original author meant to break after each loop
> iteration, instead of going all the way until the continue label.
> 
> Nevertheless, stepping over a single iteration or stopping after the
> entire loop has no impact on the test results.  So, what my patch does
> is simply remove the second "continue" command that would prematurely
> end inferior's execution, so we can actually test whether both elements
> are printed correctly after executing the loop.
> 
> Other than that, when I first submitted this patch, Sergio asked me to
> simplify the testcase, because it felt unusual.  That is what the other
> modifications are about.

OK, thank you for the explanation of the issue.

>  2014-08-17  Gabriel Krisman Bertazi  <gabriel@krisman.be>
>
>       * gdb.fortran/array-element.exp: Remove wrong "continue"
>         command.  Simplify test case.

OK to push.

My only comment is that simplications are indeed good, but it is
better if you can submit those separately from other changes.
It's easier to review the patch series that way, and it also
allows us separate the real change from the enhancement which
is expected to be a no-op.

Thanks :)
-- 
Joel

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PING] [PATCH] Fix gdb.fortran/array-element.exp failures.
  2014-09-10 12:50                         ` Joel Brobecker
@ 2014-09-10 16:11                           ` Sergio Durigan Junior
  2014-09-11  3:25                           ` Gabriel Krisman Bertazi
  1 sibling, 0 replies; 18+ messages in thread
From: Sergio Durigan Junior @ 2014-09-10 16:11 UTC (permalink / raw)
  To: Joel Brobecker; +Cc: Gabriel Krisman Bertazi, gdb-patches

On Wednesday, September 10 2014, Joel Brobecker wrote:

> My only comment is that simplications are indeed good, but it is
> better if you can submit those separately from other changes.
> It's easier to review the patch series that way, and it also
> allows us separate the real change from the enhancement which
> is expected to be a no-op.

Mea culpa.

I thought it would be nice to simplify the testcase in one shot, because
it seemed to me that the error happened *also* because the testcase was
not very clear.  And since I had applied the patch, investigated and
figured out that it did the right thing, I thought it'd be good to
simplify it too.

Sorry if it made the review harder!

-- 
Sergio
GPG key ID: 0x65FC5E36
Please send encrypted e-mail if possible
http://sergiodj.net/

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PING] [PATCH] Fix gdb.fortran/array-element.exp failures.
  2014-09-10 12:50                         ` Joel Brobecker
  2014-09-10 16:11                           ` Sergio Durigan Junior
@ 2014-09-11  3:25                           ` Gabriel Krisman Bertazi
  1 sibling, 0 replies; 18+ messages in thread
From: Gabriel Krisman Bertazi @ 2014-09-11  3:25 UTC (permalink / raw)
  To: Joel Brobecker; +Cc: gdb-patches

[-- Attachment #1: Type: text/plain, Size: 190 bytes --]

Joel Brobecker <brobecker@adacore.com> writes:

> OK to push.

Just pushed this: <https://sourceware.org/ml/gdb-cvs/2014-09/msg00046.html>.

Thanks :)

-- 
Gabriel Krisman Bertazi

[-- Attachment #2: Type: application/pgp-signature, Size: 818 bytes --]

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2014-09-11  3:25 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-07-04  5:58 [PATCH] Fix gdb.fortran/array-element.exp failures Gabriel Krisman Bertazi
2014-07-04 15:35 ` Sergio Durigan Junior
2014-07-04 23:20   ` Gabriel Krisman Bertazi
2014-07-05 12:14     ` Sergio Durigan Junior
2014-07-05 17:17       ` Gabriel Krisman Bertazi
2014-07-06 19:03         ` Sergio Durigan Junior
2014-07-14 23:28           ` Gabriel Krisman Bertazi
2014-08-17  4:07             ` Gabriel Krisman Bertazi
2014-08-26  0:30               ` [PING] " Gabriel Krisman Bertazi
2014-09-04 19:31                 ` Gabriel Krisman Bertazi
2014-09-09 13:09                 ` Joel Brobecker
2014-09-09 14:55                   ` Gabriel Krisman Bertazi
2014-09-09 15:45                     ` Joel Brobecker
2014-09-09 18:17                       ` Gabriel Krisman Bertazi
2014-09-10 12:50                         ` Joel Brobecker
2014-09-10 16:11                           ` Sergio Durigan Junior
2014-09-11  3:25                           ` Gabriel Krisman Bertazi
2014-09-09 15:08                   ` Sergio Durigan Junior

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).