Introduce a tester for extfs helpers #3730
Comments
Some random notes:
|
|
|
|
|
|
|
|
|
|
|
|
|
(My web host is down so I copied the files to GitHub Pages and updated the links. The downside is that GitHub doesn't let me set "text/plain" MIME type on all files so you won't be able to browse them online. But it's not a big deal: just look in the patches. I wanted a web hosting just because of README.html.) |
I get the following log: |
Replying to andrew_b:
(I didn't discover this bug because on 32-bit Ubuntus (my system) @HAVE_ZIPINFO@ is always zero.) |
|
Hi mooffie,
What you've done here is truly amazing, and I was meaning to write something encouraging for a few weeks already, but, very unfortunately, I've been kept more busy than ever due to infrastructure problems with a few other OSS projects which needed to be urgently dealt with before it's too late.
Just a few general comments I can make without having a deep look at your code:
I happened to guide my colleagues to implement a system similar in spirit last year for a "compiler" project (which, crazily enough, compiled business models + data into offers for customers at a certain price, if you are interested), checking that the compiled offers/prices are correct for given inputs. At that time, I suggested to use py.test as a test framework, and write a small helper class, from which the tests could inherit. This gave us tons of things for free, and the most important were:
I understand that probably we don't want to depend on Python (and worse even, non-standard modules for it) for this project even for the test suite, but I would still consider integrating with Autotools [1] test harness if possible instead of writing an own one, or looking at something like BATS [2]. I once had to write a test harness in plain POSIX sh that generated JUnit-compatible output, and I got to hate every bit of it...
[1]: https://www.gnu.org/software/automake/manual/html_node/Tests.html#Tests
The latter looks *very* neat if you ask me, but on the con side, it seems it's not very well maintained, which is sad. Not sure if you know better ones, this one is the best I've stumbled upon so far.
In any case, even if you chose to keep your own harness in its entirety, maybe you can still get some inspiration from looking at those, and, at very least, I think it would be great if you could also add TAP output. I don't think that's it's really a lot of work, but one could then integrate it with Jenkins plugins as I did the other time with py.test output, or generate nice HTML reports with TAP-Formatter-HTML @ Travis & push them to a page like sources.m-c.org...
I think one problem that we currently have with the test suite, is that the test reports are buried in the logs and can't be visually inspected in a great way. Check this out for instance: http://i.imgur.com/K2WK3.png & the web is full of Jenkins Test Result / TAP / JUnit report screenshots.
Thanks once again for tackling this long standing pain point, and cheers for some great code written for that purpose! |
Replying to mooffie:
Squashed. Thanks.
I moved the tester from tests/src/extfs-helpers-listcmd into tests/src/vfs/extfs/helpers in order to make tests/ tree consistent with src/ one. |
@yury: Thanks for the info-laden comment, it's much appreciated. Lots of juicy pointers in it.
Just a quick note, as I have to go (I'll reply some other time(s) as I study further your comment):
|
Replying to andrew_b:
(Personally, though, I don't mind "helpers". We can always rename it in the future if we add other kinds of tests.)
(@yury: don't worry, I'm not ignoring the things you wrote!) |
Hi Mooffie,
You ignoring my comments is the least of my fears ;-) in fact, I'd be happy to have it merged as is, because we can always improve on it later, and it already solves a very important problem that has been annoying us for a long time.
Re. 2a) sure, I get it, this is why I said it was amazing work in the first place. Re. 2b) I quickly glanced through your runner and it *felt* like it was many screenfuls of code (which may very well be an inflated impression), some usual custom ANSI coloring thing (which I suspect doesn't check whether it's being run interactively or not), etc., and this led me to wondering whether we could re-use more stuff from Autotools to keep it slimmer. In any case, as I said, my concerns with custom harnesses is that they usually don't implement any kind of machine parseable output (simplest of all being TAP), and they quickly grow out of control as one starts adding more fancy stuff like parallel execution, incremental testing, etc.
Re. 1), I absolutely agree that test autodiscovery is awesome, but I was thinking in terms of using a configure substitution to provide the list of tests to Autotools automatically, instead of hiding all the tests from its harness behind one single custom runner. I understand that the annoying downside is that you'd have to reconfigure if you plug in a new test (even though this can be automated I guess?), but the advantage is that you can use Autotools parallel test harness, TAP support, etc.
It would be great if you could look into this approach before we dismiss it, even if you'll conclude that we should like to keep the current system as you proposed it, and extend it as needed.
To try to clarify once again my line of thinking, I think that currently one very annoying thing about our test suite is that the output is not grokable. Imagine we had an awesome dashboard summary after running make check, that would have made assessing the situation much easier. To generate such a dashboard we need granular machine-parseable output, like TAP or JUnit.
The unit test framework we use, check, can do it, but we are not taking advantage of it. There is a TAP driver for Autotools as well (same here). Now, if you throw in more custom harnesses, that's not making it any easier. Now, there is stuff like the blood chilling FISH scripts, and if we ever come up with something to test those, this will be yet another harness, etc.
Cheers! |
Btw, finally got to push the patches for #2707 that I have cleaned up some time ago. Unfortunately, it's listing function doesn't seem to be easily amenable to testing. Another data point to look at... in the mean time, maybe we should merge #2707 in anyways. |
Replying to zaytsev:
If I change the tester now it will need to undergo a review process that will take time. It's much more urgent to have these tests in place already. The tester is good enough to be committed as-is.
In fact, I wouldn't mind getting rid of this autodiscovery feature if the price was adding just two or three words per test to the Makefile. I'll be looking into this possibility too.
|
I think that you should chill out a little bit and worry less. We are not as heinous of perpetrators as it might appear... :-) Christmas time is coming and all is good. |
|
|
Merged to master: [f1bc449]. |
@mooffie, also, it now occurred to me, that we still have to list test data in EXTRA_DIST, so maybe one could just as well put test names in a variable, re-arrange them to live in each individual subdirectory, specify them in TESTS and use same variable for EXTRA_DIST.
Or else, simply add the data directory as a whole to EXTRA_DIST. It just feels somehow wrong to me to argue for test autodiscovery to avoid listing test names in TESTS and then still have to register every single test file individually with the build system.
I guess I still like the first idea better, unless we decide that further integration with Autotools harness is not something we want to pursue. Would be interested to hear what do you think about these suggestions... |
Important
This issue was migrated from Trac:
mooffie(@mooffie)Here's a tester intended to test the "list" command of extfs helpers.
(Others would use the phrase "test framework" instead of "tester". I'm trying to sound friendly.)
I provide with it tests for uzip, urar, uzoo, and lslR. That's to demonstrate the power of the tester. If this tester gets merged into MC I'll provide tests for more helpers (anybody could, though; it's very easy).
Why
The dire problem of us not currently having tests for covering extfs has been mentioned several times here.
How it works
The tester works by feeding the helpers example input, parsing their output and comparing it to the known correct output.
To understand this better, I uploaded the tester here for easier browsing; specifically:
How it looks like
(a) Either do "make check"; or:
(b) Run the tester directly, with "run", to see colorful output:
Note
Original attachments:
mooffie(@mooffie) onNov 23, 2016 at 13:49 UTCmooffie(@mooffie) onNov 23, 2016 at 13:50 UTCmooffie(@mooffie) onNov 23, 2016 at 13:50 UTCmooffie(@mooffie) onNov 23, 2016 at 13:51 UTCmooffie(@mooffie) onNov 23, 2016 at 13:51 UTCmooffie(@mooffie) onNov 23, 2016 at 13:51 UTCmooffie(@mooffie) onNov 23, 2016 at 13:52 UTCmooffie(@mooffie) onNov 23, 2016 at 13:52 UTCmooffie(@mooffie) onNov 23, 2016 at 13:52 UTCmooffie(@mooffie) onNov 23, 2016 at 13:52 UTCmooffie(@mooffie) onNov 23, 2016 at 13:53 UTCmooffie(@mooffie) onDec 14, 2016 at 11:21 UTCmooffie(@mooffie) onDec 19, 2016 at 13:00 UTCmooffie(@mooffie) onDec 19, 2016 at 13:36 UTCThe text was updated successfully, but these errors were encountered: