[patch, testsuite] Fixes for gdb.python tests on remote Windows host

classic Classic list List threaded Threaded
9 messages Options
Reply | Threaded
Open this post in threaded view
|

[patch, testsuite] Fixes for gdb.python tests on remote Windows host

Sandra Loosemore
The attached patch fixes a bunch of FAILs and ERRORs I've seen in
gdb.python tests running on remote Windows host.  As noted in the commit
message, the fixes are mostly obvious and repetitive;  e.g., remember to
copy the .py script to the remote host before trying to source it.
Since this is a pretty big patch, though, I wasn't sure the whole thing
qualifies as "obvious", and wanted to give folks a chance to object
before I check it in.  So I propose to push it to trunk in a week if I
don't hear any objection or review (or promise to review) meanwhile.

-Sandra

python-testsuite.patch (19K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [patch, testsuite] Fixes for gdb.python tests on remote Windows host

Sourceware - gdb-patches mailing list
Do the mingw checks in this patch also need to consider cygwin? E.g.:
This GDB was configured as "x86_64-pc-cygwin".

On Mon, Aug 12, 2019 at 5:20 PM Sandra Loosemore
<[hidden email]> wrote:

>
> The attached patch fixes a bunch of FAILs and ERRORs I've seen in
> gdb.python tests running on remote Windows host.  As noted in the commit
> message, the fixes are mostly obvious and repetitive;  e.g., remember to
> copy the .py script to the remote host before trying to source it.
> Since this is a pretty big patch, though, I wasn't sure the whole thing
> qualifies as "obvious", and wanted to give folks a chance to object
> before I check it in.  So I propose to push it to trunk in a week if I
> don't hear any objection or review (or promise to review) meanwhile.
>
> -Sandra
Reply | Threaded
Open this post in threaded view
|

Re: [patch, testsuite] Fixes for gdb.python tests on remote Windows host

Simon Marchi-4
In reply to this post by Sandra Loosemore
On 2019-08-12 6:20 p.m., Sandra Loosemore wrote:

> The attached patch fixes a bunch of FAILs and ERRORs I've seen in
> gdb.python tests running on remote Windows host.  As noted in the commit
> message, the fixes are mostly obvious and repetitive;  e.g., remember to
> copy the .py script to the remote host before trying to source it.
> Since this is a pretty big patch, though, I wasn't sure the whole thing
> qualifies as "obvious", and wanted to give folks a chance to object
> before I check it in.  So I propose to push it to trunk in a week if I
> don't hear any objection or review (or promise to review) meanwhile.
>
> -Sandra

Hi Sandra,

Windows and remote host testing are two aspects that are really not tested upstream,
so thanks a lot for doing this.  I ran the gdb.python testsuite locally on Linux and
didn't see any change in the results, which is good.

Just two nits on my side:

> diff --git a/gdb/testsuite/gdb.python/py-section-script.exp b/gdb/testsuite/gdb.python/py-section-script.exp
> index c4a6974..0ab1519 100644
> --- a/gdb/testsuite/gdb.python/py-section-script.exp
> +++ b/gdb/testsuite/gdb.python/py-section-script.exp
> @@ -77,22 +77,35 @@ gdb_exit
>  gdb_start
>  gdb_reinitialize_dir $srcdir/$subdir
>
> -gdb_test_no_output "set auto-load safe-path ${remote_python_file}:${binfile}" \
> +# Get the name of the binfile on the host; on a remote host this means
> +# stripping off any directory prefix.
> +if [is_remote host] {
> +  set remote_binfile [file tail ${binfile}]
> +} else {
> +  set remote_binfile ${binfile}
> +}

4 spaces indentation

> +
> +if [ishost *-*-mingw*] {
> +    set remote_pathsep ";"
> +} else {
> +    set remote_pathsep ":"
> +}
> +gdb_test_no_output "set auto-load safe-path ${remote_python_file}${remote_pathsep}${remote_binfile}" \
>      "set auto-load safe-path"
>  gdb_load ${binfile}
>
>  # Verify gdb loaded each script and they appear once in the list.
>  set test_name "verify scripts loaded"
>  gdb_test_multiple "info auto-load python-scripts" "$test_name" {
> +    -re "Yes.*${testfile}.py.*Yes.*inlined-script.*$gdb_prompt $" {
> + pass "$test_name"
> +    }
>      -re "${testfile}.py.*${testfile}.py.*$gdb_prompt $" {
>   fail "$test_name"
>      }
>      -re "inlined-script.*inlined-script.*$gdb_prompt $" {
>   fail "$test_name"
>      }
> -    -re "Yes.*${testfile}.py.*Yes.*inlined-script.*$gdb_prompt $" {
> - pass "$test_name"
> -    }

Is this last change necessary?

Simon
Reply | Threaded
Open this post in threaded view
|

Re: [patch, testsuite] Fixes for gdb.python tests on remote Windows host

Sandra Loosemore
In reply to this post by Sourceware - gdb-patches mailing list
On 8/12/19 7:34 PM, Christian Biesinger wrote:
> Do the mingw checks in this patch also need to consider cygwin? E.g.:
> This GDB was configured as "x86_64-pc-cygwin".

I believe they are completely separate targets.  I don't know much about
cygwin, but I assume that Python built for cygwin library is linked with
the cygwin C library and understands cygwin's fake symbolic links, while
Python built for the mingw C library certainly does not.  Similarly,
using ";" instead of ":" in PATH-like things is a Windows thing, while
I'm pretty sure cygwin emulates the POSIX syntax.

-Sandra
Reply | Threaded
Open this post in threaded view
|

Re: [patch, testsuite] Fixes for gdb.python tests on remote Windows host

Sandra Loosemore
In reply to this post by Simon Marchi-4
On 8/12/19 7:46 PM, Simon Marchi wrote:

> On 2019-08-12 6:20 p.m., Sandra Loosemore wrote:
>> The attached patch fixes a bunch of FAILs and ERRORs I've seen in
>> gdb.python tests running on remote Windows host.  As noted in the commit
>> message, the fixes are mostly obvious and repetitive;  e.g., remember to
>> copy the .py script to the remote host before trying to source it.
>> Since this is a pretty big patch, though, I wasn't sure the whole thing
>> qualifies as "obvious", and wanted to give folks a chance to object
>> before I check it in.  So I propose to push it to trunk in a week if I
>> don't hear any objection or review (or promise to review) meanwhile.
>>
>> -Sandra
>
> Hi Sandra,
>
> Windows and remote host testing are two aspects that are really not tested upstream,
> so thanks a lot for doing this.  I ran the gdb.python testsuite locally on Linux and
> didn't see any change in the results, which is good.
>
> Just two nits on my side:
>
>> diff --git a/gdb/testsuite/gdb.python/py-section-script.exp b/gdb/testsuite/gdb.python/py-section-script.exp
>> index c4a6974..0ab1519 100644
>> --- a/gdb/testsuite/gdb.python/py-section-script.exp
>> +++ b/gdb/testsuite/gdb.python/py-section-script.exp
>> @@ -77,22 +77,35 @@ gdb_exit
>>   gdb_start
>>   gdb_reinitialize_dir $srcdir/$subdir
>>
>> -gdb_test_no_output "set auto-load safe-path ${remote_python_file}:${binfile}" \
>> +# Get the name of the binfile on the host; on a remote host this means
>> +# stripping off any directory prefix.
>> +if [is_remote host] {
>> +  set remote_binfile [file tail ${binfile}]
>> +} else {
>> +  set remote_binfile ${binfile}
>> +}
>
> 4 spaces indentation

Ooops!  I need to do something to make Emacs go into tcl mode
automatically for .exp files.

>
>> +
>> +if [ishost *-*-mingw*] {
>> +    set remote_pathsep ";"
>> +} else {
>> +    set remote_pathsep ":"
>> +}
>> +gdb_test_no_output "set auto-load safe-path ${remote_python_file}${remote_pathsep}${remote_binfile}" \
>>       "set auto-load safe-path"
>>   gdb_load ${binfile}
>>
>>   # Verify gdb loaded each script and they appear once in the list.
>>   set test_name "verify scripts loaded"
>>   gdb_test_multiple "info auto-load python-scripts" "$test_name" {
>> +    -re "Yes.*${testfile}.py.*Yes.*inlined-script.*$gdb_prompt $" {
>> + pass "$test_name"
>> +    }
>>       -re "${testfile}.py.*${testfile}.py.*$gdb_prompt $" {
>>   fail "$test_name"
>>       }
>>       -re "inlined-script.*inlined-script.*$gdb_prompt $" {
>>   fail "$test_name"
>>       }
>> -    -re "Yes.*${testfile}.py.*Yes.*inlined-script.*$gdb_prompt $" {
>> - pass "$test_name"
>> -    }
>
> Is this last change necessary?

Yes.  On Windows host, the output I'm seeing for this test that it's
trying to match is:

(gdb) info auto-load python-scripts
Loaded  Script


Yes     py-section-script.py


         full name: \\long\windows\path\to\py-section-script.py
Yes     gdb.inlined-script


(gdb)

This matches the first "fail" pattern as well as the "pass" pattern, so
the ordering is important.  From the comment on this test, it's clear
this output is intended to be a "pass", so that one should go first.

-Sandra
Reply | Threaded
Open this post in threaded view
|

Re: [patch, testsuite] Fixes for gdb.python tests on remote Windows host

Simon Marchi-4
On 2019-08-12 10:31 p.m., Sandra Loosemore wrote:

> Yes.  On Windows host, the output I'm seeing for this test that it's
> trying to match is:
>
> (gdb) info auto-load python-scripts
> Loaded  Script
>
>
> Yes     py-section-script.py
>
>
>          full name: \\long\windows\path\to\py-section-script.py
> Yes     gdb.inlined-script
>
>
> (gdb)
>
> This matches the first "fail" pattern as well as the "pass" pattern, so
> the ordering is important.  From the comment on this test, it's clear
> this output is intended to be a "pass", so that one should go first.
>
> -Sandra

Ok, that clears it up, thanks.

Simon
Reply | Threaded
Open this post in threaded view
|

Re: [patch, testsuite] Fixes for gdb.python tests on remote Windows host

Simon Marchi-4
In reply to this post by Sandra Loosemore
On 2019-08-12 10:18 p.m., Sandra Loosemore wrote:
> I believe they are completely separate targets.  I don't know much about
> cygwin, but I assume that Python built for cygwin library is linked with
> the cygwin C library and understands cygwin's fake symbolic links, while
> Python built for the mingw C library certainly does not.  Similarly,
> using ";" instead of ":" in PATH-like things is a Windows thing, while
> I'm pretty sure cygwin emulates the POSIX syntax.

Indeed, testing on cygwin would be a whole other task.

I forgot to mention, the patch LGTM, so if Christian is fine with this response too, then please push.

Simon
Reply | Threaded
Open this post in threaded view
|

Re: [patch, testsuite] Fixes for gdb.python tests on remote Windows host

Sourceware - gdb-patches mailing list
On Mon, Aug 12, 2019 at 9:54 PM Simon Marchi <[hidden email]> wrote:

>
> On 2019-08-12 10:18 p.m., Sandra Loosemore wrote:
> > I believe they are completely separate targets.  I don't know much about
> > cygwin, but I assume that Python built for cygwin library is linked with
> > the cygwin C library and understands cygwin's fake symbolic links, while
> > Python built for the mingw C library certainly does not.  Similarly,
> > using ";" instead of ":" in PATH-like things is a Windows thing, while
> > I'm pretty sure cygwin emulates the POSIX syntax.
>
> Indeed, testing on cygwin would be a whole other task.
>
> I forgot to mention, the patch LGTM, so if Christian is fine with this response too, then please push.

Yeah, that sounds good.

Christian
Reply | Threaded
Open this post in threaded view
|

Re: [patch, testsuite] Fixes for gdb.python tests on remote Windows host

Sandra Loosemore
In reply to this post by Simon Marchi-4
On 8/12/19 8:54 PM, Simon Marchi wrote:

> On 2019-08-12 10:18 p.m., Sandra Loosemore wrote:
>> I believe they are completely separate targets.  I don't know much about
>> cygwin, but I assume that Python built for cygwin library is linked with
>> the cygwin C library and understands cygwin's fake symbolic links, while
>> Python built for the mingw C library certainly does not.  Similarly,
>> using ";" instead of ":" in PATH-like things is a Windows thing, while
>> I'm pretty sure cygwin emulates the POSIX syntax.
>
> Indeed, testing on cygwin would be a whole other task.
>
> I forgot to mention, the patch LGTM, so if Christian is fine with this response too, then please push.

Thanks for the speedy review!  I fixed the indentation problem you noted
before pushing the patch.

BTW, these are not the only issues I noticed in running the Python tests
on Windows host.  There's another set involving the "explore" command
and the python interactive help which fail with timeouts because the
expected output is getting buffered.  At this point I'm not sure whether
this is a problem with our test environment (we use cygwin ssh, which
native Windows doesn't recognize as a tty), or some misconfiguration in
the version of Windows python we are using.  I've got a local patch to
temporarily disable those tests to speed up testing, but I don't think
that's a good fix for mainline.  It would be better to figure out how to
force Python to unbuffer the output.

-Sandra