add tests for report options #379
Conversation
|
One doesn't normally waste test time on simple setters. Is there a point to these tests? |
|
I think what @jralls is stating is: ideally a unit test would test the functionality of a module rather than its implementation... let's look at the particular commit ea71c69 describing gnc:html-string-sanitize - it is a very short function https://github.com/christopherlam/gnucash/blob/ea71c696c21d3868030bb7427f079e6b05c5d78e/gnucash/report/report-system/html-utilities.scm#L857 which gets about 7 tests to illustrate the functionality - how it handles & < and > - note the html-string-sanitize could be written in C, in scheme using named lets, using recursive lists etc. doesn't really matter -- we want to test the function does exactly what we want: it fixes & < and > and leaves everything else (including extended unicode) alone. I think options.scm is a very complicated beast - it uses antiquated scheme paradigms, and deserves to be rewritten. The above test suite seems to mainly test the implementation of the various options, verifying the various functions are storing strings/values correctly into the option data structures - this is the old-school scheme paradigm, but we really need the functionality tested, i.e. does the (make-string-option) genuinely accepts strings? Does it correctly refuse numbers? Does the (make-date-option) correctly checks for valid time64 dates (any integer)? Does it refuse fractions? Does it accept good relative-dates eg 'end-of-last-year'? Does it accept old timepair dates? Do the functions read the saved-reports values correctly? Can they generate good saved-reports code? etc. A good options.scm unit test would ideally verify the "options.scm API" exists and is functioning well, so that in the future, options.scm can be refactored completely either in scheme, C++ or anything, and still present the exact same functionality that the reports (and the tests) will use. |
Not quite. A test obviously tests the implementation, it being a bit difficult to test anything else. The objective is to ensure that the implementation achieves the desired functionality (as @christopherlam says in the later bit). My point is that if a function simply takes a list of arguments and assigns them to a set of variables there's not much point in testing it: It can be easily determined by inspection that it does that. Expanding, if a function starts off setting a bunch of variables and then does other stuff there's no need to test the setting, get straight to the other stuff. |
|
Hi John, Christopher, Not to waste even more time on those kind of discussions: |
|
It's a very practical issue: Unnecessary/not-useful tests add code clutter and take additional run time with no payback. |
Hi,
here is my next follow-up on Bug787401 "tests for the report system", this time for report options.