hadoop-common-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Chris Nauroth (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (HADOOP-9112) test-patch should -1 for @Tests without a timeout
Date Thu, 07 Feb 2013 17:07:14 GMT

    [ https://issues.apache.org/jira/browse/HADOOP-9112?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13573673#comment-13573673
] 

Chris Nauroth commented on HADOOP-9112:
---------------------------------------

I wonder if it's simpler to start with a regex matching approach, try it for a while, and
see if that turns out to be good enough in practice that we don't need to actually introspect
the code with apt or similar.  We could grep -c the patch file for addition of lines (presence
of '+') that have "@Test" not followed by "(timeout=...)".  If the count is non-zero, then
Jenkins votes -1.

Here is an example of a regex that matches "\+@Test" and "\+@Test()" but not "\+@Test(timeout=123)".

{code}
scala> val p = Pattern.compile("\\+@Test(?!\\(timeout|timeout=\\d+\\))")
val p = Pattern.compile("\\+@Test(?!\\(timeout|timeout=\\d+\\))")
p: java.util.regex.Pattern = \+@Test(?!\(timeout|timeout=\d+\))

scala> val strings = List("+@Test", "+@Test()", "+@Test(timeout=123)")
val strings = List("+@Test", "+@Test()", "+@Test(timeout=123)")
strings: List[java.lang.String] = List(+@Test, +@Test(), +@Test(timeout=123))

scala> strings foreach((s: String) => { println(s + " matches? " + p.matcher(s).find)
})
find) })
+@Test matches? true
+@Test() matches? true
+@Test(timeout=123) matches? false
{code}

I happened to have a Scala shell opened, so that's where I experimented.  In practice, we'd
need to pick something on Jenkins that has regular expression support for negative lookahead.
 I don't recall if stock grep supports that.

The regex above is incomplete.  It would need additional work to handle whitespace correctly,
and I'm sure there are other edge cases I haven't thought of yet.

                
> test-patch should -1 for @Tests without a timeout
> -------------------------------------------------
>
>                 Key: HADOOP-9112
>                 URL: https://issues.apache.org/jira/browse/HADOOP-9112
>             Project: Hadoop Common
>          Issue Type: Improvement
>            Reporter: Todd Lipcon
>
> With our current test running infrastructure, if a test with no timeout set runs too
long, it triggers a surefire-wide timeout, which for some reason doesn't show up as a failed
test in the test-patch output. Given that, we should require that all tests have a timeout
set, and have test-patch enforce this with a simple check

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

Mime
View raw message