[Lldb-commits] [PATCH] D60300: [testsuite] Split Objective-C data formatter

2019-04-05 Thread Davide Italiano via Phabricator via lldb-commits
davide added a comment. In D60300#1456491 , @davide wrote: > In D60300#1456472 , @clayborg wrote: > > > In D60300#1456468 , @JDevlieghere > > wrote: > > > > > In D60300#1455

[Lldb-commits] [PATCH] D60300: [testsuite] Split Objective-C data formatter

2019-04-05 Thread Davide Italiano via Phabricator via lldb-commits
davide added a comment. In D60300#1456472 , @clayborg wrote: > In D60300#1456468 , @JDevlieghere > wrote: > > > In D60300#1455790 , @davide wrote: > > > > > I think this is

[Lldb-commits] [PATCH] D60300: [testsuite] Split Objective-C data formatter

2019-04-05 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere added a comment. In D60300#1456485 , @aprantl wrote: > > What's the motivation behind all the `@no_debug_info_test` decorators? Are > > those codepaths guaranteed to be tested elsewhere? > > As a rule of thumb I'd say, let's only stick `@no_d

[Lldb-commits] [PATCH] D60300: [testsuite] Split Objective-C data formatter

2019-04-05 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added a comment. > What's the motivation behind all the `@no_debug_info_test` decorators? Are > those codepaths guaranteed to be tested elsewhere? As a rule of thumb I'd say, let's only stick `@no_debug_info_test` on tests where a (future) coverage bot proves that it is safe. How does t

[Lldb-commits] [PATCH] D60300: [testsuite] Split Objective-C data formatter

2019-04-05 Thread Jonas Devlieghere via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL357786: [testsuite] Split Objective-C data formatter (authored by JDevlieghere, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit: https://re

[Lldb-commits] [PATCH] D60300: [testsuite] Split Objective-C data formatter

2019-04-05 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added a comment. What's the motivation behind all the `@no_debug_info_test` decorators? Are those codepaths guaranteed to be tested elsewhere? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60300/new/ https://reviews.llvm.org/D60300 ___

[Lldb-commits] [PATCH] D60300: [testsuite] Split Objective-C data formatter

2019-04-05 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added a comment. In D60300#1456468 , @JDevlieghere wrote: > In D60300#1455790 , @davide wrote: > > > I think this is good regardless for readability, but I would really > > appreciate if we can collect so

[Lldb-commits] [PATCH] D60300: [testsuite] Split Objective-C data formatter

2019-04-05 Thread Davide Italiano via Phabricator via lldb-commits
davide accepted this revision. davide added a comment. This revision is now accepted and ready to land. great! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60300/new/ https://reviews.llvm.org/D60300 ___ lldb-commits mailing list lldb-commi

Re: [Lldb-commits] [PATCH] D60300: [testsuite] Split Objective-C data formatter

2019-04-05 Thread Greg Clayton via lldb-commits
Nice! > On Apr 5, 2019, at 10:52 AM, Jonas Devlieghere via Phabricator via > lldb-commits wrote: > > JDevlieghere added a comment. > > In D60300#1455790 , @davide wrote: > >> I think this is good regardless for readability, but I would really >> appre

[Lldb-commits] [PATCH] D60300: [testsuite] Split Objective-C data formatter

2019-04-05 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere added a comment. In D60300#1455790 , @davide wrote: > I think this is good regardless for readability, but I would really > appreciate if we can collect some numbers to see how effective this change > actually is. I see 120 -> 12 seconds o

[Lldb-commits] [PATCH] D60300: [testsuite] Split Objective-C data formatter

2019-04-05 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere updated this revision to Diff 193909. JDevlieghere marked 5 inline comments as done. JDevlieghere added a comment. Code review feedback CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60300/new/ https://reviews.llvm.org/D60300 Files: lldb/packages/Python/lldbsuite/test/fu

[Lldb-commits] [PATCH] D60300: [testsuite] Split Objective-C data formatter

2019-04-05 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added inline comments. Comment at: lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-objc/TestDataFormatterObjNSException.py:30 +def nsexception_data_formatter_commands(self): +self.expect( +'frame variable except0

[Lldb-commits] [PATCH] D60300: [testsuite] Split Objective-C data formatter

2019-04-05 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added inline comments. Comment at: lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-objc/ObjCDataFormatterTestCase.py:33 +lldbutil.run_break_set_by_file_and_line( +self, "main.m", self.line, num_expected_locations=1,

[Lldb-commits] [PATCH] D60300: [testsuite] Split Objective-C data formatter

2019-04-05 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added inline comments. Comment at: lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-objc/ObjCDataFormatterTestCase.py:21 +# Find the line number to break at. +self.line = line_number('main.m', '// Set break point at this l

[Lldb-commits] [PATCH] D60300: [testsuite] Split Objective-C data formatter

2019-04-05 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. Looks fine to me, with some inline ideas you can implement if you think they're worthwhile. Also, since you're looking at reducing test time, the question you can ask yourself is: is it really worth it running separate dwarf+dsym flavours of these tests? I'm of the view

[Lldb-commits] [PATCH] D60300: [testsuite] Split Objective-C data formatter

2019-04-04 Thread Davide Italiano via Phabricator via lldb-commits
davide added a comment. I think this is good regardless for readability, but I would really appreciate if we can collect some numbers to see how effective this change actually is. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60300/new/ https://reviews.llvm.org/D60300 __

[Lldb-commits] [PATCH] D60300: [testsuite] Split Objective-C data formatter

2019-04-04 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere updated this revision to Diff 193822. JDevlieghere added a comment. Fix CF test CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60300/new/ https://reviews.llvm.org/D60300 Files: lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-objc/ObjCDat

[Lldb-commits] [PATCH] D60300: [testsuite] Split Objective-C data formatter

2019-04-04 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere created this revision. JDevlieghere added reviewers: aprantl, davide, labath. Herald added a project: LLDB. JDevlieghere updated this revision to Diff 193822. JDevlieghere added a comment. Fix CF test The testcase for objective-c data formatters is very big as it checks a bunch of