[PATCH 4/4] added autotest support

[William Hubbs]

Hi Andrei,

I feel like there are several changes in this patch that are not
documented in the commit message, and some of them should be separate
commits because they are not part of the autotest support.

Also, I'm concerned about the choice of licensing that you used for some
of the test files.  Since everything in the tree is gpl 2 or later, I
think we should stick with that.

On Sun, Sep 12, 2010 at 12:43:33PM +0200, Andrei Kholodnyi wrote:
> diff --git a/configure.ac b/configure.ac
> index 8ad25fa..f4812f1 100644
> --- a/configure.ac
> +++ b/configure.ac
> +AC_CONFIG_FILES([src/tests/Makefile])

> @@ -417,24 +421,25 @@ AC_SUBST(RDYNAMIC)
>  localedir=${datadir}/locale
>  AC_SUBST(localedir)
>  
> -AC_OUTPUT([Makefile
> -           speech-dispatcher.pc
> -           config/Makefile
> -           config/modules/Makefile
> -           config/clients/Makefile
> -           doc/Makefile
> -           po/Makefile.in
> -           src/Makefile
> -           src/server/Makefile
> -           src/modules/Makefile
> -           src/c/Makefile
> -           src/c/api/Makefile
> -           src/c/clients/Makefile
> -           src/c/clients/say/Makefile
> -           src/c/clients/spdsend/Makefile
> -           src/tests/Makefile
> -           src/audio/Makefile
> -           src/audio/static_plugins.c
> -           src/python/Makefile
> -           src/python/speechd/Makefile
> -           src/python/speechd_config/Makefile])
> +AC_CONFIG_FILES([Makefile
> +                 speech-dispatcher.pc
> +                 config/Makefile
> +                 config/modules/Makefile
> +                 config/clients/Makefile
> +                 doc/Makefile
> +                 po/Makefile.in
> +                 src/Makefile
> +                 src/server/Makefile
> +                 src/modules/Makefile
> +                 src/c/Makefile
> +                 src/c/api/Makefile
> +                 src/c/clients/Makefile
> +                 src/c/clients/say/Makefile
> +                 src/c/clients/spdsend/Makefile
> +                 src/audio/Makefile
> +                 src/audio/static_plugins.c
> +                 src/python/Makefile
> +                 src/python/speechd/Makefile
> +                 src/python/speechd_config/Makefile])
> +                 
> +AC_OUTPUT

Converting from the old AC_OUTPUT usage to AC_CONFIG_FILES/AC_OUTPUT
should be a separate patch; this is not part of adding autotest support.

Also, AC_CONFIG_FILES should be called only once, at the end of
configure.ac, just above AC_OUTPUT.

> diff --git a/src/tests/Makefile.am b/src/tests/Makefile.am
> index 6e12c6f..9eeb73d 100644
> --- a/src/tests/Makefile.am
> +++ b/src/tests/Makefile.am
> @@ -1,3 +1,22 @@
> +# Makefile for tests
> +
> +# Copyright (C) 2010 Brailcom, o.p.s.
> +#
> +# This program is free software: you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation, either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.

What does adding this header have to do with autotest support?  Also, I
am concerned about you using gpl 3 for this.  Most of the tree is gpl 2
or later, so I think we should stick with that.


> +DISTCLEANFILES = atconfig $(TESTSUITE)
> +MAINTAINERCLEANFILES = Makefile.in

What does adding the MAINTAINERCLEANFILES line have to do with autotest
support?

> diff --git a/src/tests/c_api.at b/src/tests/c_api.at
> new file mode 100644
> index 0000000..baebead
> --- /dev/null
> +++ b/src/tests/c_api.at
> @@ -0,0 +1,26 @@
> +# Copyright (C) 2010 Andrei Kholodnyi <andrei.kholodnyi at gmail.com>
> +#
> +# This program is free software: you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation, either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.

Shouldn't this be gpl 2 or later to match the rest of the tree?

> diff --git a/src/tests/ssip.at b/src/tests/ssip.at
> new file mode 100644
> index 0000000..fe3e2de
> --- /dev/null
> +++ b/src/tests/ssip.at
> @@ -0,0 +1,26 @@
> +# Copyright (C) 2010 Andrei Kholodnyi <andrei.kholodnyi at gmail.com>
> +#
> +# This program is free software: you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation, either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.

Again, shouldn't this be version 2 or later?

> +AT_KEYWORDS([punctuation])
> +AT_CHECK([/home/akholodn/wrgit/speechd/src/tests/run_test 
> /home/akholodn/wrgit/speechd/src/tests/punctuation.test])

Shouldn't "/home/akholodn/wrgit/speechd" be $(abs_builddir)?

> diff --git a/src/tests/testsuite.at b/src/tests/testsuite.at
> new file mode 100644
> index 0000000..cb9890c
> --- /dev/null
> +++ b/src/tests/testsuite.at
> @@ -0,0 +1,20 @@
> +# Copyright (C) 2010 Andrei Kholodnyi <andrei.kholodnyi at gmail.com>
> +#
> +# This program is free software: you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation, either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.

Same as above about the license, shouldn't we stick with version 2 or
later?

Thanks,

William

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: not available
URL: 
<http://lists.freebsoft.org/pipermail/speechd/attachments/20100912/832cc23b/attachment.pgp>