I agree, I'll change it to -Dquick On Tue, Feb 21, 2017 at 12:07 AM Davor Bonaci <[email protected]> wrote:
> (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é <[email protected]> > 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 <[email protected]> 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é <[email protected] > > > >>> 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" <[email protected]> > >>>> 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 < > >>>>> [email protected]> wrote: > >>>>> > >>>>> On Fri, Feb 10, 2017 at 8:45 AM, Dan Halperin > >>>>>> > >>>>> <[email protected] > >>>>> > >>>>>> > >>>>>>> wrote: > >>>>>> > >>>>>> On Fri, Feb 10, 2017 at 7:42 AM, Kenneth Knowles > >>>>>>> > >>>>>> <[email protected] > >>>>> > >>>>>> > >>>>>>> wrote: > >>>>>>> > >>>>>>> On Feb 10, 2017 07:36, "Dan Halperin" > >>>>>>>> > >>>>>>> <[email protected]> > >>>>> > >>>>>> 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é > > [email protected] > > http://blog.nanthrax.net > > Talend - http://www.talend.com > > >
