I want to throw this out to the list for feedback. Something we have been doing for a while now with the jQuery project is to require a unit test(s) for any PR or change committed. This has worked very well in two ways. First, it provides a built in way to see the issue being fixed/implemented within the PR. That way, the reviewer doesn't have to build their own test(s) to see if the issue being addressed is actually fixed. Second, it helps prevent regressions down the road since more of the code is covered by tests so you know if some change you think is unrelated breaks something that fixed days, weeks, years ago. I would like to suggest we make this policy for at least the JS library (since that is the one I review most often) but I believe this policy would be useful across the entire project. Let me know what you think. Thanks, Kris _______________________________________________ aerogear-dev mailing list aerogear-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/aerogear-dev

On Thu, Jan 31, 2013 at 12:12 PM, Kris Borchers <kris(a)redhat.com&gt; wrote: > I want to throw this out to the list for feedback. Something we have been doing for a while now with the jQuery project is to require a unit test(s) for any PR or change committed. This has worked very > well in two ways. First, it provides a built in way to see the issue being fixed/implemented within the PR. That way, the reviewer doesn't have to build their own test(s) to see if the issue being > addressed is actually fixed. Second, it helps prevent regressions down the road since more of the code is covered by tests so you know if some change you think is unrelated breaks something that > fixed days, weeks, years ago. +1 most OS projects I worked on do that. (besides for typos and other minor things) > I would like to suggest we make this policy for at least the JS library (since that is the one I review most often) but I believe this policy would be useful across the entire project. Let me know what > you think. IMO this is a must, for all the bits. Also... I'd strongly vote for supporting an (automated) integration test suite (against a real server). Mock testing is OK... but we should never forget about the real environment -M

> Thanks, > Kris > _______________________________________________ > aerogear-dev mailing list > aerogear-dev(a)lists.jboss.org > https://lists.jboss.org/mailman/listinfo/aerogear-dev

On 01/31/2013 12:33 PM, Matthias Wessendorf wrote: > On Thu, Jan 31, 2013 at 12:12 PM, Kris Borchers <kris(a)redhat.com&gt; wrote: >> I want to throw this out to the list for feedback. Something we have been doing for a while now with the jQuery project is to require a unit test(s) for any PR or change committed. This has worked very >> well in two ways. First, it provides a built in way to see the issue being fixed/implemented within the PR. That way, the reviewer doesn't have to build their own test(s) to see if the issue being >> addressed is actually fixed. Second, it helps prevent regressions down the road since more of the code is covered by tests so you know if some change you think is unrelated breaks something that >> fixed days, weeks, years ago. > +1 most OS projects I worked on do that. > (besides for typos and other minor things) > > >> I would like to suggest we make this policy for at least the JS library (since that is the one I review most often) but I believe this policy would be useful across the entire project. Let me know what >> you think. > IMO this is a must, for all the bits. > > Also... I'd strongly vote for supporting an (automated) integration > test suite (against a real server). > Mock testing is OK... but we should never forget about the real environment > > -M If we do this, I'm going to throw out the integration test server should rerun the dependant tests as part of its build too.

> >> Thanks, >> Kris >> _______________________________________________ >> aerogear-dev mailing list >> aerogear-dev(a)lists.jboss.org >> https://lists.jboss.org/mailman/listinfo/aerogear-dev > > _______________________________________________ aerogear-dev mailing list aerogear-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/aerogear-dev

On 01/31/2013 12:12 PM, Kris Borchers wrote: > I want to throw this out to the list for feedback. Something we have been doing for a while now with the jQuery project is to require a unit test(s) for any PR or change committed. This has worked very well in two ways. First, it provides a built in way to see the issue being fixed/implemented within the PR. That way, the reviewer doesn't have to build their own test(s) to see if the issue being addressed is actually fixed. Second, it helps prevent regressions down the road since more of the code is covered by tests so you know if some change you think is unrelated breaks something that fixed days, weeks, years ago. > > I would like to suggest we make this policy for at least the JS library (since that is the one I review most often) but I believe this policy would be useful across the entire project. Let me know what you think. On the one hand I really REALLY appreciate good testing. If your code is easy to write tests for it is (in theory) easy to use. Otoh I REALLY hate writing tests for obvious implementations/impossible scenarios just to get coverage.

(ex In Java land obvious is getter/setters and an impossible scenario would be catching UnsupportedEncodingException for String.getBytes("UTF-8").) > > Thanks, > Kris > _______________________________________________ > aerogear-dev mailing list > aerogear-dev(a)lists.jboss.org > https://lists.jboss.org/mailman/listinfo/aerogear-dev _______________________________________________ aerogear-dev mailing list aerogear-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/aerogear-dev

+1 Not only for JS libs. -- "The measure of a man is what he does with power" - Plato - @abstractj - Volenti Nihil Difficile On Thursday, January 31, 2013 at 3:12 PM, Kris Borchers wrote: I want to throw this out to the list for feedback. Something we have been doing for a while now with the jQuery project is to require a unit test(s) for any PR or change committed. This has worked very well in two ways. First, it provides a built in way to see the issue being fixed/implemented within the PR. That way, the reviewer doesn't have to build their own test(s) to see if the issue being addressed is actually fixed. Second, it helps prevent regressions down the road since more of the code is covered by tests so you know if some change you think is unrelated breaks something that fixed days, weeks, years ago. I would like to suggest we make this policy for at least the JS library (since that is the one I review most often) but I believe this policy would be useful across the entire project. Let me know what you think. Thanks, Kris _______________________________________________ aerogear-dev mailing list aerogear-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/aerogear-dev _______________________________________________ aerogear-dev mailing list aerogear-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/aerogear-dev

On Thu, Jan 31, 2013 at 06:15:47PM +0100, Sebastien Blanc wrote: > Test Driven Development is for sure the way to go ! Let's not confound TDD with having tests ;) -- qmx _______________________________________________ aerogear-dev mailing list aerogear-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/aerogear-dev

On Thu, Jan 31, 2013 at 12:53:35PM -0500, Summers Pittman wrote: > If we do this, I'm going to throw out the integration test server should > rerun the dependant tests as part of its build too. That's the default behavior for jenkins building maven projects :)

-- qmx _______________________________________________ aerogear-dev mailing list aerogear-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/aerogear-dev