http://linuxtesting.org/upstream-tracker/test_results/avogadro/1.0.1/test_results.html
This automatically generated tests illustrate ways of possible incorrect usages of libavogadro
API leading to crashes. I’ve already submitted patches for some of these issues to Gerrit.
–
Regards,
Konstantin
This automatically generated tests illustrate ways of possible incorrect usages of libavogadro
API leading to crashes. I’ve already submitted patches for some of these issues to Gerrit.
I appreciate some of the “defensive programming” in your patches. I’d prefer to review a larger patch with a bunch of small fixes, particularly if we’re going to get 54 of these. It seems easier on your end too.
Why don’t you submit for master a few “prevent segfault” patches?
Thanks,
-Geoff
On Fri, Feb 11, 2011 at 9:33 AM, Konstantin Tokarev annulen@yandex.ru wrote:
http://linuxtesting.org/upstream-tracker/test_results/avogadro/1.0.1/test_results.html
This automatically generated tests illustrate ways of possible incorrect usages of libavogadro
API leading to crashes. I’ve already submitted patches for some of these issues to Gerrit.
We never wrote Avogadro with tests like this in mind. For the objects
that are part of a molecule, the API should probably be restricted by
simply making the constructors private. The API evolved to where it is
now, I would prefer not to have lots of tiny patches for each
individual change.
I would like to actually consider why some of these things segfault
and address it at a higher level where appropriate too. We need to be
careful as at least one of your patches would have introduced new
bugs. I would rather have a coherent set of regression tests in
addition to some of these auto-generated unit tests that only scratch
the surface.
I think one of the larger issues is to change the API a little, as we
allow things that will never work correctly. I have some local changes
I have been testing to make things safer here, and a new data
structure I have been prototyping for a 2.0 approach.
Marcus
12.02.2011, 02:01, “Marcus D. Hanwell” mhanwell@gmail.com:
On Fri, Feb 11, 2011 at 9:33 AM, Konstantin Tokarev annulen@yandex.ru; wrote:
http://linuxtesting.org/upstream-tracker/test_results/avogadro/1.0.1/test_results.html
This automatically generated tests illustrate ways of possible incorrect usages of libavogadro
API leading to crashes. I’ve already submitted patches for some of these issues to Gerrit.We never wrote Avogadro with tests like this in mind.
I realize that. BTW, test generation system is still under development, so it may generate
“bad” tests.
For the objects
that are part of a molecule, the API should probably be restricted by
simply making the constructors private.
If you know these classes are useless outside Molecule, it would be reasonable.
To keep API, we could introduce constructors taking Molecule* and mark old
ones as deprecated.
The API evolved to where it is
now, I would prefer not to have lots of tiny patches for each
individual change. I would like to actually consider why some of these things segfault
and address it at a higher level where appropriate too.
I completely agree with you.
We need to be
careful as at least one of your patches would have introduced new
bugs.
If you are about that patch with hydrogens in drawcommand, I was not sure
I’m doing it right and expected review will help
I would rather have a coherent set of regression tests in
addition to some of these auto-generated unit tests that only scratch
the surface.
On the other hand, auto-generated tests also could be improved. For
example, many Qt objects cannot work properly without QCoreApplication
instance, so it’s created in generated tests for Qt applications. We can also
make similar customizations for tests.
I think one of the larger issues is to change the API a little, as we
allow things that will never work correctly. I have some local changes
I have been testing to make things safer here, and a new data
structure I have been prototyping for a 2.0 approach.
Great news!
–
Regards,
Konstantin
11.02.2011, 19:46, “Geoffrey Hutchison” geoff.hutchison@gmail.com:
This automatically generated tests illustrate ways of possible incorrect usages of libavogadro
API leading to crashes. I’ve already submitted patches for some of these issues to Gerrit.I appreciate some of the “defensive programming” in your patches. I’d prefer to review a larger patch with a bunch of small fixes, particularly if we’re going to get 54 of these. It seems easier on your end too.
I’m not sure if it’s worthwile to fix all 54: test system is not perfect and we should decide
what crash cases are realistic enough, and how to prevent more of them with less shots
Why don’t you submit for master a few “prevent segfault” patches?
I hope Marcus will cherry-pick them eventually (git log master…1.0)
–
Regards,
Konstantin