Bug 199493 – [webkitpy] test-webkitpy is broken on Linux since r246662

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Carlos Alberto Lopez Perez]

Since r246662 test-webkitpy fails on Linux with:

$ Tools/Scripts/test-webkitpy
Traceback (most recent call last):
  File "/home/clopez/webkit/webkit/Tools/Scripts/webkitpy/test/main.py", line 374, in <module>
    sys.exit(main())
  File "/home/clopez/webkit/webkit/Tools/Scripts/webkitpy/test/main.py", line 72, in main
    if not _supports_building_and_running_lldb_tests():
  File "/home/clopez/webkit/webkit/Tools/Scripts/webkitpy/test/main.py", line 107, in _supports_building_and_running_lldb_tests
    return not _host.platform.build_version().startswith('19A')
AttributeError: 'NoneType' object has no attribute 'startswith'

[Carlos Alberto Lopez Perez]

Created attachment 373459 [details]
Patch

[Carlos Alberto Lopez Perez]

Comment on attachment 373459 [details]
Patch

Clearing flags on attachment: 373459

Committed r247139: <https://trac.webkit.org/changeset/247139>

[Carlos Alberto Lopez Perez]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/52644428>

[Fujii Hironori]

*** Bug 199428 has been marked as a duplicate of this bug. ***

[Jonathan Bedard]

Comment on attachment 373459 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=373459&action=review

> Tools/Scripts/webkitpy/test/main.py:109
> +        return False

Shouldn't this be true?

I don't have a Linux build handy at the moment, but I feel like Linux builds should be running lldb tests, no?

[Michael Catanzaro]

Comment on attachment 373459 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=373459&action=review

>> Tools/Scripts/webkitpy/test/main.py:109
>> +        return False
> 
> Shouldn't this be true?
> 
> I don't have a Linux build handy at the moment, but I feel like Linux builds should be running lldb tests, no?

Does lldb even work on Linux?

Even if it does, lldb is not expected to be installed so the tests shouldn't run automatically.

[Adrian Perez]

(In reply to Michael Catanzaro from comment #7)
> Comment on attachment 373459 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=373459&action=review
> 
> >> Tools/Scripts/webkitpy/test/main.py:109
> >> +        return False
> > 
> > Shouldn't this be true?
> > 
> > I don't have a Linux build handy at the moment, but I feel like Linux builds should be running lldb tests, no?
> 
> Does lldb even work on Linux?

Well, lldb has worked on Linux for ages, sometimes I use it
when GDB is having a bad release that has weird C++ demangling
bugs :)

> Even if it does, lldb is not expected to be installed so the tests shouldn't
> run automatically.

I think it's okay that they run if lldb is installed. At any
rate, they should be skipped if it is not available, indeed.

[Carlos Alberto Lopez Perez]

(In reply to Jonathan Bedard from comment #6)
> Comment on attachment 373459 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=373459&action=review
> 
> > Tools/Scripts/webkitpy/test/main.py:109
> > +        return False
> 
> Shouldn't this be true?
> 
> I don't have a Linux build handy at the moment, but I feel like Linux builds
> should be running lldb tests, no?

If lldb tests can run in Linux then a proper check for that can be added, and when the requirements are meet (lldb installed, etc) then return true

But this patch just returns False early to avoid calling startswith() over a None value. That is correct IMHO.

In any case I just tried quickly changing that to True and trying to run the lldb tests and I get:

$ Tools/Scripts/build-lldbwebkittester
lldbWebKitTester is currently only supported on Mac.

So, it seems that adding support for running WebKit lldb tests on Linux should be done in a new bug.

By the way, I'm not familiar with this lldb tests. What is the purpose of them?

[Jonathan Bedard]

(In reply to Carlos Alberto Lopez Perez from comment #9)
> (In reply to Jonathan Bedard from comment #6)
> > Comment on attachment 373459 [details]
> > Patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=373459&action=review
> > 
> > > Tools/Scripts/webkitpy/test/main.py:109
> > > +        return False
> > 
> > Shouldn't this be true?
> > 
> > I don't have a Linux build handy at the moment, but I feel like Linux builds
> > should be running lldb tests, no?
> 
> If lldb tests can run in Linux then a proper check for that can be added,
> and when the requirements are meet (lldb installed, etc) then return true
> 
> But this patch just returns False early to avoid calling startswith() over a
> None value. That is correct IMHO.
> 
> In any case I just tried quickly changing that to True and trying to run the
> lldb tests and I get:
> 
> $ Tools/Scripts/build-lldbwebkittester
> lldbWebKitTester is currently only supported on Mac.
> 
> So, it seems that adding support for running WebKit lldb tests on Linux
> should be done in a new bug.
> 
> By the way, I'm not familiar with this lldb tests. What is the purpose of
> them?

If lldbWebKitTester is currently only supported on Mac, false is correct.

My recollection is that the lldb tests were added by Dan Bates some time ago so that WebKit stopped breaking lldb debugging, but it's been awhile, so I'm not 100% certain of the original motivation.