As far connecting in findbugs, etc. should we consider integrating with Apache Yetus for all this?
Alan. > On Oct 14, 2016, at 12:27, Siddharth Seth <ss...@apache.org> wrote: > > Once we agree upon what the flow should be, I'm also in favor of reverting > patches which break things. Such commits cause problems for all subsequent > patches - where multiple people end up debugging the test. > In terms of disabling tests - we may want to consider how often the test > fails, before disabling them. There are flaky tests - and I suspect we'll > end up with a lot of disabled tests if a test is disabled for a single > failure. > There have been offline suggestions about adding findbugs analysis, rat > checks etc. The indentation checks, I think, falls into the same category. > Would be really nice to hear from others as well. These changes will be > painful to start with - but should help once the tests stabilize further. > > On Fri, Oct 14, 2016 at 2:01 AM, Peter Vary <pv...@cloudera.com> wrote: > >> +1 from me too. >> >> I think it lowers the barrier for the contributors, if there are clean and >> easy to follow rules for adding a patch. I had very good experience with >> the Hadoop commit flow where only “all green” results are accepted. >> >> If we are thinking about changes in the commit flow, it would be great to >> run code format checks before and after the patch, and call out any >> increase of formatting errors. I see too many nits in the reviews where the >> reviewer points out formatting problems. This could be avoided and the >> reviewer can concentrate on the real code instead of formatting. >> >> After we decide the new commit flow we should update the documentation on >> the wiki as well. >> >> Of course I am happy to help out with any of the related tasks too, so if >> you need my help just say so :) >> >> Thanks, >> Peter >> >>> On Oct 14, 2016, at 10:29 AM, Zsombor Klara <zsombor.kl...@cloudera.com> >> wrote: >>> >>> +1 from my side as well. I also like the suggestion to revert a patch >>> causing new test failures instead of expecting the owner to fix the >> issues >>> in follow up jiras. >>> >>> And I would also have a borderline "radical" suggestion.. if a flaky test >>> is failing and the committer isn't sure at a glance that the failure is >>> benign (stat diff failure for example I would consider low risk, race >>> conditions on the other hand high risk), put an ignore annotation on it. >> In >>> my view a flaky test is pretty much useless. If it breaks people will >> just >>> ignore it (because it fails so often...) and anyway how can you be sure >>> that the green run is the standard and the failure is the exception and >> not >>> the other way around. We have thousands of tests, ignoring a dozen won't >>> decrease coverage significantly and will take us that much closer to a >>> point where we can demand a green pre-commit run. >>> >>> Thanks >>> >>> On Fri, Oct 14, 2016 at 9:45 AM, Prasanth Jayachandran < >>> pjayachand...@hortonworks.com> wrote: >>> >>>> +1 on the proposal. Adding more to it. >>>> >>>> A lot of time has been spent on improving the test runtime and bringing >>>> down the flaky tests. >>>> Following jiras should give an overview of the effort involved >>>> https://issues.apache.org/jira/browse/HIVE-14547 >>>> https://issues.apache.org/jira/browse/HIVE-13503 >>>> >>>> Committers please ensure that the reported failures are absolutely not >>>> related >>>> to the patch before committing it. >>>> >>>> I would also propose the following to maintain a clean and some tips to >>>> maintain fast test runs >>>> >>>> 1) Revert patch that is causing a failure. It should be the >> responsibility >>>> of >>>> the contributor to make sure the patch is not causing any failures. I am >>>> against creating follow ups >>>> for fixing test failures usually because it gets ignored or it gets >> lower >>>> priority causing wasted effort >>>> and time for failure analysis for every other developers waiting to >> commit >>>> their patch. >>>> >>>> 2) +1 from reviewers AFTER a clean green run. Or if a reviewer is >>>> convinced that test failures are unrelated. >>>> May be we should stop conditional +1s and wait for clean green run. >>>> >>>> 3) Avoid slow tests (there is jira to print out runtime of newly added >>>> tests). In general, its good >>>> to have many smaller tests as opposed to single big tests. If the qfile >> or >>>> junit test is big, splitting it >>>> up will help in parallelizing them and avoiding stragglers. >>>> >>>> 4) Avoid adding tests to MiniMr (slowest of all). >>>> >>>> 5) Try to keep the test runtime (see surefire-report to get correct >>>> runtime without initialization time) under a minute. >>>> >>>> 6) Avoid adding more read-only tables to init script as this will >> increase >>>> the initialization time. >>>> >>>> 7) If the test case does not require explain plan then avoid it as most >>>> failures are explain diffs. >>>> >>>> 8) If the test case requires explain and if it does not depend on table >> or >>>> partition stats explicitly set stats for the table or partition. >>>> Explicitly setting stats will avoid expensive stats computation time and >>>> avoids flakiness due to stats diff. >>>> >>>> 9) Prefer jUnit over qtest. >>>> >>>> 10) Add explicitly timeout for jUnit test to avoid indefinite hanging of >>>> tests (surefire timeouts after 40 mins) >>>> >>>> Thoughts? >>>> >>>> Thanks >>>> Prasanth >>>> >>>> On Oct 13, 2016, at 11:10 PM, Siddharth Seth <ss...@apache.org<mailto: >>>> ss...@apache.org>> wrote: >>>> >>>> There's been a lot of work to make the test runs faster, as well as more >>>> reliable via HIVE-14547, HIVE-13503, and several other jiras. Test >> runtimes >>>> are around the 1 hour mark, and going down. There were a few green >>>> pre-commit runs (after years?). At the same time, there's still some >> flaky >>>> tests. >>>> >>>> We really should try to keep the test runtimes down, as well as the >> number >>>> of failures - so that the pre-commit runs can provide useful >> information. >>>> >>>> I'm not sure what the current approach w.r.t precommit runs before a >>>> commit. What I've seen in other projects is that the pre-commit needs to >>>> run, and come back clean (mostly) before a commit goes in. Between what >>>> used to be 5 day wait times, and inconsistent runs - I don't think this >> is >>>> always followed in Hive. >>>> >>>> It'll be useful to start relying on pre-commit test results again. Given >>>> the flaky tests, I'd suggest the following >>>> 1. Pre-commit must be run on a patch before committing (with very few >>>> exceptions) >>>> 2. A green test run is ideal >>>> 3. In case there are failures - keep track of these as sub-jiras under a >>>> flaky test umbrella jira (Some under HIVE-14547 already) - to be >> eventually >>>> fixed. >>>> 4. Before committing - cite relevant jiras for a flaky test (create and >>>> cite if it doesn't already exist). >>>> >>>> This should help us build up a list of flaky tests over various runs, >> which >>>> will hopefully get fixed at some point. >>>> >>>> Thoughts? >>>> >>>> Thanks, >>>> Sid >>>> >>>> >> >>
