[Spice-devel] [vdagent-win PATCH 3/3] Add a test for logging functions

Christophe Fergeau cfergeau at redhat.com
Wed Jul 26 11:06:39 UTC 2017


On Wed, Jul 26, 2017 at 07:01:05AM -0400, Frediano Ziglio wrote:
> > 
> > On Wed, Jul 26, 2017 at 11:03:42AM +0100, Frediano Ziglio wrote:
> > > Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
> > > ---
> > >  Makefile.am         | 13 +++++++++++++
> > >  common/test-log.cpp | 53
> > >  +++++++++++++++++++++++++++++++++++++++++++++++++++++
> > >  test-log            |  3 +++
> > >  3 files changed, 69 insertions(+)
> > >  create mode 100644 common/test-log.cpp
> > >  create mode 100755 test-log
> > > 
> > > Unfortunately this test fails compiled with Fedora 25/26
> > > (with default options) but the problem is a compiler error,
> > > see https://bugzilla.redhat.com/show_bug.cgi?id=1475237.
> > > I honestly prefer to have a test that fails instead
> > > of distributing a possible buggy executable.
> > 
> > This part can be removed after your patch removing -flto.
> > 
> 
> is after the --- line so would not be in the commit message
> anyway, I wrote these before the -flto issue.
> 
> > > 
> > > diff --git a/Makefile.am b/Makefile.am
> > > index 411bf0d..40db903 100644
> > > --- a/Makefile.am
> > > +++ b/Makefile.am
> > > @@ -88,6 +88,19 @@ imagetest_SOURCES =			\
> > >  TESTS = test-png
> > >  EXTRA_DIST += test-png
> > >  
> > > +check_PROGRAMS += test-log-win
> > > +TESTS += test-log
> > > +EXTRA_DIST += test-log
> > > +
> > > +test_log_win_LDFLAGS = $(AM_LDFLAGS) -Wl,--subsystem,console
> > > +test_log_win_SOURCES =			\
> > > +	common/vdcommon.cpp             \
> > > +	common/vdcommon.h		\
> > > +	common/vdlog.cpp		\
> > > +	common/vdlog.h			\
> > > +	common/test-log.cpp		\
> > > +	$(NULL)
> > > +
> > >  deps.txt:
> > >  	$(AM_V_GEN)rpm -qa | grep $(host_os) | sort | unix2dos > $@
> > >  
> > > diff --git a/common/test-log.cpp b/common/test-log.cpp
> > > new file mode 100644
> > > index 0000000..71b3de2
> > > --- /dev/null
> > > +++ b/common/test-log.cpp
> > > @@ -0,0 +1,53 @@
> > > +/*
> > > +   Copyright (C) 2017 Red Hat, Inc.
> > > +
> > > +   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 2 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/>.
> > > +*/
> > > +#undef NDEBUG
> > > +#include <assert.h>
> > > +#include <iostream>
> > > +#include <fstream>
> > > +#include <string>
> > > +#include "vdlog.h"
> > > +
> > > +using namespace std;
> > > +
> > > +#define TEST_FILE "test.log"
> > > +
> > > +int main(int argc, char **argv)
> > > +{
> > > +    unlink(TEST_FILE);
> > 
> > I would not delete random files from random locations on the user's
> > computer... Just fail the test if you cannot create your temp file.
> > 
> 
> The problem is that I need to erase the file as the
> log appends lines to it and I need to check it after has been
> written. Maybe a more unique name would help.
> Kind of "test-log-output.log" ? ("test-log.log" is used by
> make check).

It's erased at the end of the test suite, so in most cases everything
will be fine. You can use mkstemp or similar if you need a very unique
name, or is it not available on Windows?

Christophe
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20170726/ecab5c90/attachment-0001.sig>


More information about the Spice-devel mailing list