<html><head><meta http-equiv="Content-Type" content="text/html charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;">Hi Daniel,<div>I am very aware of the code duplication, and am not excited about it.</div><div><br></div><div>The issue is that Python decorators can be called in two ways:</div><div>@decorator</div><div>or</div><div>@decorator(args)</div><div><br></div><div>and I did not want to have to change every test case from @expectedFailure to @expectedFailure()</div><div>The “if callable” line serves the purpose of supporting both syntaxes. I am afraid that to clean that up we would need a third level of decoration (the current decorator is a function that returns what essentially was the old decorator function).</div><div>If you feel like playing with Python for a while and get it right, feel free to. Or I can just keep it on my queue as something to do at the first idle-time moment.</div><div><br></div><div>As for the decorator supporting a string, is there anything (other than the argument being named <i>bugnumber</i>)<i> </i>that prevents it? If not, maybe we should just call it “bugreference” or something equivalent.</div><div><br></div><div>Sincerely,<br><div>
<div style="color: rgb(0, 0, 0); letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px; word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;"><div style="font-size: medium; orphans: 2; widows: 2; border-collapse: separate; border-spacing: 0px;"><span style="font-size: 12px; orphans: auto; widows: auto;">Enrico Granata</span><br style="font-size: 12px; orphans: auto; widows: auto;"><span style="font-size: 12px; orphans: auto; widows: auto;">✉ egranata@</span><font color="#ff2600" style="font-size: 12px; orphans: auto; widows: auto;"></font><span style="font-size: 12px; orphans: auto; widows: auto;">.com</span><br style="font-size: 12px; orphans: auto; widows: auto;"><span style="font-size: 12px; orphans: auto; widows: auto;">✆ 27683</span></div></div>
</div>
<br><div><div>On Feb 25, 2013, at 3:13 PM, "Malea, Daniel" <<a href="mailto:daniel.malea@intel.com">daniel.malea@intel.com</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite">Hi Enrico,<br><br>Thanks for doing this, it will help get much better test (and buildbot) output.<br><br>However, this commit seems to duplicate some code -- any way around that? I have a patch that adds arguments to the uses of @expectedFailure[Linux|Clang|Gcc|i386] and friends in the test cases... if that would help you avoid the giant "if callable(..)" block in the decorator body, let me know and I'll post it to the list. I'd really like to avoid that logic if possible, because as-is the variable named 'bugnumber' can be a rdar tracking number integer, or a function....quite confusing!<br><br>We are planning to enable more test compilers than GCC or Clang.... if we had a function that takes the compiler name as a string, adding that feature would be a lot cleaner than duplicating all this decorator logic.<br><br>Also, I think it would be great if the decorator would be used with a full string that describes the bug (i.e. "<a href="http://llvm.org/pr15130">llvm.org/pr15130</a>") that way we can distinguish between rdar issues and <a href="http://llvm.org">llvm.org</a> bugtracker issues.<br><br>Cheers,<br>Dan<br><br><br><br>-----Original Message-----<br>From: <a href="mailto:lldb-commits-bounces@cs.uiuc.edu">lldb-commits-bounces@cs.uiuc.edu</a> [<a href="mailto:lldb-commits-bounces@cs.uiuc.edu">mailto:lldb-commits-bounces@cs.uiuc.edu</a>] On Behalf Of Enrico Granata<br>Sent: Friday, February 22, 2013 8:35 PM<br>To: <a href="mailto:lldb-commits@cs.uiuc.edu">lldb-commits@cs.uiuc.edu</a><br>Subject: [Lldb-commits] [lldb] r175946 - This should get clang/gcc decorators working<br><br>Author: enrico<br>Date: Fri Feb 22 19:35:21 2013<br>New Revision: 175946<br><br>URL: <a href="http://llvm.org/viewvc/llvm-project?rev=175946&view=rev">http://llvm.org/viewvc/llvm-project?rev=175946&view=rev</a><br>Log:<br>This should get clang/gcc decorators working<br><br>Modified:<br>    lldb/trunk/test/lldbtest.py<br><br>Modified: lldb/trunk/test/lldbtest.py<br>URL: <a href="http://llvm.org/viewvc/llvm-project/lldb/trunk/test/lldbtest.py?rev=175946&r1=175945&r2=175946&view=diff">http://llvm.org/viewvc/llvm-project/lldb/trunk/test/lldbtest.py?rev=175946&r1=175945&r2=175946&view=diff</a><br>==============================================================================<br>--- lldb/trunk/test/lldbtest.py (original)<br>+++ lldb/trunk/test/lldbtest.py Fri Feb 22 19:35:21 2013<br>@@ -368,25 +368,25 @@ def dwarf_test(func):<br>     wrapper.__dwarf_test__ = True<br>     return wrapper<br><br>-def expectedFailureCompiler(func,compiler,bugnumber=None):<br>+def expectedFailureGcc(bugnumber=None):<br>      if callable(bugnumber):<br>         @wraps(bugnumber)<br>-        def expectedFailureCompiler_easy_wrapper(*args, **kwargs):<br>+        def expectedFailureGcc_easy_wrapper(*args, **kwargs):<br>             from unittest2 import case<br>             self = args[0]<br>             test_compiler = self.getCompiler()<br>             try:<br>                 bugnumber(*args, **kwargs)<br>             except Exception:<br>-                if compiler in test_compiler:<br>+                if "gcc" in test_compiler:<br>                     raise case._ExpectedFailure(sys.exc_info(),None)<br>                 else:<br>                     raise<br>-            if compiler in test_compiler:<br>+            if "gcc" in test_compiler:<br>                 raise case._UnexpectedSuccess(sys.exc_info(),None)<br>-        return expectedFailureCompiler_easy_wrapper<br>+        return expectedFailureGcc_easy_wrapper<br>      else:<br>-        def expectedFailureCompiler_impl(func):<br>+        def expectedFailureGcc_impl(func):<br>               @wraps(func)<br>               def wrapper(*args, **kwargs):<br>                 from unittest2 import case @@ -395,26 +395,51 @@ def expectedFailureCompiler(func,compile<br>                 try:<br>                     func(*args, **kwargs)<br>                 except Exception:<br>-                    if compiler in test_compiler:<br>+                    if "gcc" in test_compiler:<br>                         raise case._ExpectedFailure(sys.exc_info(),bugnumber)<br>                     else:<br>                         raise<br>-                if compiler in test_compiler:<br>+                if "gcc" in test_compiler:<br>                     raise case._UnexpectedSuccess(sys.exc_info(),bugnumber)<br>               return wrapper<br>-        return expectedFailureCompiler_impl<br>+        return expectedFailureGcc_impl<br><br>-def expectedFailureGcc(func):<br>-    """Decorate the item as a GCC only expectedFailure."""<br>-    if isinstance(func, type) and issubclass(func, unittest2.TestCase):<br>-        raise Exception("@expectedFailureClang can only be used to decorate a test method")<br>-    return expectedFailureCompiler(func, "gcc")<br>+def expectedFailureClang(bugnumber=None):<br>+     if callable(bugnumber):<br>+        @wraps(bugnumber)<br>+        def expectedFailureClang_easy_wrapper(*args, **kwargs):<br>+            from unittest2 import case<br>+            self = args[0]<br>+            test_compiler = self.getCompiler()<br>+            try:<br>+                bugnumber(*args, **kwargs)<br>+            except Exception:<br>+                if "clang" in test_compiler:<br>+                    raise case._ExpectedFailure(sys.exc_info(),None)<br>+                else:<br>+                    raise<br>+            if "clang" in test_compiler:<br>+                raise case._UnexpectedSuccess(sys.exc_info(),None)<br>+        return expectedFailureClang_easy_wrapper<br>+     else:<br>+        def expectedFailureClang_impl(func):<br>+              @wraps(func)<br>+              def wrapper(*args, **kwargs):<br>+                from unittest2 import case<br>+                self = args[0]<br>+                test_compiler = self.getCompiler()<br>+                try:<br>+                    func(*args, **kwargs)<br>+                except Exception:<br>+                    if "clang" in test_compiler:<br>+                        raise case._ExpectedFailure(sys.exc_info(),bugnumber)<br>+                    else:<br>+                        raise<br>+                if "clang" in test_compiler:<br>+                    raise case._UnexpectedSuccess(sys.exc_info(),bugnumber)<br>+              return wrapper<br>+        return expectedFailureClang_impl<br><br>-def expectedFailureClang(func):<br>-    """Decorate the item as a Clang only expectedFailure."""<br>-    if isinstance(func, type) and issubclass(func, unittest2.TestCase):<br>-        raise Exception("@expectedFailureClang can only be used to decorate a test method")<br>-    return expectedFailureCompiler(func, "clang")<br><br> def expectedFailurei386(bugnumber=None):<br>      if callable(bugnumber):<br><br><br>_______________________________________________<br>lldb-commits mailing list<br><a href="mailto:lldb-commits@cs.uiuc.edu">lldb-commits@cs.uiuc.edu</a><br>http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits<br></blockquote></div><br></div></body></html>