(I'm also in favor of not overloading existing flags; they have some meaning/semantics that developers have come to expect.)
On Sun, Feb 19, 2017 at 12:34 PM, Jean-Baptiste Onofré <j...@nanthrax.net> wrote: > Thanks Aviem, > > Not sure if we should use skipTests as it really means unit tests and > integration tests (in Karaf, skipTests skips the unit test, integration > test, archetype itests and maven plugin invoker test, but it doesn't skip > checkstyle, findbugs, etc, for that, we have a fastinstall property). > > Maybe it would make more sense to use a specific property like > -DfastBuild=true. > > WDYT ? > > Regards > JB > > > On 02/19/2017 09:27 PM, Aviem Zur wrote: > >> I've created a PR which disables slow verifications if '-DskipTests' was >> specified, otherwise runs them. >> I think this satisfies all the considerations mentioned in this thread. >> >> PTAL: https://github.com/apache/beam/pull/2048 >> Ticket: https://issues.apache.org/jira/browse/BEAM-1513 >> >> On Thu, Feb 16, 2017 at 3:05 PM Ismaël Mejía <ieme...@gmail.com> wrote: >> >> JB, Maybe I was not clear, when I talked about the tests I was thinking >>> more about execute them in parallel in the same machine, this is not the >>> case today for some test suites, and for these the tests need to be >>> refined >>> to support this, and configured via the pom to execute the tests in >>> parallel per method, class, etc. Of course we need to check if this is >>> worth, because I can imagine that the more expensive time for example in >>> the IO case comes from starting the embedded versions of the IOs (e.g. >>> HadoopMiniCluster, MongodExecutable, HBasetestingutility, etc) and not >>> from >>> the tests themselves but this has to be evaluated. >>> >>> >>> >>> On Wed, Feb 15, 2017 at 5:46 PM, Jean-Baptiste Onofré <j...@nanthrax.net> >>> wrote: >>> >>> On Jenkins it's possible to run several jobs in the same time but on >>>> different executor. That's the easiest way. >>>> >>>> Regards >>>> JB >>>> >>>> On Feb 15, 2017, 10:15, at 10:15, "Ismaël Mejía" <ieme...@gmail.com> >>>> wrote: >>>> >>>>> This question got lost in the discussion, but there is a small >>>>> improvement >>>>> that we can do: >>>>> >>>>> Just to check, are we doing parallel builds? >>>>>> >>>>> >>>>> We are on jenkins, not in travis, there is an ongoing PR to fix this. >>>>> >>>>> What we can improve is to check if we can run some of the test suites >>>>> in >>>>> parallel to gain some extra time. For exemple most of the IOs and some >>>>> runners don't execute tests in parallel. >>>>> >>>>> Ismael >>>>> >>>>> (slightly related), is there a way to get change the timeout of travis >>>>> jobs). Because it is failing most of the time now because of this, and >>>>> it >>>>> is quite noisey to have so many false positives. >>>>> >>>>> >>>>> >>>>> >>>>> On Fri, Feb 10, 2017 at 8:00 PM, Robert Bradshaw < >>>>> rober...@google.com.invalid> wrote: >>>>> >>>>> On Fri, Feb 10, 2017 at 8:45 AM, Dan Halperin >>>>>> >>>>> <dhalp...@google.com.invalid >>>>> >>>>>> >>>>>>> wrote: >>>>>> >>>>>> On Fri, Feb 10, 2017 at 7:42 AM, Kenneth Knowles >>>>>>> >>>>>> <k...@google.com.invalid >>>>> >>>>>> >>>>>>> wrote: >>>>>>> >>>>>>> On Feb 10, 2017 07:36, "Dan Halperin" >>>>>>>> >>>>>>> <dhalp...@google.com.invalid> >>>>> >>>>>> wrote: >>>>>>> >>>>>>>> >>>>>>>> Before we added checkstyle it was under a minute. Now it's over >>>>>>>> >>>>>>> five? >>>>> >>>>>> That's awful IMO >>>>>>>> >>>>>>>> >>>>>>>> Checkstyle didn't cause all that, did it? >>>>>>>> >>>>>>>> >>>>>>> The "5 minutes" was going with Aviem's numbers after this change. >>>>>>> >>>>>> But >>>>> >>>>>> yes, >>>>>> >>>>>>> Checkstyle alone substantially (>+50%) the time from what it was >>>>>>> >>>>>> previously >>>>>> >>>>>>> to adding it back to the default build. >>>>>>> >>>>>> >>>>>> >>>>>> Just to check, are we doing parallel builds? >>>>>> >>>>>> >>>>>> >>>>>>> Noting that findbugs takes quite a lot more time. Javadoc and jar >>>>>>> >>>>>> are the >>>>> >>>>>> other two slow ones. >>>>>>>> >>>>>>>> RAT is fast. But it has very poor error messages, so we wouldn't >>>>>>>> >>>>>>> want a >>>>> >>>>>> new >>>>>>> >>>>>>>> contributor trying to figure out what is going on without our >>>>>>>> >>>>>>> help. >>>>> >>>>>> >>>>>>>> >>>>>>> There is a larger philosophical issue here: is there a point of >>>>>>> >>>>>> Jenkins >>>>> >>>>>> precommit testing? Why not just make `mvn install` run everything >>>>>>> >>>>>> that >>>>> >>>>>> Jenkins does? For that matter, why don't committers just push >>>>>>> >>>>>> directly to >>>>> >>>>>> master? Wouldn't that make everyone's life easier? >>>>>>> >>>>>>> I'd argue that's not true. >>>>>>> >>>>>>> 1. Developer productivity -- Jenkins should run many more checks >>>>>>> >>>>>> than >>>>> >>>>>> developers do. Especially time-, resource-, or setup- intensive >>>>>>> >>>>>> tasks. >>>>> >>>>>> 2. Automated enforcement -- Jenkins is better at running the right >>>>>>> >>>>>> commands >>>>>> >>>>>>> than we are. >>>>>>> 3. Lower the barrier to entry -- individual developers need not >>>>>>> >>>>>> have a >>>>> >>>>>> running Spark/Flink/Apex/Dataflow setup in order to contribute >>>>>>> >>>>>> code. >>>>> >>>>>> 4. Focus on the user -- someone checking out the code and using it >>>>>>> >>>>>> for >>>>> >>>>>> the >>>>>> >>>>>>> first time does not care whether the code style checks or has the >>>>>>> >>>>>> right >>>>> >>>>>> licenses -- that should have been enforced by the Beam team before >>>>>>> committing. >>>>>>> >>>>>>> We should be *very* choosy about what we enforce on every developer >>>>>>> >>>>>> every >>>>> >>>>>> time they go to compile. I probably compile Beam 50x-100x a day. >>>>>>> >>>>>> Literally, >>>>>> >>>>>>> the extra minutes you want to add here will cost me an hour daily. >>>>>>> >>>>>>> >>>>>> By the same token of having a different bar for the Jenkins presubmit >>>>>> >>>>> vs. >>>>> >>>>>> what's run locally, I think it makes a lot of sense to run a >>>>>> >>>>> different >>>>> >>>>>> command for iterative development than you run before creating a pull >>>>>> request. E.g. during development I'll often run only one test rather >>>>>> >>>>> than >>>>> >>>>>> the entire suite, but do run the entire suite occasionally (often >>>>>> >>>>> before >>>>> >>>>>> commit, especially before pushing). >>>>>> >>>>>> The contributors guild should give a suggested command to run before >>>>>> creating a PR, right in the docs of how to create a PR, which may be >>>>>> >>>>> more >>>>> >>>>>> expensive than what you run during development. IMHO, this should be >>>>>> >>>>> fairly >>>>> >>>>>> comprehensive (certainly tests and checkstyle, maybe javadoc and >>>>>> >>>>> findbugs). >>>>> >>>>>> This should be the "default" command that the one-time-contributor >>>>>> >>>>> should >>>>> >>>>>> know. For those compiling 50x or more a day, I think the burden of >>>>>> >>>>> learning >>>>> >>>>>> a second (or more) cheaper commands is not high, and we could even >>>>>> >>>>> put such >>>>> >>>>>> a thing in the docs (and hopefully a common maven convention like >>>>>> >>>>> "mvn >>>>> >>>>>> test"). >>>>>> >>>>>> I've listed the fraction of commits I think will break one of the >>>>>> >>>>> following >>>>> >>>>>> if that property is not tested: >>>>>>> >>>>>>> * compiling (100%) >>>>>>> * tests (100%) >>>>>>> * checkstyle (90%) >>>>>>> * javadoc (30%) >>>>>>> * findbugs (5%) >>>>>>> * rat (1%) >>>>>>> >>>>>>> So you can see where I stand and why. I'm sorry that 1/20 PRs has >>>>>>> >>>>>> Apache >>>>> >>>>>> RAT catch a licensing issue or Findbugs catch a threading issue -- >>>>>>> >>>>>> you >>>>> >>>>>> can >>>>>> >>>>>>> always get a larger set of the precommit checks using -Prelease, >>>>>>> >>>>>> though >>>>> >>>>>> of >>>>>> >>>>>>> course the integration tests and runnableonservice tests may catch >>>>>>> >>>>>> more >>>>> >>>>>> issues still. But I want my developer minutes back for the 95%+ of >>>>>>> >>>>>> the >>>>> >>>>>> rest. >>>>>>> >>>>>>> Dan >>>>>>> >>>>>>> >>>>>> >>>> >>> >> > -- > Jean-Baptiste Onofré > jbono...@apache.org > http://blog.nanthrax.net > Talend - http://www.talend.com >
