Bug 522979 - Review Request: lua-lunit - Unit testing framework for Lua
Summary: Review Request: lua-lunit - Unit testing framework for Lua
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Tim Niemueller
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: 522980
TreeView+ depends on / blocked
 
Reported: 2009-09-12 23:12 UTC by Michel Alexandre Salim
Modified: 2009-10-16 19:33 UTC (History)
3 users (show)

Fixed In Version: 0.4-1.el5
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2009-09-29 14:28:50 UTC
tim: fedora-review+
kevin: fedora-cvs+


Attachments (Terms of Use)

Description Michel Alexandre Salim 2009-09-12 23:12:11 UTC
Spec URL: http://salimma.fedorapeople.org/specs/funpl/lua-lunit.spec
SRPM URL: http://salimma.fedorapeople.org/specs/funpl/lua-lunit-0.4-1.fc12.src.rpm
Description:
Lunit is a unit testing framework for lua, written in lua.

Comment 1 Tim Niemueller 2009-09-21 21:47:22 UTC
MUST

* OK: rpmlint
# rpmlint lua-lunit.spec 
0 packages and 1 specfiles checked; 0 errors, 0 warnings.
# rpmlint lua-lunit-0.4-1.fc11.src.rpm
1 packages and 0 specfiles checked; 0 errors, 0 warnings.
# rpmlint lua-lunit-0.4-1.fc11.noarch.rpm
1 packages and 0 specfiles checked; 0 errors, 0 warnings.

* OK: package name
* OK: package version and release
* OK: spec file name
* OP: package guideline-compliant
* OK: license complies with guidelines
* OK: license field accurate
* OK: license file not deleted
* OK: spec in US English
* OK: spec legible
* OK: source matches upstream
  sha256sum 09efe9f35132353c6810c57367cba29659afc7348ff593c529cbee1831d66d7a
* OK: builds under >= 1 archs, others excluded
* OK: dependencies (requires)
* FAIL: build dependencies complete
  lua >= %{luaver} should be in BR for %check to work. The lua package actually is pulled in (cf. root.log from scratch build below), but this is only intermediate, should be included directly as BR.
* N/A: locales handled using %find_lang, no %{_datadir}/locale
* N/A: library -> ldconfig
* N/A: relocatable: give reason
* N/A: own all directories
* OK: no dupes in %files
* OK: permission
* OK: %clean RPM_BUILD_ROOT
* FAIL: macros used consistently
  Most of the time you use %{dir} macros, but then you use $RPM_BUILD_ROOT,
  should be %{buildroot}
* OK: Package contains code
* N/A: large docs => -doc
* OK: doc not runtime dependent
* N/A: headers in -devel
* N/A: static in -static
* N/A: if contains *.pc, req pkgconfig
* N/A: if libfiles are suffixed, the non-suffixed goes to devel
* N/A: devel requires versioned base package
* N/A: desktop file uses desktop-file-install
* OK: clean buildroot before install
* OK: filenames UTF-8

SHOULD
* OK: if license text missing, ask upstream to include it
* N/A: desc and summary contain translations if available
* OK: package builds in mock on all architectures
  https://koji.fedoraproject.org/koji/taskinfo?taskID=1695926
* OK: package functions as described
# lunit /usr/share/doc/lua-lunit-0.4/example.lua 
Loaded testsuite with 4 tests in 2 testcases.

    F...

6 Assertions checked.

  1) Failure (simple.test_failure):
/usr/share/doc/lua-lunit-0.4/example.lua:13: true expected but was false
/usr/share/doc/lua-lunit-0.4/example.lua:13: This test always fails!

Testsuite finished (3 passed, 1 failed, 0 errors).

* OK: scriplets are sane
  If %check fails, you should cat testlog.txt such that the user knows what
  happened.
* N/A: other subpackages should require versioned base
* N/A: if main pkg is development-wise, pkgconfig can go in main package
* N/A: require package not files

Question:
Is the koji scratch build enough to assert "package builds in mock on all architectures"?

Preventing approval:
- BR lua missing
- Inconsistent macro usage (I know that's debatable, but the guidelines state that you should stick to one type or the other)

Both are only minor, but for the first review I must be pedantic, right ;-)

Comment 2 Michel Alexandre Salim 2009-09-21 22:38:10 UTC
(In reply to comment #1)
> * FAIL: build dependencies complete
>   lua >= %{luaver} should be in BR for %check to work. The lua package actually
> is pulled in (cf. root.log from scratch build below), but this is only
> intermediate, should be included directly as BR.
That's a good suggestion. lua is pulled in right now because rpm-libs depend on it, but making this explicit would be a good idea.

> * FAIL: macros used consistently
>   Most of the time you use %{dir} macros, but then you use $RPM_BUILD_ROOT,
>   should be %{buildroot}
Non-issue, as in lua-json, but again, I can change this if you insist.

 > Question:
> Is the koji scratch build enough to assert "package builds in mock on all
> architectures"?
Enough, yes. Well, all primary architectures: Koji builds on %{ix86}, x86_64, ppc and ppc64. Sometimes the package has to be modified later because the ARM and SPARC porting projects report errors, but that cannot be done during review.

> Preventing approval:
> - BR lua missing
> - Inconsistent macro usage (I know that's debatable, but the guidelines state
> that you should stick to one type or the other)
One style or another for buildroot, I think. That's my interpretation anyway :)

SRPM updated, at the same location.

Comment 3 Tim Niemueller 2009-09-21 22:58:26 UTC
Looks good to me now, I agree on your buildroot guideline interpretation.

APPROVED

Comment 4 Michel Alexandre Salim 2009-09-23 11:45:49 UTC
Thanks, Tim!

New Package CVS Request
=======================
Package Name: lua-lunit 
Short Description: Unit testing framework for Lua
Owners: salimma
Branches: EL-5 F-10 F-11
InitialCC:

Comment 5 Kevin Fenzi 2009-09-24 04:36:47 UTC
cvs done.

Comment 6 Fedora Update System 2009-09-25 03:56:57 UTC
lua-lunit-0.4-1.fc10 has been submitted as an update for Fedora 10.
http://admin.fedoraproject.org/updates/lua-lunit-0.4-1.fc10

Comment 7 Fedora Update System 2009-09-25 03:57:03 UTC
lua-lunit-0.4-1.fc11 has been submitted as an update for Fedora 11.
http://admin.fedoraproject.org/updates/lua-lunit-0.4-1.fc11

Comment 8 Fedora Update System 2009-09-25 03:57:08 UTC
lua-lunit-0.4-1.el5 has been submitted as an update for Fedora EPEL 5.
http://admin.fedoraproject.org/updates/lua-lunit-0.4-1.el5

Comment 9 Fedora Update System 2009-09-26 01:28:00 UTC
lua-lunit-0.4-1.el5 has been pushed to the Fedora EPEL 5 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update lua-lunit'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/EL-5/FEDORA-EPEL-2009-0515

Comment 10 Fedora Update System 2009-09-29 14:28:45 UTC
lua-lunit-0.4-1.fc10 has been pushed to the Fedora 10 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 11 Fedora Update System 2009-09-29 14:32:40 UTC
lua-lunit-0.4-1.fc11 has been pushed to the Fedora 11 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 12 Fedora Update System 2009-10-16 19:33:32 UTC
lua-lunit-0.4-1.el5 has been pushed to the Fedora EPEL 5 stable repository.  If problems still persist, please make note of it in this bug report.


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