Patchwork [testsuite] Disable dw2-dir-file-name.exp on remote and/or Windows host

login
register
mail settings
Submitter Sandra Loosemore
Date Aug. 13, 2019, 10:29 p.m.
Message ID <9bb4446e-8a94-496d-ab8c-6d1197ac0728@codesourcery.com>
Download mbox | patch
Permalink /patch/34077/
State New
Headers show

Comments

Sandra Loosemore - Aug. 13, 2019, 10:29 p.m.
This is yet another testsuite fix to clean up results on remote Windows 
host.

For this testcase, I did consider trying to fix it rather than just 
disabling it for remote host, but it looked like it was going to be an 
awful lot of work and trial-and-error (it has almost no comments to 
explain what it is trying to test, or how it is getting there).  I think 
it is at least an incremental improvement to document that it isn't 
expected to work as-is on remote host.  And disabling it does get rid of 
33 completely bogus FAILs.  :-P

OK?

-Sandra
Simon Marchi - Aug. 15, 2019, 3:12 a.m.
On 2019-08-13 6:29 p.m., Sandra Loosemore wrote:
> This is yet another testsuite fix to clean up results on remote Windows 
> host.
> 
> For this testcase, I did consider trying to fix it rather than just 
> disabling it for remote host, but it looked like it was going to be an 
> awful lot of work and trial-and-error (it has almost no comments to 
> explain what it is trying to test, or how it is getting there).  I think 
> it is at least an incremental improvement to document that it isn't 
> expected to work as-is on remote host.  And disabling it does get rid of 
> 33 completely bogus FAILs.  :-P
> 
> OK?
> 
> -Sandra
> 

> +# This test has hard-wired assumptions that host and build filenames are
> +# the same, and assumes POSIX pathname syntax.
> +if { [is_remote host] || [ishost *-*-mingw*] } {
> +    return 0
> +}
> +

Should we use "untested" or "unsupported" before returning, to have at least a status
in the logs?  The definition of "unsupported" seems appropriate for this case:

    Declares that a test case depends on some facility that does not exist in the
    testing environment.

From: https://www.gnu.org/software/dejagnu/manual/unsupported-procedure.html

Simon
Sandra Loosemore - Aug. 15, 2019, 2:37 p.m.
On 8/14/19 9:12 PM, Simon Marchi wrote:
> On 2019-08-13 6:29 p.m., Sandra Loosemore wrote:
>> This is yet another testsuite fix to clean up results on remote Windows
>> host.
>>
>> For this testcase, I did consider trying to fix it rather than just
>> disabling it for remote host, but it looked like it was going to be an
>> awful lot of work and trial-and-error (it has almost no comments to
>> explain what it is trying to test, or how it is getting there).  I think
>> it is at least an incremental improvement to document that it isn't
>> expected to work as-is on remote host.  And disabling it does get rid of
>> 33 completely bogus FAILs.  :-P
>>
>> OK?
>>
>> -Sandra
>>
> 
>> +# This test has hard-wired assumptions that host and build filenames are
>> +# the same, and assumes POSIX pathname syntax.
>> +if { [is_remote host] || [ishost *-*-mingw*] } {
>> +    return 0
>> +}
>> +
> 
> Should we use "untested" or "unsupported" before returning, to have at least a status
> in the logs?  The definition of "unsupported" seems appropriate for this case:
> 
>      Declares that a test case depends on some facility that does not exist in the
>      testing environment.
> 
> From: https://www.gnu.org/software/dejagnu/manual/unsupported-procedure.html
> 
> Simon
> 

I did consider that, but the existing previous bit in that file is just

# This test can only be run on targets which support DWARF-2 and use gas.
if {![dwarf2_support]} {
     return 0
}

(which also appears in almost all the other gdb.dwarf2/*.exp files) so I 
thought it would be locally consistent to just return quietly.  Is the 
policy to use "untested" for host issues but not target properties, 
maybe?  I'll do whatever conforms to recommended practice here, of course.

-Sandra
Simon Marchi - Aug. 15, 2019, 2:48 p.m.
On 2019-08-15 10:37 a.m., Sandra Loosemore wrote:
> 
> I did consider that, but the existing previous bit in that file is just
> 
> # This test can only be run on targets which support DWARF-2 and use gas.
> if {![dwarf2_support]} {
>     return 0
> }
> 
> (which also appears in almost all the other gdb.dwarf2/*.exp files) so I thought it would be locally consistent to just return quietly.  Is the policy to use "untested" for host issues but not target properties, maybe?  I'll do whatever conforms to recommended practice here, of course.

Personally, any time a test is skipped for whatever reason, I'd like to see some "UNTESTED"
or "UNSUPPORTED" in the .sum file, to at least know the test existed and was skipped (along
with the reason), rather than just it being omitted.

I don't know if we have an "official" recommended practice here, this is just my opinion.
The patch LGTM in both cases, so I'll let you choose :).

Simon

Patch

commit b1c596ea64186661fc17b9e7b410b95ef4b9cc2e
Author: Sandra Loosemore <sandra@codesourcery.com>
Date:   Tue Aug 13 15:13:13 2019 -0700

    Disable dw2-dir-file-name.exp on remote and/or Windows host.
    
    This test has many hardwired assumptions that pathnames on build and
    host are the same, and that POSIX pathname syntax is used.  This
    results in dozens of failures on a remote Windows host.  Fixing these
    assumptions would involve nontrivial rewrites; meanwhile, let's make
    the test results reflect the reality that this testcase isn't supported
    on remote host.
    
    2019-08-13  Sandra Loosemore  <sandra@codesourcery.com>
    
    	gdb/testsuite/
    	* gdb.dwarf2/dw2-dir-file-name.exp: Skip on remote or
    	Windows host.

diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 38689c1..65fdc6e 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,5 +1,10 @@ 
 2019-08-13  Sandra Loosemore  <sandra@codesourcery.com>
 
+	* gdb.dwarf2/dw2-dir-file-name.exp: Skip on remote or
+	Windows host.
+
+2019-08-13  Sandra Loosemore  <sandra@codesourcery.com>
+
 	* gdb.base/batch-preserve-term-settings.exp
 	(test_terminal_settings_preserved_after_sigterm): Skip on Windows.
 
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-dir-file-name.exp b/gdb/testsuite/gdb.dwarf2/dw2-dir-file-name.exp
index 246b4af..2933df9 100644
--- a/gdb/testsuite/gdb.dwarf2/dw2-dir-file-name.exp
+++ b/gdb/testsuite/gdb.dwarf2/dw2-dir-file-name.exp
@@ -19,6 +19,12 @@  if {![dwarf2_support]} {
     return 0  
 }
 
+# This test has hard-wired assumptions that host and build filenames are
+# the same, and assumes POSIX pathname syntax.
+if { [is_remote host] || [ishost *-*-mingw*] } {
+    return 0
+}
+
 # Find length of addresses in bytes.
 if {[is_64_target]} {
     set addr_len 8