Re: [OpenBabel-Devel] Proposal to require test cases

[Noel O'Boyle]

My recommended procedure would be to write to stdout and capture it.
There's no guarantee that we can write to disk. I can add a line on that...

On 26 April 2018 at 12:06, Koes, David  wrote:

> Do you want to have a recommended procedure for tests that write files and
> then need to compare them?  I’m assuming from what you wrote that tests are
> automatically run from the tests/files directory and I think you would want
> to discourage people from cluttering that directory with output files.
>
> David Koes
>
> Assistant Professor
> Computational & Systems Biology
> University of Pittsburgh
>
>
> On Apr 26, 2018, at 2:43 AM, Noel O'Boyle  wrote:
>
> Okay, I think it's close to completion. Again, comments welcome...
>
> https://gist.github.com/baoilleach/d9f02fb906e70277f6b4721d63d68b59
> 
>
> - Noel
>
>
>
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot___
OpenBabel-Devel mailing list
OpenBabel-Devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openbabel-devel

Re: [OpenBabel-Devel] Proposal to require test cases

[Koes, David]

Do you want to have a recommended procedure for tests that write files and then 
need to compare them?  I’m assuming from what you wrote that tests are 
automatically run from the tests/files directory and I think you would want to 
discourage people from cluttering that directory with output files.

David Koes

Assistant Professor
Computational & Systems Biology
University of Pittsburgh


On Apr 26, 2018, at 2:43 AM, Noel O'Boyle 
> wrote:

Okay, I think it's close to completion. Again, comments welcome...

https://gist.github.com/baoilleach/d9f02fb906e70277f6b4721d63d68b59

- Noel

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot___
OpenBabel-Devel mailing list
OpenBabel-Devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openbabel-devel

Re: [OpenBabel-Devel] Proposal to require test cases

[Noel O'Boyle]

Okay, I think it's close to completion. Again, comments welcome...

https://gist.github.com/baoilleach/d9f02fb906e70277f6b4721d63d68b59

- Noel

On 25 April 2018 at 13:50, David Koes  wrote:

> Missing description of OB_COMPARE (and other asserts?).  Is the idea of
> OB_REQUIRE that it should be used for checks that would indicate there is
> something wrong with the overall test harness because it will end _all_
> tests, not the current one?
>
> Could give one sentence description of run_exec (a helper function that
> passes the contents of its first argument as stdin to the commandline
> specified as the second argument).
>
> Definitely want to know how to add input/output files and properly
> reference them from all three test scenarios.
>
> David Koes
>
> Assistant Professor
> Computational & Systems Biology
> University of Pittsburgh
>
> On 04/25/2018 02:21 AM, Noel O'Boyle wrote:
>
>> (Sorry - previous email was sent by accident)
>>
>> On the topic of documentation, here's what I've got so far:
>> https://gist.github.com/baoilleach/d9f02fb906e70277f6b4721d63d68b59 <
>> https://na01.safelinks.protection.outlook.com/?url=https%
>> 3A%2F%2Fgist.github.com%2Fbaoilleach%2Fd9f02fb906e70277f6b47
>> 21d63d68b59=01%7C01%7Cdkoes%40pitt.edu%7Cf543c3eb2
>> 60b487ad9ad08d5aa74c860%7C9ef9f489e0a04eeb87cc3a526112fd0d%
>> 7C1=gBUFcMEX89RR%2FA95ZIh2w9wzBFTS2grmYxFA2uZMI4Q%3D=0>
>>
>> I still need to add descriptions of how to specify testfiles, and handle
>> exceptions. Apart from that, anything missing or could be explained better?
>>
>> - Noel
>>
>
> 
> --
> Check out the vibrant tech community on one of the world's most
> engaging tech sites, Slashdot.org! http://sdm.link/slashdot
> ___
> OpenBabel-Devel mailing list
> OpenBabel-Devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/openbabel-devel
>
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot___
OpenBabel-Devel mailing list
OpenBabel-Devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openbabel-devel

Re: [OpenBabel-Devel] Proposal to require test cases

[David Koes]

Missing description of OB_COMPARE (and other asserts?).  Is the idea of OB_REQUIRE that it should be used for checks 
that would indicate there is something wrong with the overall test harness because it will end _all_ tests, not the 
current one?


Could give one sentence description of run_exec (a helper function that passes the contents of its first argument as 
stdin to the commandline specified as the second argument).


Definitely want to know how to add input/output files and properly reference 
them from all three test scenarios.

David Koes

Assistant Professor
Computational & Systems Biology
University of Pittsburgh

On 04/25/2018 02:21 AM, Noel O'Boyle wrote:

(Sorry - previous email was sent by accident)

On the topic of documentation, here's what I've got so far:
https://gist.github.com/baoilleach/d9f02fb906e70277f6b4721d63d68b59 



I still need to add descriptions of how to specify testfiles, and handle exceptions. Apart from that, anything missing 
or could be explained better?


- Noel


--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
OpenBabel-Devel mailing list
OpenBabel-Devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openbabel-devel

Re: [OpenBabel-Devel] Proposal to require test cases

[Noel O'Boyle]

(Sorry - previous email was sent by accident)

On the topic of documentation, here's what I've got so far:
https://gist.github.com/baoilleach/d9f02fb906e70277f6b4721d63d68b59

I still need to add descriptions of how to specify testfiles, and handle
exceptions. Apart from that, anything missing or could be explained better?

- Noel

On 25 April 2018 at 07:17, Noel O'Boyle  wrote:

> On the topic of documentation, here's what I've got so far. Anything
> missing?
>
> On 17 April 2018 at 06:58, David van der Spoel 
> wrote:
>
>> Den 2018-04-17 kl. 07:51, skrev Noel O'Boyle:
>>
>>> To avoid digressing, absolutely we would like to do this and have the
>>> technical means to enforce...once we reduce the warnings. I did a certain
>>> amount already this year. Once a particular type of warning is eliminated
>>> we can add it as a requirement using gcc's treat warnings as errors. But
>>> we're not there yet. Like Geoff says, we encourage people to help.
>>>
>>> If there's some other way this can be enforced on a patch by patch basis
>>> (e.g. controlling for an increase in warnings), I'd be interested to hear.
>>> Maybe you can point me to the relevant person over at Gromacs.
>>>
>>> The GROMACS system uses gerrit https://www.gerritcodereview.com/ for
>> reviewing code and jenkins for building in the background
>> https://jenkins.io/index.html
>>
>> As far as I understand this is not entirely trivial to maintain, but once
>> it is set up it works nicely.
>>
>>
>> - Noel
>>>
>>>
>>> On Tue, 17 Apr 2018, 05:18 David van der Spoel, >> > wrote:
>>>
>>> Den 2018-04-16 kl. 22:46, skrev Dominik 'Rathann' Mierzejewski:
>>>  > On Monday, 16 April 2018 at 20:20, David van der Spoel wrote:
>>>  >> Den 2018-04-16 kl. 17:36, skrev David Koes:
>>>  >>> I didn't chime in since I thought it was obviously a good idea.
>>>  >>> However, I strongly agree that the process of creating a test
>>> case needs
>>>  >>> to be as simple and documented as possible.  I had a test case
>>> with my
>>>  >>> last pull request, but it required a fair amount of poking
>>> around to
>>>  >>> figure out how to best implement it (and this experience
>>> prompted the
>>>  >>> GSoC project).
>>>  >>>
>>>  >>> Also, test cases may not make sense for some pull requests (e.g.
>>>  >>> documentation).
>>>  >>
>>>  >> Agree tests are a must.
>>>  >>
>>>  >> How about making warning-free code a must?
>>>  >
>>>  > Warning-free under which compiler (and version)? GCC adds new
>>> warnings
>>>  > in every release.
>>> Under all compilers. Obviously we can only fix the warnings we are
>>> getting.
>>>
>>> We have this policy in http://www.gromacs.org and it is enforced
>>> automatically by the robot that verifies patches. No patches that
>>> produce warnings on any platform (Linux, Mac, Windows) will be
>>> accepted.
>>>
>>>  >
>>>  > Regards,
>>>  > Dominik
>>>  >
>>>
>>>
>>> -- David van der Spoel, Ph.D., Professor of Biology
>>> Head of Department, Cell & Molecular Biology, Uppsala University.
>>> Box 596, SE-75124 Uppsala, Sweden. Phone: +46184714205.
>>> http://www.icm.uu.se
>>>
>>> 
>>> --
>>> Check out the vibrant tech community on one of the world's most
>>> engaging tech sites, Slashdot.org! http://sdm.link/slashdot
>>> ___
>>> OpenBabel-Devel mailing list
>>> OpenBabel-Devel@lists.sourceforge.net
>>> 
>>> https://lists.sourceforge.net/lists/listinfo/openbabel-devel
>>> 
>>>
>>>
>>
>> --
>> David van der Spoel, Ph.D., Professor of Biology
>> Head of Department, Cell & Molecular Biology, Uppsala University.
>> Box 596, SE-75124 Uppsala, Sweden. Phone: +46184714205.
>> http://www.icm.uu.se
>>
>> 
>> --
>> Check out the vibrant tech community on one of the world's most
>> engaging tech sites, Slashdot.org! http://sdm.link/slashdot
>> ___
>> OpenBabel-Devel mailing list
>> OpenBabel-Devel@lists.sourceforge.net
>> https://lists.sourceforge.net/lists/listinfo/openbabel-devel
>>
>
>
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot___
OpenBabel-Devel mailing list
OpenBabel-Devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openbabel-devel

Re: [OpenBabel-Devel] Proposal to require test cases

[Noel O'Boyle]

On the topic of documentation, here's what I've got so far. Anything
missing?

On 17 April 2018 at 06:58, David van der Spoel  wrote:

> Den 2018-04-17 kl. 07:51, skrev Noel O'Boyle:
>
>> To avoid digressing, absolutely we would like to do this and have the
>> technical means to enforce...once we reduce the warnings. I did a certain
>> amount already this year. Once a particular type of warning is eliminated
>> we can add it as a requirement using gcc's treat warnings as errors. But
>> we're not there yet. Like Geoff says, we encourage people to help.
>>
>> If there's some other way this can be enforced on a patch by patch basis
>> (e.g. controlling for an increase in warnings), I'd be interested to hear.
>> Maybe you can point me to the relevant person over at Gromacs.
>>
>> The GROMACS system uses gerrit https://www.gerritcodereview.com/ for
> reviewing code and jenkins for building in the background
> https://jenkins.io/index.html
>
> As far as I understand this is not entirely trivial to maintain, but once
> it is set up it works nicely.
>
>
> - Noel
>>
>>
>> On Tue, 17 Apr 2018, 05:18 David van der Spoel, > > wrote:
>>
>> Den 2018-04-16 kl. 22:46, skrev Dominik 'Rathann' Mierzejewski:
>>  > On Monday, 16 April 2018 at 20:20, David van der Spoel wrote:
>>  >> Den 2018-04-16 kl. 17:36, skrev David Koes:
>>  >>> I didn't chime in since I thought it was obviously a good idea.
>>  >>> However, I strongly agree that the process of creating a test
>> case needs
>>  >>> to be as simple and documented as possible.  I had a test case
>> with my
>>  >>> last pull request, but it required a fair amount of poking
>> around to
>>  >>> figure out how to best implement it (and this experience
>> prompted the
>>  >>> GSoC project).
>>  >>>
>>  >>> Also, test cases may not make sense for some pull requests (e.g.
>>  >>> documentation).
>>  >>
>>  >> Agree tests are a must.
>>  >>
>>  >> How about making warning-free code a must?
>>  >
>>  > Warning-free under which compiler (and version)? GCC adds new
>> warnings
>>  > in every release.
>> Under all compilers. Obviously we can only fix the warnings we are
>> getting.
>>
>> We have this policy in http://www.gromacs.org and it is enforced
>> automatically by the robot that verifies patches. No patches that
>> produce warnings on any platform (Linux, Mac, Windows) will be
>> accepted.
>>
>>  >
>>  > Regards,
>>  > Dominik
>>  >
>>
>>
>> -- David van der Spoel, Ph.D., Professor of Biology
>> Head of Department, Cell & Molecular Biology, Uppsala University.
>> Box 596, SE-75124 Uppsala, Sweden. Phone: +46184714205.
>> http://www.icm.uu.se
>>
>> 
>> --
>> Check out the vibrant tech community on one of the world's most
>> engaging tech sites, Slashdot.org! http://sdm.link/slashdot
>> ___
>> OpenBabel-Devel mailing list
>> OpenBabel-Devel@lists.sourceforge.net
>> 
>> https://lists.sourceforge.net/lists/listinfo/openbabel-devel
>> 
>>
>>
>
> --
> David van der Spoel, Ph.D., Professor of Biology
> Head of Department, Cell & Molecular Biology, Uppsala University.
> Box 596, SE-75124 Uppsala, Sweden. Phone: +46184714205.
> http://www.icm.uu.se
>
> 
> --
> Check out the vibrant tech community on one of the world's most
> engaging tech sites, Slashdot.org! http://sdm.link/slashdot
> ___
> OpenBabel-Devel mailing list
> OpenBabel-Devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/openbabel-devel
>
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot___
OpenBabel-Devel mailing list
OpenBabel-Devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openbabel-devel

Re: [OpenBabel-Devel] Proposal to require test cases

[David van der Spoel]

Den 2018-04-17 kl. 07:51, skrev Noel O'Boyle:
To avoid digressing, absolutely we would like to do this and have the 
technical means to enforce...once we reduce the warnings. I did a 
certain amount already this year. Once a particular type of warning is 
eliminated we can add it as a requirement using gcc's treat warnings as 
errors. But we're not there yet. Like Geoff says, we encourage people to 
help.


If there's some other way this can be enforced on a patch by patch basis 
(e.g. controlling for an increase in warnings), I'd be interested to 
hear. Maybe you can point me to the relevant person over at Gromacs.


The GROMACS system uses gerrit https://www.gerritcodereview.com/ for 
reviewing code and jenkins for building in the background 
https://jenkins.io/index.html


As far as I understand this is not entirely trivial to maintain, but 
once it is set up it works nicely.




- Noel

On Tue, 17 Apr 2018, 05:18 David van der Spoel, > wrote:


Den 2018-04-16 kl. 22:46, skrev Dominik 'Rathann' Mierzejewski:
 > On Monday, 16 April 2018 at 20:20, David van der Spoel wrote:
 >> Den 2018-04-16 kl. 17:36, skrev David Koes:
 >>> I didn't chime in since I thought it was obviously a good idea.
 >>> However, I strongly agree that the process of creating a test
case needs
 >>> to be as simple and documented as possible.  I had a test case
with my
 >>> last pull request, but it required a fair amount of poking
around to
 >>> figure out how to best implement it (and this experience
prompted the
 >>> GSoC project).
 >>>
 >>> Also, test cases may not make sense for some pull requests (e.g.
 >>> documentation).
 >>
 >> Agree tests are a must.
 >>
 >> How about making warning-free code a must?
 >
 > Warning-free under which compiler (and version)? GCC adds new
warnings
 > in every release.
Under all compilers. Obviously we can only fix the warnings we are
getting.

We have this policy in http://www.gromacs.org and it is enforced
automatically by the robot that verifies patches. No patches that
produce warnings on any platform (Linux, Mac, Windows) will be accepted.

 >
 > Regards,
 > Dominik
 >


-- 
David van der Spoel, Ph.D., Professor of Biology

Head of Department, Cell & Molecular Biology, Uppsala University.
Box 596, SE-75124 Uppsala, Sweden. Phone: +46184714205.
http://www.icm.uu.se


--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
OpenBabel-Devel mailing list
OpenBabel-Devel@lists.sourceforge.net

https://lists.sourceforge.net/lists/listinfo/openbabel-devel





--
David van der Spoel, Ph.D., Professor of Biology
Head of Department, Cell & Molecular Biology, Uppsala University.
Box 596, SE-75124 Uppsala, Sweden. Phone: +46184714205.
http://www.icm.uu.se

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
OpenBabel-Devel mailing list
OpenBabel-Devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openbabel-devel

Re: [OpenBabel-Devel] Proposal to require test cases

[Noel O'Boyle]

To avoid digressing, absolutely we would like to do this and have the
technical means to enforce...once we reduce the warnings. I did a certain
amount already this year. Once a particular type of warning is eliminated
we can add it as a requirement using gcc's treat warnings as errors. But
we're not there yet. Like Geoff says, we encourage people to help.

If there's some other way this can be enforced on a patch by patch basis
(e.g. controlling for an increase in warnings), I'd be interested to hear.
Maybe you can point me to the relevant person over at Gromacs.

- Noel

On Tue, 17 Apr 2018, 05:18 David van der Spoel, 
wrote:

> Den 2018-04-16 kl. 22:46, skrev Dominik 'Rathann' Mierzejewski:
> > On Monday, 16 April 2018 at 20:20, David van der Spoel wrote:
> >> Den 2018-04-16 kl. 17:36, skrev David Koes:
> >>> I didn't chime in since I thought it was obviously a good idea.
> >>> However, I strongly agree that the process of creating a test case
> needs
> >>> to be as simple and documented as possible.  I had a test case with my
> >>> last pull request, but it required a fair amount of poking around to
> >>> figure out how to best implement it (and this experience prompted the
> >>> GSoC project).
> >>>
> >>> Also, test cases may not make sense for some pull requests (e.g.
> >>> documentation).
> >>
> >> Agree tests are a must.
> >>
> >> How about making warning-free code a must?
> >
> > Warning-free under which compiler (and version)? GCC adds new warnings
> > in every release.
> Under all compilers. Obviously we can only fix the warnings we are getting.
>
> We have this policy in http://www.gromacs.org and it is enforced
> automatically by the robot that verifies patches. No patches that
> produce warnings on any platform (Linux, Mac, Windows) will be accepted.
>
> >
> > Regards,
> > Dominik
> >
>
>
> --
> David van der Spoel, Ph.D., Professor of Biology
> Head of Department, Cell & Molecular Biology, Uppsala University.
> Box 596, SE-75124 Uppsala, Sweden. Phone: +46184714205.
> http://www.icm.uu.se
>
> 
> --
> Check out the vibrant tech community on one of the world's most
> engaging tech sites, Slashdot.org! http://sdm.link/slashdot
> ___
> OpenBabel-Devel mailing list
> OpenBabel-Devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/openbabel-devel
>
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot___
OpenBabel-Devel mailing list
OpenBabel-Devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openbabel-devel

Re: [OpenBabel-Devel] Proposal to require test cases

[David van der Spoel]

Den 2018-04-16 kl. 22:46, skrev Dominik 'Rathann' Mierzejewski:

On Monday, 16 April 2018 at 20:20, David van der Spoel wrote:

Den 2018-04-16 kl. 17:36, skrev David Koes:

I didn't chime in since I thought it was obviously a good idea.
However, I strongly agree that the process of creating a test case needs
to be as simple and documented as possible.  I had a test case with my
last pull request, but it required a fair amount of poking around to
figure out how to best implement it (and this experience prompted the
GSoC project).

Also, test cases may not make sense for some pull requests (e.g.
documentation).


Agree tests are a must.

How about making warning-free code a must?


Warning-free under which compiler (and version)? GCC adds new warnings
in every release.

Under all compilers. Obviously we can only fix the warnings we are getting.

We have this policy in http://www.gromacs.org and it is enforced 
automatically by the robot that verifies patches. No patches that 
produce warnings on any platform (Linux, Mac, Windows) will be accepted.




Regards,
Dominik




--
David van der Spoel, Ph.D., Professor of Biology
Head of Department, Cell & Molecular Biology, Uppsala University.
Box 596, SE-75124 Uppsala, Sweden. Phone: +46184714205.
http://www.icm.uu.se

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
OpenBabel-Devel mailing list
OpenBabel-Devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openbabel-devel

Re: [OpenBabel-Devel] Proposal to require test cases

[Dominik 'Rathann' Mierzejewski]

On Monday, 16 April 2018 at 20:20, David van der Spoel wrote:
> Den 2018-04-16 kl. 17:36, skrev David Koes:
> > I didn't chime in since I thought it was obviously a good idea.
> > However, I strongly agree that the process of creating a test case needs
> > to be as simple and documented as possible.  I had a test case with my
> > last pull request, but it required a fair amount of poking around to
> > figure out how to best implement it (and this experience prompted the
> > GSoC project).
> > 
> > Also, test cases may not make sense for some pull requests (e.g.
> > documentation).
> 
> Agree tests are a must.
> 
> How about making warning-free code a must?

Warning-free under which compiler (and version)? GCC adds new warnings
in every release.

Regards,
Dominik
-- 
Fedora   https://getfedora.org  |  RPMFusion   http://rpmfusion.org
There should be a science of discontent. People need hard times and
oppression to develop psychic muscles.
-- from "Collected Sayings of Muad'Dib" by the Princess Irulan

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
OpenBabel-Devel mailing list
OpenBabel-Devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openbabel-devel

Re: [OpenBabel-Devel] Proposal to require test cases

[Dominik 'Rathann' Mierzejewski]

On Monday, 16 April 2018 at 21:59, Geoffrey Hutchison wrote:
[...]
> Keep in mind many (most?) of the warnings are from the InChI code
> - which isn't something we want to mess with.

Fixes for warnings in InChI should be sent upstream, to
inchi-disc...@lists.sourceforge.net .

Regards,
Dominik
-- 
Fedora   https://getfedora.org  |  RPMFusion   http://rpmfusion.org
There should be a science of discontent. People need hard times and
oppression to develop psychic muscles.
-- from "Collected Sayings of Muad'Dib" by the Princess Irulan

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
OpenBabel-Devel mailing list
OpenBabel-Devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openbabel-devel

Re: [OpenBabel-Devel] Proposal to require test cases

[Geoffrey Hutchison]

> How about making warning-free code a must?
> Whenever I compile OB I get tons of warnings of potentially serious character.

Patches that reduce/fix warnings are always accepted. :-) Julien Nabet 
(serval2412) has contributed *many* such patches.

Keep in mind many (most?) of the warnings are from the InChI code - which isn't 
something we want to mess with.

-Geoff--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot___
OpenBabel-Devel mailing list
OpenBabel-Devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openbabel-devel

Re: [OpenBabel-Devel] Proposal to require test cases

[David van der Spoel]

Den 2018-04-16 kl. 17:36, skrev David Koes:
I didn't chime in since I thought it was obviously a good idea.  
However, I strongly agree that the process of creating a test case needs 
to be as simple and documented as possible.  I had a test case with my 
last pull request, but it required a fair amount of poking around to 
figure out how to best implement it (and this experience prompted the 
GSoC project).


Also, test cases may not make sense for some pull requests (e.g. 
documentation).


Agree tests are a must.

How about making warning-free code a must?
Whenever I compile OB I get tons of warnings of potentially serious 
character.




David Koes

Assistant Professor
Computational & Systems Biology
University of Pittsburgh

On 04/16/2018 02:31 AM, Noel O'Boyle wrote:
I'm disappointed in the lack of support for this. David Koes - you 
suggested a GSoc on this topic, for example. Anyway, for my part, I 
will write up info on adding testcases, but also start to nudge PR 
submitters towards including testcases.


Regards,
- Noel



On 3 April 2018 at 13:15, Noel O'Boyle > wrote:


    Noted. We will do this. In fact, I commit to doing this whether or 
not this proposal goes ahead.


    - Noel

    On 3 April 2018 at 11:54, David Hall > wrote:


    I mostly agree, but I’ll mention the big hurdle I came across 
the first time I wrote a test for openbabel.


    Many times, one comes across a bug when using the command line 
tools, e.g. obabel , so debugging and testing

    goes through using that program.

    I think (correct if I’m wrong), the easiest way to go from an 
obabel command line run to a test is through
    writing a test in python like those that import functions from 
testbabel


    Having a document we can point to that walks them through that 
process, and shows them that it is quite easy,
    might be helpful. I know my first few times looking at the 
test directory, I said “well, I’m not using the
    python bindings, so I’ll ignore those files and try to figure 
out the c++ files and how they run tests”, when
    the reality is that the python files provide the easy route to 
testing the command line programs.


    -David


 > On Apr 3, 2018, at 4:42 AM, Noel O'Boyle 
> wrote:

 >
 > Hi all,
 >
 > Very few PRs come with test cases. Basically, we just don't 
know if any of them do what they say, and even if
    they do, they probably will bit rot at a future date due to 
other PRs. The irony is that the person who wrote
    the code clearly tested it (one would assume) - but just 
didn't check in the test case. I would argue that the
    majority of developer time spent on Open Babel is on fixing 
bugs (or bitrotted code) which would never have

    existed in the first place if a test had been added.
 >
 > When I refactored the code to handle implict valence, I 
relied on the small number of existing tests to help
    return the code back to its preexisting state. Anything that 
wasn't tested may (still) not be working. For
    example, recently David Koes found that my changes broke the 
PDBQT format.

 >
 > In short, I propose that we require a testcase for every 
PR. There may be special circumstances (huge test
    files, build system changes), but this would be the general 
rule. As a lower bar, one could imagine requiring

    them for every new feature implemented.
 >
 > Regards,
 > - Noel
 > 
-- 

 > Check out the vibrant tech community on one of the world's 
most
 > engaging tech sites, Slashdot.org! 
http://sdm.link/slashdot___

 


 > OpenBabel-Devel mailing list
 > OpenBabel-Devel@lists.sourceforge.net 


 > https://lists.sourceforge.net/lists/listinfo/openbabel-devel

 







-- 


Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! 

Re: [OpenBabel-Devel] Proposal to require test cases

[Noel O'Boyle]

I'm disappointed in the lack of support for this. David Koes - you
suggested a GSoc on this topic, for example. Anyway, for my part, I will
write up info on adding testcases, but also start to nudge PR submitters
towards including testcases.

Regards,
- Noel



On 3 April 2018 at 13:15, Noel O'Boyle  wrote:

> Noted. We will do this. In fact, I commit to doing this whether or not
> this proposal goes ahead.
>
> - Noel
>
> On 3 April 2018 at 11:54, David Hall  wrote:
>
>> I mostly agree, but I’ll mention the big hurdle I came across the first
>> time I wrote a test for openbabel.
>>
>> Many times, one comes across a bug when using the command line tools,
>> e.g. obabel , so debugging and testing goes through using that program.
>>
>> I think (correct if I’m wrong), the easiest way to go from an obabel
>> command line run to a test is through writing a test in python like those
>> that import functions from testbabel
>>
>> Having a document we can point to that walks them through that process,
>> and shows them that it is quite easy, might be helpful. I know my first few
>> times looking at the test directory, I said “well, I’m not using the python
>> bindings, so I’ll ignore those files and try to figure out the c++ files
>> and how they run tests”, when the reality is that the python files provide
>> the easy route to testing the command line programs.
>>
>> -David
>>
>>
>> > On Apr 3, 2018, at 4:42 AM, Noel O'Boyle  wrote:
>> >
>> > Hi all,
>> >
>> > Very few PRs come with test cases. Basically, we just don't know if any
>> of them do what they say, and even if they do, they probably will bit rot
>> at a future date due to other PRs. The irony is that the person who wrote
>> the code clearly tested it (one would assume) - but just didn't check in
>> the test case. I would argue that the majority of developer time spent on
>> Open Babel is on fixing bugs (or bitrotted code) which would never have
>> existed in the first place if a test had been added.
>> >
>> > When I refactored the code to handle implict valence, I relied on the
>> small number of existing tests to help return the code back to its
>> preexisting state. Anything that wasn't tested may (still) not be working.
>> For example, recently David Koes found that my changes broke the PDBQT
>> format.
>> >
>> > In short, I propose that we require a testcase for every PR. There may
>> be special circumstances (huge test files, build system changes), but this
>> would be the general rule. As a lower bar, one could imagine requiring them
>> for every new feature implemented.
>> >
>> > Regards,
>> > - Noel
>> > 
>> --
>> > Check out the vibrant tech community on one of the world's most
>> > engaging tech sites, Slashdot.org! http://sdm.link/slashdot__
>> _
>> > OpenBabel-Devel mailing list
>> > OpenBabel-Devel@lists.sourceforge.net
>> > https://lists.sourceforge.net/lists/listinfo/openbabel-devel
>>
>>
>
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot___
OpenBabel-Devel mailing list
OpenBabel-Devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openbabel-devel

Re: [OpenBabel-Devel] Proposal to require test cases

[Noel O'Boyle]

Noted. We will do this. In fact, I commit to doing this whether or not this
proposal goes ahead.

- Noel

On 3 April 2018 at 11:54, David Hall  wrote:

> I mostly agree, but I’ll mention the big hurdle I came across the first
> time I wrote a test for openbabel.
>
> Many times, one comes across a bug when using the command line tools, e.g.
> obabel , so debugging and testing goes through using that program.
>
> I think (correct if I’m wrong), the easiest way to go from an obabel
> command line run to a test is through writing a test in python like those
> that import functions from testbabel
>
> Having a document we can point to that walks them through that process,
> and shows them that it is quite easy, might be helpful. I know my first few
> times looking at the test directory, I said “well, I’m not using the python
> bindings, so I’ll ignore those files and try to figure out the c++ files
> and how they run tests”, when the reality is that the python files provide
> the easy route to testing the command line programs.
>
> -David
>
>
> > On Apr 3, 2018, at 4:42 AM, Noel O'Boyle  wrote:
> >
> > Hi all,
> >
> > Very few PRs come with test cases. Basically, we just don't know if any
> of them do what they say, and even if they do, they probably will bit rot
> at a future date due to other PRs. The irony is that the person who wrote
> the code clearly tested it (one would assume) - but just didn't check in
> the test case. I would argue that the majority of developer time spent on
> Open Babel is on fixing bugs (or bitrotted code) which would never have
> existed in the first place if a test had been added.
> >
> > When I refactored the code to handle implict valence, I relied on the
> small number of existing tests to help return the code back to its
> preexisting state. Anything that wasn't tested may (still) not be working.
> For example, recently David Koes found that my changes broke the PDBQT
> format.
> >
> > In short, I propose that we require a testcase for every PR. There may
> be special circumstances (huge test files, build system changes), but this
> would be the general rule. As a lower bar, one could imagine requiring them
> for every new feature implemented.
> >
> > Regards,
> > - Noel
> > 
> --
> > Check out the vibrant tech community on one of the world's most
> > engaging tech sites, Slashdot.org! http://sdm.link/slashdot__
> _
> > OpenBabel-Devel mailing list
> > OpenBabel-Devel@lists.sourceforge.net
> > https://lists.sourceforge.net/lists/listinfo/openbabel-devel
>
>
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot___
OpenBabel-Devel mailing list
OpenBabel-Devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openbabel-devel