Note: This is a beta release of Red Hat Bugzilla 5.0. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Also email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback here.
Bug 165985 - Review Request: Aeryn - A C++ testing framework
Summary: Review Request: Aeryn - A C++ testing framework
Keywords:
Status: CLOSED NOTABUG
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: John Mahowald
QA Contact: David Lawrence
URL: http://www.paulgrenyer.dyndns.org/aeryn/
Whiteboard:
Depends On:
Blocks: FE-DEADREVIEW
TreeView+ depends on / blocked
 
Reported: 2005-08-15 14:40 UTC by Paul F. Johnson
Modified: 2007-11-30 22:11 UTC (History)
1 user (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2006-10-12 16:18:01 UTC


Attachments (Terms of Use)
spec patch (deleted)
2005-12-16 21:39 UTC, John Mahowald
no flags Details | Diff

Description Paul F. Johnson 2005-08-15 14:40:01 UTC
Description: Aeryn is a C++ testing framework. Although it is primarily intended for unit testing, Aeryn is adaptable enough to handle integration testing and can be adapted for most other forms of C++ testing.

Aeryn is intended to be light weight with the minimal of code needed to create a test fixture. Unlike other testing frameworks Aeryn does not require all test fixtures to be inherited from a particular class. Test fixtures can be standalone functions or standalone classes.

Aeryn is adaptable via context objects that can be passed to test fixtures prior to running and through its call back reporting interface.

Comment 1 Ville Skyttä 2005-08-15 14:53:10 UTC
specfile/SRPM URL missing. 

Comment 2 Paul F. Johnson 2005-08-15 16:48:00 UTC
http://www.all-the-johnsons.co.uk/aeryn/aeryn2.spec for the spec, the source
archive is there also. I'm having a problem with the install though (it's the
first time I've done this!)

Comment 3 Paul F. Johnson 2005-08-20 21:16:09 UTC
The spec file has now been updated and the RPM builds cleanly. Can you please
review it for inclusion?

http://www.all-the-johnsons.co.uk/aeryn/aeryn2.spec

The RPMs are also there

http://www.all-the-johnsons.co.uk/aeryn2/aeryn2-2-1.1.i386.rpm
http://www.all-the-johnsons.co.uk/aeryn2/aeryn2-debuginfo-2-1.1.i386.rpm

Comment 4 Paul F. Johnson 2005-08-20 21:17:43 UTC
src.rpm also available from
http://www.all-the-johnsons.co.uk/aeryn2/aeryn2-2-1.1.src.rpm

Comment 5 Michael Schwendt 2005-08-20 23:55:29 UTC
Okay, let's start somewhere. A first rather brief look:

Package builds as "aeryn2-2-1.1.i386.rpm" although the upstream version
is "2.1.1", so why not name the package "aeryn" with version "2.1.1"
and release "1"?

> $ rpmls aeryn2 | grep ^d
> drwxr-xr-x  /usr/include/aeryn/aeryn
> drwxr-xr-x  /usr/include/aeryn/aeryn/details

This reveals that the directory

  /usr/include/aeryn/

is not included in the package. A look at the complete package listing
reveals that

  /usr/lib/aeryn/

is not included either.

Further, /usr/bin/TestClient is a rather generic name. It doesn't look
like a useful binary that should be included, as it only runs a test suite
which most likely ought to be ran during rpmbuild in %check or so instead.
It looks like it returns a proper exit code for "failed/successful" state.

> $ rpm -qd aeryn2
> $

No documentation included at all?

[...]

Now to the spec file (I usually start a review at the spec level, but moved
it here):

> Summary: Aeryn2 is a C++ unit test package

Better IMO  =>  Summary: C++ unit testing framework

> Name: aeryn2
> Version: 2
> Release: 1.1%{?dist}

As mentioned above, I believe this should be "Name: aeryn", "Version: 2.1.1"
and "Release: 1%{?dist}".

> Source0: http://www.all-the-johnsons.co.uk/aeryn/aeryn2.tar.bz2

This was a surprise, since the Source0 URL should point to the upstream
projects download location. It seems you repackaged upstream author's ZIP
archive and included the PDF documentation file in the tarball. Why not
just take his ZIP as "Source0" and put the PDF into "Source1"? Sort of:

%prep
%setup -T -c -n %{name}
unzip %{SOURCE0}

and everything you did to build your tar.bz2 should be done after
extracting the zip.

> $ rpmlint aeryn2-2-1.1.src.rpm 
> E: aeryn2 description-line-too-long

Keep %description lines below 80 characters.

> %build
> make %{?_smp_mflags}

Code compiles without $RPM_OPT_FLAGS, e.g.

> g++ -W -Wall -Werror -pedantic -Wcast-qual -Wcast-align -Wwrite-strings
> -Winline -finline-limit=1048576 -g3 -c -o noncopyable.o
> ../src/noncopyable.cpp -I../include -DNO_OUTPUT_OPERATOR_DETECTION

It may be necessary to patch ./make/common.mk and adjust CXXFLAGS there.

> %clean
> [ "x${RPM_BUILD_ROOT}" != "x/" ] && rm -rf "${RPM_BUILD_ROOT}"

Just "rm -rf $RPM_BUILD_ROOT" is enough and more readable. You do define
a default buildroot. Rare users, who rpmbuild as superuser and redefine
the buildroot play with fire anyway.

> %files
> %defattr(-, root, root)
> %{_bindir}/*
> %{_includedir}/aeryn/*
> %{_libdir}/aeryn/*

Instead, these two

%{_includedir}/aeryn/
%{_libdir}/aeryn/

would also include the two directories and not just their contents.


Comment 6 Paul F. Johnson 2005-08-21 13:29:12 UTC
The package is actually called Aeryn2 with a version number of 2.1.1 hense the name!

Documentation is a couple of PDFs - I can convert them to something sane, so
that's not an issue and I'll change the TestClient executable to be performed as
%check. I'll also make the changes so that it's not /usr/include/aeryn/aeryn but
/usr/include/aeryn.

No problems with the others. Thanks for the pointers and I'll get a newer
version up at some point today. Other things are pressing though :-(

Comment 7 Michael Schwendt 2005-09-08 16:41:19 UTC
> The package is actually called Aeryn2 with a version number
> of 2.1.1 hense the name!

Okay, then let me prove the opposite:

$ ls aeryn2-2-1.1.src.rpm 
aeryn2-2-1.1.src.rpm

$ grep 'Ver\|Nam\|Rel' aeryn2.spec 
Name: aeryn2
Version: 2
Release: 1.1%{?dist}

Comment 8 John Mahowald 2005-12-16 21:39:02 UTC
Created attachment 122353 [details]
spec patch

Makes most of the recommended changes in comment 5.

%changelog
* Fri Dec 16 2005 John Mahowald <jpmahowald@gmail.com> - 2.1.1-2
- Rename to aeryn
- Add devel package
- Copy files to build root
- ldconfig
- Use upstream sources

Comment 9 John Mahowald 2006-10-03 01:14:59 UTC
Any progress on this? If not, will close in a week as per
http://fedoraproject.org/wiki/Extras/Policy/StalledReviews

Comment 10 John Mahowald 2006-10-12 16:18:01 UTC
Closing. Reopen to resume work on this.


Note You need to log in before you can comment on or make changes to this bug.