[yast-devel] Regressions due to Simple Mistakes
Hi, I see again and again regressions due to simple mistakes, e.g. bsc #1119678 or bsc #1119699. Apparently code reviews, unit tests nor rubocop did help in these cases (although the reviewers found quite some mistakes in https://github.com/yast/yast-yast2/pull/872). Real tests would have helped but it seems as if those were not done. Even simple static code analysis would have prevented those two bugs but we do not have it for Ruby. So what can be done to avoid such regressions in the future? Or do we just bury our heads in the sand? ciao Arvin -- Arvin Schnell, <aschnell@suse.com> Senior Software Engineer, Research & Development SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg) Maxfeldstraße 5 90409 Nürnberg Germany -- To unsubscribe, e-mail: yast-devel+unsubscribe@opensuse.org To contact the owner, e-mail: yast-devel+owner@opensuse.org
V Tue, 18 Dec 2018 08:51:48 +0000 Arvin Schnell <aschnell@suse.com> napsáno:
Hi,
I see again and again regressions due to simple mistakes, e.g. bsc #1119678 or bsc #1119699. Apparently code reviews, unit tests nor rubocop did help in these cases (although the reviewers found quite some mistakes in https://github.com/yast/yast-yast2/pull/872).
Real tests would have helped but it seems as if those were not done. Even simple static code analysis would have prevented those two bugs but we do not have it for Ruby.
So what can be done to avoid such regressions in the future? Or do we just bury our heads in the sand?
Ideally all modified code should be covered by tests, but in this case we get security audit with stuff to fix which is too huge to be done with proper test coverage. And also this parts of code was quite old and almost not covered by tests ( even old ones ). So in this case to prevent potential security which need a lot of changes ( 500 just in shell injection and relative paths ) I do not do proper unit testing which will otherwise shows this issues. So answer is as usual unit testing and as last stand before customers openQA ( which in this case works well, as I see all bugs are caught by it ). It just need time when working with old code to cover changes properly and sadly in this case we did not have time. Josef
ciao Arvin
-- To unsubscribe, e-mail: yast-devel+unsubscribe@opensuse.org To contact the owner, e-mail: yast-devel+owner@opensuse.org
On 12/18/18 10:30 AM, Josef Reidinger wrote:
V Tue, 18 Dec 2018 08:51:48 +0000 Arvin Schnell <aschnell@suse.com> napsáno:
Hi,
I see again and again regressions due to simple mistakes, e.g. bsc #1119678 or bsc #1119699. Apparently code reviews, unit tests nor rubocop did help in these cases (although the reviewers found quite some mistakes in https://github.com/yast/yast-yast2/pull/872).
Real tests would have helped but it seems as if those were not done. Even simple static code analysis would have prevented those two bugs but we do not have it for Ruby.
So what can be done to avoid such regressions in the future? Or do we just bury our heads in the sand?
Ideally all modified code should be covered by tests, but in thiscase we get security audit with stuff to fix which is too huge to be done with proper test coverage. And also this parts of code was quite old and almost not covered by tests ( even old ones ). So in this case to prevent potential security which need a lot of changes ( 500 just in shell injection and relative paths ) I do not do proper unit testing which will otherwise shows this issues. So answer is as usual unit testing and as last stand before customers openQA ( which in this case works well, as I see all bugs are caught by it ). It just need time when working with old code to cover changes properly and sadly in this case we did not have time.
"all bugs are caught by it" sounds pretty optimistic. ;-) BTW, it would be nice to have test coverage information for openQA. Cheers. -- Ancor González Sosa YaST Team at SUSE Linux GmbH -- To unsubscribe, e-mail: yast-devel+unsubscribe@opensuse.org To contact the owner, e-mail: yast-devel+owner@opensuse.org
V Tue, 18 Dec 2018 10:38:51 +0100 Ancor Gonzalez Sosa <ancor@suse.de> napsáno:
On 12/18/18 10:30 AM, Josef Reidinger wrote:
V Tue, 18 Dec 2018 08:51:48 +0000 Arvin Schnell <aschnell@suse.com> napsáno:
Hi,
I see again and again regressions due to simple mistakes, e.g. bsc #1119678 or bsc #1119699. Apparently code reviews, unit tests nor rubocop did help in these cases (although the reviewers found quite some mistakes in https://github.com/yast/yast-yast2/pull/872).
Real tests would have helped but it seems as if those were not done. Even simple static code analysis would have prevented those two bugs but we do not have it for Ruby.
So what can be done to avoid such regressions in the future? Or do we just bury our heads in the sand?
Ideally all modified code should be covered by tests, but in thiscase we get security audit with stuff to fix which is too huge to be done with proper test coverage. And also this parts of code was quite old and almost not covered by tests ( even old ones ). So in this case to prevent potential security which need a lot of changes ( 500 just in shell injection and relative paths ) I do not do proper unit testing which will otherwise shows this issues. So answer is as usual unit testing and as last stand before customers openQA ( which in this case works well, as I see all bugs are caught by it ). It just need time when working with old code to cover changes properly and sadly in this case we did not have time.
"all bugs are caught by it" sounds pretty optimistic. ;-)
BTW, it would be nice to have test coverage information for openQA.
Cheers.
Sorry, one word missing "all mentioned bugs are caught by it" :) Josef -- To unsubscribe, e-mail: yast-devel+unsubscribe@opensuse.org To contact the owner, e-mail: yast-devel+owner@opensuse.org
participants (3)
-
Ancor Gonzalez Sosa -
Arvin Schnell -
Josef Reidinger