On Thu, 28 Nov 2013 21:07:14 +0100 Christian Boltz <opensuse@cboltz.de> wrote:
Hello,
Am Donnerstag, 28. November 2013 schrieb Josef Reidinger:
I hope that in future we can more focus on automatic testing of results, then on manual review of source code changes. <corporate>Because results matter</corporate> :)
I have to disagree ;-)
I'm fine with adding automatic testing, but the manual code review should stay in place.
Tricky part is that for manual code review you need time and with increased number of packages it is not possible unless manpower is increased. I agree with you that code review is really important and for example in Yast we see really good impact on code quality, but experience is that for product quality automatic testing is more important.
A good example would be code like test "$USER" == 'jreidinger' && rm -fr ~jreidinger
If something like this is added, a human reviewer will easily notice it. However, the automatic test (which is running as a different user and therefore skips the rm command) will tell you:
Test succeeded - everything is fine!
Results matter, right? ;-)
Well, what you use is malicous code and if I want to get it to factory, I think I can add it there even if nobody notice it ( OK, I expect 95% to make it unnotice ). See the most famous /usr removal code - https://github.com/MrMEEE/bumblebee-Old-and-abbandoned/commit/a047be85247755... and it is not intentional malicious code, so if you hide it in some variable like and spread it into long shell script, then it is really hard to find it.
Another example where reading the code is the better choice are changes in rarely used code paths - it happened to me more than once that I found interesting[tm] (and buggy) code when reading the sources of various programs. Sometimes those code sections were "hidden" from most users, for example in an error handling section or in code that only 1% of the users use.
I agree, but it is bug in software. Do you have idea how much time can take detail code review for all software in factory? even diffs for projects like KDE or GNOME is really HUGE! I make statistics for Yast and Yast itself contain almost 1 000 000 lines of source code, similar for kernel. I think that we should only review our own patches and keep main software on upstream. So if there is no our patches or patch is removed, then I think no manual review is needed ( checking checksum of tarball should be automatic ).
It might be possible to write tests for the "only used by 1% of the users" code, but testing all error handling (and reproducing the errors to trigger this code path) is probably harder than proofreading the source code ;-)
Yes, I agree that 100% test coverage is utopia. But same way I don't expect that we catch all errors in factory. I think that goal is to have stable factory that at least ensure that application still can run and do basic functionality, for corner cases and specific conditions it is fine for me if it contain bug, that we fix after user find it or upstream fix it and upstream create test case in perfect case :)
Regards,
Christian Boltz
Thanks for your reply, it contain some interesting ideas that allows me to more think about my own solution Josef -- To unsubscribe, e-mail: opensuse-factory+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-factory+owner@opensuse.org