licensecheck: improvements

[Benjamin Drung]

Hi Dmitry,

sorry for the long delay. I was at UDS and busy afterwards.

Am Mittwoch, den 24.10.2012, 10:20 +1100 schrieb Dmitry Smirnov:
> > What do the other maintainer think in this regard? If other maintainers
> > think that the --help and the man page content should be merged, I will
> > not block the patch.
> 
> We can keep --help as a dedicated section 'HELP' in Pod. This way we will have 
> consistent formatting for all documentation that will be in one place. However 
> duplication between OPTIONS and HELP will be even more apparent.

Exactly.

> > > No worries, although IMHO it is unnecessary to print license on --version
> > > I don't mind doing it as long as we don't duplicate the code for that
> > > reason.
> > > 
> > > Please have a look how it is implemented in the attached files, I hope
> > > you'll like it much better. --version still prints license and copyright
> > > statements but it take 'em from embedded POD sections. Much more elegant
> > > IMO.
> > 
> > I prefer the style of other GNU tools. The first line contains the
> > version (easier to parse):
> > 
> > licensecheck (devscripts) ###version###
> > 
> > The following lines can then contain the copyright and license information.
> > 
> 
> That's easy to fix by reordering sections in POD, we just need to put VERSION 
> before LICENSE.

Even then the first line will state only "Version:".

> > > I have many reasons to stick to embedded tests for now. For example
> > > because I introduce tests first, some of them will be failing by
> > > definition until viable fix will be implemented. I'm going to introduce
> > > new test with the new reported bugs so some tests will be failing until
> > > we agree on viable solution for the bug fix. This way tests will be also
> > > a road map for ongoing work.
> > 
> > This approach is unrelated to the used test framework.
> > 
> > > I don't want to interfere with shunit2 for those tests.
> > > 
> > > Also it is much easier and quicker to invoke "./licensecheck.pl --tests"
> > > rather than require source package to be able to run tests.
> > 
> 
> My argument is that we probably don't want failed tests in shunit2 as it may 
> give false impression that something is wrong.

The shunit2 test should be submitted/committed once they pass. You can
keep the failing ones locally until a fix is developed.

> > Invoking "test/test_licensecheck" is as easy and quick as invoking
> > "./licensecheck.pl --tests". You can also run licensecheck manually on a
> > license file: "scripts/licensecheck.pl test/licensecheck/gpl-3.sh"
> 
> This is only true if you work on source package and have prior knowledge of 
> how to run shunit2 tests.
> I want to have embedded tests so people who send patches to licensecheck could 
> test them on the spot. I want tests to be separate from packaging so 
> licensecheck could be tested without devscripts source package.

There is a difference between packaging and source package. You need the
source package to run the test, but you do not have to build a package
for it.

> After all why not bundle tests if it's not affecting performance?

I don't like having the test in the user installed system. It makes the
package bigger.

> > > I would like to hear a better argument for "shunit2" rather than that you
> > > prefer it without explaining why.
> > 
> > shunit2 is not bound to a specific language. I could rewrite
> > licensecheck in another language and still use the test cases.
> 
> IMHO language-agnotsic argument is irrelevant even *if* we were going to 
> rewrite using different language.

Why? I once rewrote distro-info in another language. Having a
language-agnotsic test suite was a benefit. I didn't had to rewrite the
tests and I was able to make sure that the user didn't noticed a
difference.

> > shunit2
> > tests the user interface (= command line) instead of only internal Perl
> > functions.
> 
> Is it really a benefit for us? 
> Test::More allows to test everything you want including command line interface 
> although I believe testing this interface is not a priority to say the least.
> 
> shunit2 is not encouraging to write testable code. There is a difference 
> between testable code and testable application.
> With native test framework you can define interface to the function for the  
> benefit of being able to isolate tests for the function specifically and 
> therefore know exactly where the failure occurs. 
> 
> I can't do such tests with shunit2. It would be nice to have tests for 
> stripping comments, formatting etc. In that regards shunit2 is nothing but 
> obstacle for me.

I have nothing against having additional Perl tests that test internal
stuff of licensecheck. These test should be more specific than what
shunit2 tests.

> > The test are easily manually reproducible by just running the
> > command from the shunit2 test manually.
> 
> As I mentioned above this is only true for testing application as a whole and 
> only if you have tests as a part of source packaging.

Agreed.

PS: We disagree on some points, but I do not want to discourage your
work on licensecheck. There are other things that I would love to apply
a patch (for example, using DEP-5 license names or creating a DEP-5
copyright template).

-- 
Benjamin Drung
Debian & Ubuntu Developer