[Patch] Test suite for compare function

[Sam Crawley]

On Thu, 12 Nov 2020, at 00:18, Pavel Sanda wrote:
> On Wed, Nov 11, 2020 at 09:10:09PM +1300, Sam Crawley wrote:
> > 
> > I'm not sure I understand. When cmd_mode == true (which is always the case when run() is invoked via the LFUN),
> 
> Maybe I do not understand your intentions then. I thought you 
> introduced cmd_mode to distinguish command line case, but now you say 
> it should be always true (when called via lfun).
> I'll try to explain my concern from a different angle, maybe we'll meet 
> that way:
> To my knowledge you had two ways to reach 'run' - either (i) normally 
> through compare dialog's OK or (ii) via version control history 
> comparison trigger.
> And then perhaps (iii) via commandline, which I believe no one ever 
> seriously tried and per your comments might not even work due to 
> synchronization issues. 
> he (ii) case was hijacking slotOK call in initializeparams to mimick as 
> if the comparison was normally launched by user.
> 
> Now, your patch is fixing (iii) but at the same time undoing the logic 
> of how error() and finished() is used in case (ii) so my concern is 
> that (ii) will be broken.
> My first thought was that cmd_mode was intended to distinguish (ii) and 
> (iii) and let (ii) to stay untouched but the rest of code and our 
> discussion suggest otherwise.
> Does it make sense now?
> 

Thanks, yes I understand now. I didn't realise the VC compare invoked Compare in the same way I am from the command line, and my changes will indeed break it.

In that case, I'll add a different parameter to the LFUN to differentiate between case (ii) and (iii). So instead of passing "run" (and the two file names), you will pass "run-sync" to indicate that you want it to block for the results. (Alternatively it could be "run-cmd" to indicate the command line, but it technically could be called in other ways, so "run-sync" is more general).

Does that sound reasonable?

Thanks,
Sam.