Mailinglist Archive: opensuse-factory (1029 mails)

< Previous Next >
Re: [opensuse-factory] O Factory - Where art Thou?
On Thu, 28 Nov 2013 21:07:14 +0100
Christian Boltz <opensuse@xxxxxxxxx> 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/a047be85247755cdbe0acce6f1dafc8beb84f2ac#diff-3fbb47e318cd8802bd325e7da9aaabe8L351

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@xxxxxxxxxxxx
To contact the owner, e-mail: opensuse-factory+owner@xxxxxxxxxxxx

< Previous Next >
Follow Ups