Document instructions on how to run the tests

[jan-matejka]

Also

1. it would be nice to use one of the common method's like python setup.py test or nosetests.
2. and do not use python3 directly. I'm not sure how it works on other distros but in gentoo we run just python which will then execute the proper python-X.Y

[ymattw]

1. You should know people hates to write unit test for code with simple logic :)
2. I am using python3 explicitly in Makefile because I want test with both python2 and python3. I might need to add python2 explicitly too

I do not document how to test as I thought only myself care that, can I ask why you expect a document for how to test, any special reason?

[jan-matejka]

1. Dont care. If the tests exists I'm gonna execute them.
2. I'm just saying the python versions shouldn't be hardcoded. I might send you a pull request some time later on this one
3. It just should be documented. If I install your software and see it have tests, I run them to check it installed allright on my system. In order to run your tests myself I know have to go through the code and figure out how is it supposted to work instead of just reading the correct way on how to execute the tests, written by upstream.

I suspect you just run make test and if the return code is 0 you assume, the tests passed. You also just test that the code doesnt crash, right?

[ymattw]

I just make test before commit, I don't have test for post install check. You are right the test is manually verified with my eyes, not easy to do that programically as it is an interact tool. Pull request welcome.

[jan-matejka]

https://github.com/ymattw/cdiff/blob/master/Makefile#L21 This part of tests is to verify this https://github.com/ymattw/cdiff/blob/master/cdiff.py#L619 right?

[jan-matejka]

So, how does this look like?

[ymattw]

That's right.

On Tuesday, February 12, 2013, yaccz wrote:

https://github.com/ymattw/cdiff/blob/master/Makefile#L21 This part of tests is to verify this https://github.com/ymattw/cdiff/blob/master/cdiff.py#L619 right?

— Reply to this email directly or view it on GitHubhttps://github.com/ymattw/cdiff/issues/7#issuecomment-13428768.

[ymattw]

Thanks, Actually I did the same to project colordiff. I was considering use 'script' to force dump ansi color chars, as I am not sure when --color can be used except for test. I will look into your changes when back to a physical keyboard.

[jan-matejka]

I'm not sure either but could be helpfull in some special cases (use or environments). I made this option after the one in grep.

[ymattw]

Option --color is fine, it gives the possibility to use a pager other than less. I have several consideration on the changes on tests.

• Maintaining the expected output file would be a problem, if I changed output format, I have to dump new format and use it as expected output, the extra effort does not bring us more value because it still depends on human eyes at the beginning
• Introducing shunit2 is too much for cdiff right now, I even prefer to implement our own assertion and main proc if an acceptance tests is really required for this simple tool
• -w N tests are missing, execution bit of tests/acceptance/tests.sh is missing too

In summary, my friend, I don't want to make cdiff becomes any big or complex, I am fine to take 4f29fc8 (send pull request please), others are ok too if we can keep them simple.

[jan-matejka]

well

1. Every tests depened on human verification at first. The point is you can make further changes, run tests automaticaly and be confident you have not broken existing functionality.
2. shunit2 is required just for testing and it's pretty small dependency.
3. -w N tests weren't even before I made these.

[ymattw]

okay i forgot -w N wasn't there, i agree with you on 1, but prefer to implement simpler script to remove shunit2 dep.

[ymattw]

Send pull request for 4f29fc8 please, I will implement others in my own way.

[jan-matejka]

Go ahead, invent another wheel

[ymattw]

don't you think there are reasons people reinventing wheels? i hate that too but i have my own choice when balancing between simplification and functionality.

[TaraRed]

not easy to do that programically as it is an interact tool

Checking cdiff file anotherFile | md5sum against a known MD5sum, for which you have guaranteed that the output of cdiff looks good by eye; is a priceless way to do it.

[jan-matejka]

@TomWij that's kind of clever but not better. That way:

1. the test will tell you only that it is broken, not where it broke.
2. And while doing code review, or someone just interested browsing the tests, or whatever, you can cat expected_out and see exactly what should come out of cdiff.

[ymattw]

@yaccz I respect your professional attitude on this issue, could you update README.rst as well and send me a pull request? I am not sure which one is homepage of shunit2.

[TaraRed]

Jan Matějka, I wasn't addressing you and I thought we were on par. I was addressing the statement where @ymattw has shown that he doesn't know or understand automated testing. Anyhow, I'll address your points anyway, they might benefit ymattw too:

(1) the test will tell you only that it is broken, not where it broke.

Tests only test one thing, thus there will be one thing broken so you know exactly what needs to be fixed.

(2) cat expected_out and see exactly what should come out of cdiff.

It is a waste of time to do this all manually when it can be automated, as that makes the tests actually useful; once you have a checksum mismatch, that is the moment you need to output them so you can look into the difference.

You have stated before:

The tests require manual inspection of the output and are not included in the ebuild.

This is what we're trying to solve here, right? Automated testing would give the best outcome, it makes you spot things you don't see on your local system...

If you don't go for this, excuse me, I'm just following this so I know whether to let the ebuild call automated tests or not (and to suggest them to be used).

With kind regards,

Tom Wijsman Gentoo Developer

[jan-matejka]

@TomWij

This is what we're trying to solve here, right?

right, I was just comparing the current approach that I implemented vs. your suggestion to use checksums.

Tests only test one thing, thus there will be one thing broken so you know exactly what needs to be fixed.

That's true only for unit tests. I mean, in this case, if the test with checksum fails, you know what file failed. But to see what part of the output causes the fail, you need to

1. checkout older version
2. generate the output
3. checkout the current (failing) version
4. and compare the outputs

while when using the expected output in test directly, you go straight for 4.

@ymattw will do

[TaraRed]

Ah right, the checksum was suggested for speed purposes only anyway. You're right, I haven't looked at the commits..

[ymattw]

shunit2 isn't that good, I insist to fix in my own way (one case tests one thing). That's ok if you think I am reinventing another wheel, or think I don't know or understand automated testing. I appreciate your feedback anyway.

[jan-matejka]

shunit2 isn't that good,

why?

[ymattw]

It is too big for me when I only need one assertion function and a kick-off driver, it might be good for large projects but not a good fit for cdiff, that's my opinion.

Anyway... if you look into my last commit, I just implemented your idea in my own way, I hope you are ok to this. I will update document later, right now priority for me is to figure out where I can enhance the incremental diff.

[jan-matejka]

too big?

yac@deathstar % find -type f -exec du {} \;   
32  ./lib/shflags
8   ./lib/versions
4   ./lib/shlib
4   ./bin/gen_test_results.sh
4   ./bin/which
4   ./doc/RELEASE_NOTES-2.1.4.txt
16  ./doc/README.html
28  ./doc/LGPL-2.1
8   ./doc/rst2html.css
4   ./doc/RELEASE_NOTES-2.1.1.txt
4   ./doc/RELEASE_NOTES-2.1.3.txt
4   ./doc/RELEASE_NOTES-2.1.2.txt
4   ./doc/RELEASE_NOTES-2.1.5.txt
4   ./doc/RELEASE_NOTES-2.1.0.txt
4   ./doc/contributors.txt
20  ./doc/shunit2.txt
4   ./doc/TODO.txt
4   ./doc/coding_standards.txt
4   ./doc/RELEASE_NOTES-2.1.6.txt
36  ./doc/shunit2.html
8   ./doc/CHANGES-2.1.txt
8   ./doc/README.txt
4   ./doc/design_doc.txt
4   ./src/shunit2_test_standalone.sh
8   ./src/shunit2_test_misc.sh
8   ./src/shunit2_test_asserts.sh
8   ./src/shunit2_test_helpers
4   ./src/shunit2_test.sh
4   ./src/shunit2_test_failures.sh
32  ./src/shunit2
8   ./src/shunit2_test_macros.sh
4   ./examples/math_test.sh
4   ./examples/lineno_test.sh
4   ./examples/mkdir_test.sh
4   ./examples/equality_test.sh
4   ./examples/math.inc
4   ./examples/party_test.sh
--------------------------------------------------------------------------------
~/Downloads/shunit2-2.1.6  
yac@deathstar % find -type f -exec du {} \; | cut -f 1 | python -c 'import sys; print sum([int(x) for x in sys.stdin.readlines()])'
320

That's ridiculous

[jan-matejka]

1. It's not big, it's small
2. it's reusable. Everyone who already knows shunit2 can just look into it with no overhead of learning YetAnotherLimitedTestRunnerThatOnlyThisFuckingProjectIsUsing.

[jan-matejka]

[ymattw]

That's ridiculous It's not big, it's small

I need only 1 function, shunit2 is BIG for cdiff. Also shunit2 is actually to do unit test for shell scripts, cdiff is python, if you need unit test, use builtin unittest module or similar.

Everyone who already knows shunit2 can just look into it with no overhead of learning

For who doesn't already know shunit2, there's learning effort bigger than reading my simple script. I doubt how many people know it, because it's not smart to write shell scripts large enough and require unit tests.

YetAnotherLimitedTestRunnerThatOnlyThisFuckingProjectIsUsing

You use gentoo, is gentoo yet another Linux distribution? You use zsh, is zsh yet another shell?

Be nice, gentleman. Don't show angry face to people, it only makes yourself looks bad.

Learn some shell tips, try make your command line shorter.

[jan-matejka]

k, I'm done with this. Looking forward to see your script that is not too big for cdiff.