Bug 522979

Summary: Review Request: lua-lunit - Unit testing framework for Lua
Product: [Fedora] Fedora Reporter: Michel Lind <michel>
Component: Package ReviewAssignee: Tim Niemueller <tim>
Status: CLOSED ERRATA QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: fedora-package-review, notting, tim
Target Milestone: ---Flags: tim: fedora-review+
kevin: fedora-cvs+
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: 0.4-1.el5 Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2009-09-29 14:28:50 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Bug Depends On:    
Bug Blocks: 522980    

Description Michel Lind 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 Lind 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 Lind 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.