Bug 481272 - Review Request: python-webunit - it test your websites with code that acts like a web browser [NEEDINFO]
Review Request: python-webunit - it test your websites with code that acts l...
Status: ASSIGNED
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Ondrej Hudlicky
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2009-01-23 06:17 EST by Petr Sklenar
Modified: 2015-09-24 11:04 EDT (History)
5 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed:
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
mnowak: fedora‑review?
mnowak: needinfo? (psklenar)
i: needinfo? (psklenar)
i: needinfo? (psklenar)
i: needinfo? (psklenar)


Attachments (Terms of Use)

  None (edit)
Description Petr Sklenar 2009-01-23 06:17:28 EST
Spec URL: http://people.redhat.com/psklenar/python-webunit.spec
SRPM URL: http://people.redhat.com/psklenar/python-webunit-1.3.8-1.fc10.src.rpm
Description:
Unit test your websites with code that acts like a web browser.

Features in a nutshell:

   1. Browser-like page fetching including fetching the images and stylesheets
      needed for a page and following redirects
   2. Cookies stored and trackable (all automatically handled)
   3. HTTP, HTTPS, GET, POST, basic auth all handled, control over expected
      status codes, ...
   4. DOM parsing of pages to retrieve and analyse structure, including simple
      form re-posting
   5. Two-line page-fetch followed by form-submit possible, with error checking
   6. Ability to register error page content across multiple tests
   7. Uses python's standard unittest module as the underlying framework
Comment 1 Petr Sklenar 2009-01-23 06:19:28 EST
Hello,
this is my first package, and you I am seeking a sponsor
Comment 2 Tadej Janež 2009-01-25 12:03:50 EST
This is my informal review. I cannot sponsor you as I'm not (yet) an approved
packager. However, my review can help your future sponsor when he makes his
own official review.

Good:
+ rpmlint output is clean.
+ The package is named according to the Package Naming Guidelines.
+ The spec file name matches the base package %{name}, in the format %{name}.spec.
+ The package is licensed with a Fedora approved license and meets the Licensing Guidelines.
+ The License field in the package spec file matches the actual license.
+ The spec file is written in American English.
+ The spec file for the package is legible.
+ The sources used to build the package match the upstream source, as provided in the spec URL (md5sum webunit-1.3.8.tar.gz: 97b9e6b5149dadce48b86adbf2db3b0a).
+ The package successfully compiles and builds into binary rpms on at least one primary architecture.
+ All build dependencies are listed in BuildRequires, except for any that are listed in the exceptions section of the Packaging Guidelines.
+ The package owns all directories that it creates.
+ The package doesn't contain any duplicate files in the %files listing.
+ Permissions on files are set properly.
+ The package has a %clean section, which contains rm -rf %{buildroot} (or $RPM_BUILD_ROOT).
+ The package consistently uses macros.
+ The package contains code, or permissible content.
+ Package doesn't own files or directories already owned by other packages.
+ At the beginning of %install, package runs rm -rf %{buildroot} (or $RPM_BUILD_ROOT).
+ All filenames in rpm packages are valid UTF-8.
+ I tested that the package builds in mock.
+ The package compiles and builds into binary rpms on all supported architectures.
+ I did a limited test that the package functions as described.
+ egg-info files which are generated by the module's build scripts are included in the package.

Comments:
- You should use %{python_sitelib} instead of %{python_sitearch}, because your package is architecture independent (noarch)
- You should remove %{python_sitearch} macro definition as you don't need it.
- You should remove CFLAGS=... in %build section because this a noarch package.
- You should avoid putting demo folder in the top-level %{python_sitelib} folder and rather put in a subfolder like %{python_sitelib}/%{Project_name}.
- You could shorten the %files section by using:
%{python_sitearch}/%{Project_name}/
instead of:
%{python_sitearch}/%{Project_name}/HTMLParser.py*
%{python_sitearch}/%{Project_name}/IMGSucker.py*
%{python_sitearch}/%{Project_name}/SimpleDOM.py*
%{python_sitearch}/%{Project_name}/__init__.py*
%{python_sitearch}/%{Project_name}/config.py*
%{python_sitearch}/%{Project_name}/cookie.py*
%{python_sitearch}/%{Project_name}/utility.py*
%{python_sitearch}/%{Project_name}/webunittest.py*
And similarly for the demo directory.
- The version in the %changelog section should be on the same line as the date and your name.
- Because the source package does not include license text(s) as a separate file from upstream, you should query upstream to include it.
- You should improve the Summary and Description fields. Look at other packages for examples. Your summary should state something like "A python module for unit testing websites acting like a web browser".
- Optionally, you could shorten and rename %{Project_name} macro to something like %{project} or %{module}.
Comment 3 Petr Sklenar 2009-01-26 08:54:24 EST
hi,
thank much for review of specfile. I changed it up your comments, except licence: there is MIT-licence in /usr/share/doc/python-webunit-1.3.8/README.txt
Is it enough for licence ?

Spec URL: http://people.redhat.com/psklenar/python-webunit.spec
SRPM URL: http://people.redhat.com/psklenar/python-webunit-1.3.8-2.fc10.src.rpm
Comment 4 Tadej Janež 2009-01-26 10:03:14 EST
Ok, looks good to me, approved.

Now you have to wait for someone, who can sponsor you and give you an official review and approval.

As for the license text, the package review guidelines (https://fedoraproject.org/wiki/Packaging/ReviewGuidelines) states that:
"If the source package does not include license text(s) as a separate file from upstream, the packager SHOULD query upstream to include it."
So, the license should be in a separate LICENSE file instead of being at the end of README.txt file. You are not required to do this, however, when you have time, contact upstream about it.
Comment 5 Richard Jones 2009-01-27 05:02:58 EST
The license text may be found at the end of the README file. If you need it to 
be in a separate file I grant you permission to copy it there.
Comment 6 Richard Jones 2009-01-27 05:03:34 EST
(In reply to comment #5)
> The license text may be found at the end of the README file. If you need it to 
> be in a separate file I grant you permission to copy it there.

ps. I am the author of webunit
Comment 7 Petr Sklenar 2009-01-29 09:56:45 EST
Hi,
I made informal review at https://bugzilla.redhat.com/show_bug.cgi?id=480860#c4
Comment 8 Tadej Janež 2009-02-02 10:42:57 EST
(In reply to comment #5)
> The license text may be found at the end of the README file. If you need it to 
> be in a separate file I grant you permission to copy it there.

Fedora project would like to encourage you as the author of webunit to move the license text from the end of the README file to a seperate file (e.g. COPYING). You should make that change upstream (in the provided source at http://www.mechanicalcat.net/tech/webunit/webunit-%{version}.tar.gz).
Comment 9 Michal Nowak 2009-03-09 13:10:18 EDT
Will do the rest of the review, should be easy thanks to Tadej Janež. 

Task: Move the license from README to COPYING (e.g. via ``install'' in post-install section.) and publish new SPEC and SRPM pkg.

Note: I removed the FE-NEEDSPONSOR since you are sponsored packager now.
Comment 11 Christopher Meng 2013-07-20 00:19:57 EDT
NEWS?

Stalled?
Comment 12 Christopher Meng 2013-11-19 05:23:18 EST
ping after months.

If still no response in another month, will close it and submit by myself.
Comment 13 Upstream Release Monitoring 2015-09-24 11:04:55 EDT
jgrulich's scratch build of kdevelop?#c8e2b9bc57f11e41f3dc6612cdbcc591078d9062 for f22-candidate and git://pkgs.fedoraproject.org/kdevelop?#c8e2b9bc57f11e41f3dc6612cdbcc591078d9062 completed http://koji.fedoraproject.org/koji/taskinfo?taskID=11212117

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