Re: [lmi] [lmi-commits] [6055] Mark lines needing '--gui_test_path'

[Greg Chicares]

On 2014-12-10 13:46, Vadim Zeitlin wrote:
> On Wed, 10 Dec 2014 12:36:13 +0000 Greg Chicares <address@hidden> wrote:
> 
> GC> Revision: 6055
> GC>           http://svn.sv.gnu.org/viewvc/?view=rev&root=lmi&revision=6055
> GC> Author:   chicares
> GC> Date:     2014-12-10 12:36:11 +0000 (Wed, 10 Dec 2014)
> GC> Log Message:
> GC> -----------
> GC> Mark lines needing '--gui_test_path'
> 
>  Hello,
> 
>  I have a question about this patch as it goes against what I was doing to
> implement support for --gui_test_path so far and I wonder if this is
> intentional. To be precise, this patch seems to strongly hint that you'd
> like to modify the test code to use something different from
> configurable_settings::instance().default_input_filename() to obtain the
> file name to use in the test. My plan was however to keep the test code
> here unchanged and instead ensure that default_input_filename() returns
> what we want it to return, i.e. uses the value of --gui_test_path command
> line option, if any.

Glad you asked. The patch indicates exactly what we'd like to do:
it's what makes sense in the problem domain. We prefer not to alter
what default_input_filename() returns: that wouldn't correspond to
anything desirable in the problem domain.

>  I thought of doing it like this because we need to make a backup of
> configurable_settings.xml file anyhow, to prevent it from being modified by
> the tests (notably the calculation summary one), and we could also adjust
> the values to it while doing this. It's not completely trivial because the
> class configurable_settings doesn't provide any way of modifying the
> default input filename field, so it would have to be done entirely outside
> of it, but it seemed to me that it was still worth doing it because we
> could also need to modify other configurable settings for the test purposes
> (e.g. spreadsheet file extension comes to mind). Am I overcomplicating
> things needlessly here or is there some merit to this line of thought?

These are good questions. In the problem domain, there's no need
for anything like this, though, so let's avoid the complications.
We know our own workflow, and we have a good understanding of the
things that are likely and unlikely to go wrong; we focus on the
former because it's not practical to test everything.

>  Also, I assume it's intentional that no comments about using
> --gui_test_path were added to the other occurrences of
> configurable_settings::instance().default_input_filename() in the testing
> code, i.e. those in configurable_settings, default_update, and
> input_validation tests. And I suspect this is because these tests are only
> meant to be run when --distribution option is given. And this, in turn,
> seems to imply that --distribution and --gui_test_path are incompatible. Is
> this correct? I.e. should the test give an error if both of them are
> specified?

For 'wx_test_input_validation.cpp', this was an unintentional
oversight--corrected 20141210T1521Z, revision 6056. Thanks for
noticing it. As for the rest, I could say again that it's all
necessary due to problem-domain considerations, but one should
not use that trick thrice in one email, and I imagine it leaves
you unsatisfied anyway--and, this being the easiest one to
explain (relatively speaking), let me explain it.

(A) The other two files cited should not be changed. (B) Yes,
both should be run only if '--distribution' is specified. But
(B) is not the reason for (A). (A) is true because these two
files test lmi capabilities that are controlled by the
configurable-settings file.

Test files generally belong in '--gui_test_path'. Ideally we
would have specified that originally, instead of ever putting
them in the same place as default_input_filename(). (A few end
users may actually do that, though I'd recommend against it).
The purpose of revisions 6055 and 6056 is to indicate what we
would preferably have specified up front.

'--distribution' and '--gui_test_path' are not incompatible.
They are orthogonal. A path for storing the GUI test's input
files is always required, whether it's explicitly specified with
'--gui_test_path' or simply allowed to default to a hardcoded
value. It's required with '--distribution'. It's also required
without '--distribution'. The two don't interact.