Peter In terms of the modules - ignoring the time taken - I would vote for asflicense, author, findbugs, javac, maybe javadoc, wrhitespace. Not sure what checkstyle does, and some form of test4tests is already covered in ptest. This will at least help preventing new issues. Fixing the existing set would be quite an exercise.
The numbers that you have posted - I think they are on your local system? I'd expect these to be higher on the build machines. Not too keen on having the runtime go up by 10+ minutes though. Would this run before ptest is actually started? Is it possible to start this from within ptest as a parallel phase? The ptest server doesn't do much while tests are running. Running the regular ptest flow, and this set of checks could be parallelized there. Thank you for taking this up. Sid On Thu, Nov 10, 2016 at 7:57 AM, Peter Vary <pv...@cloudera.com> wrote: > Hi there, > > Previously we discussed that it would be good to integrate some automated > checks to the pre-commit flow. > Alan Gates suggested Apache Yetus and I checked what it can do for us > (Yetus 0.3.0). > > The good things that I have found: > > - Several existing tests (asflicense, author, checkstyle, findbugs, > javac, javadoc, test4tests, unitveto, whitespace, xml, junit) > - It shows changes in errors/failures so we do not have to clean up > the original code, but new code will be checked. > - Used by multiple ASF projects already - so we will be Apache conform > using it. > - Extensible, so if we decide to add the ptest framework to these test > this could be done > - It is possible to run the test only on the modules which contain > changed files > > The bad thing is it could take long time to run the tests even with > patches touching a single module. > > I think we should decide on which test to include into our pre-commit flow > based on our needs and the resource requirements. For reference I have run > the test for a fairly small patch on my macbook pro 2 times: > > 1. Adding 3 new files to the beeline module (1 java, 1 xml, 1 q.out) - > took ~4 mins - see the result in the attached beeline.out file > 2. Adding 3 new files (same as before) to the ql module (1 java, 1 > xml, 1 q.out) - took ~12 mins - see the result in the attached ql.out file > > In nutshell, the out of the box tests which are available in Yetus are > (the numbers are the time in seconds required to run the test in beeline/ql > plugin): > > - asflicense (24/23) - apache-rat:check - currently this runs for the > full path > - author (0/0) - Checks for @author tags > - checkstyle (31/66) - checkstyle:checksyle > - findbugs (73/353) - findbugs:findbugs > - javac (53/147) - install compilation warnings (the runtime presented > in the tables are not valid) > - javadoc (34/92) - javadoc warnings > - test4tests (0/0) - checks if there is any test changed > - unitveto (0/0) - checks for files in patch > - whitespace (1/2) - tabs, whitespaces at the end of the line > - xml (1/1) - xml basic validation > - junit - running junit tests - we will use ptest anyway, so not > played with this > > > I would like to know your opinion on which test should we enable, and > which test should we leave out in our pre-commit workflow. > > Thanks, > Peter > > > >
