[Patch] Test suite for compare function

[Pavel Sanda]

On Sun, Nov 08, 2020 at 05:14:42PM +1300, Sam Crawley wrote:
> Hi all,

Hi Sam,

welcome and thanks for working on this. Couple small things:

- Please send in a separate email to our list the following GPL statement:

I hereby grant permission to license my contributions to LyX under the GNU
General Public License, version 2 or any later version.


> From c11d1d7fec19f2c37ed9e2a2162e1d2966d3c643 Mon Sep 17 00:00:00 2001
> From: Sam Crawley <sam at crawley.nz>
> Date: Fri, 6 Nov 2020 20:56:09 +1300
> Subject: [PATCH 1/4] Fix issue in running compare as a command
> 
> Because Compare uses threads, we need to make sure it is finished when a
> compare is "run" through a command. This was a problem for command
> sequences, because the next command would start running before the compare
> was done.

Git commit messages tend to have the following structure: first summary line,
empty line and then the details. This helps with log summaries.


>  src/frontends/qt/GuiCompare.cpp | 28 +++++++++++++++++++---------
>  src/frontends/qt/GuiCompare.h   |  2 +-
>  2 files changed, 20 insertions(+), 10 deletions(-)
> 
> diff --git a/src/frontends/qt/GuiCompare.cpp b/src/frontends/qt/GuiCompare.cpp
> index d1da5b337f..380244a5c3 100644
> --- a/src/frontends/qt/GuiCompare.cpp
> +++ b/src/frontends/qt/GuiCompare.cpp
> @@ -42,7 +42,7 @@ namespace frontend {
>  
>  GuiCompare::GuiCompare(GuiView & lv)
>  	: GuiDialog(lv, "compare", qt_("Compare LyX files")),
> -	compare_(0), dest_buffer_(0), old_buffer_(0), new_buffer_(0)
> +       compare_(0), dest_buffer_(0), old_buffer_(0), new_buffer_(0)

If possible try to separate patches with whitespace only changes and
real code changes.

> @@ -343,7 +348,12 @@ bool GuiCompare::initialiseParams(std::string const &par)
>  	if (cmd.getArg(0) == "run") {
>  		oldFileCB->setEditText(toqstr(cmd.getArg(1)));
>  		newFileCB->setEditText(toqstr(cmd.getArg(2)));
> -		slotOK();
> +        enableControls(false);
> +        run(true);
> +
> +        compare_->wait(1000000);

On a first sight this looks fishy. We unconditionally launch run
on the place we previously did not without dependency on cmd_mode.
Do I miss something?

Secondly instead of waiting for arbitrary time, can't we just wait
until it finishes?

> From 9cdd8e876e812b6dd194df1f586a369fb5bcb3d7 Mon Sep 17 00:00:00 2001
> From: Sam Crawley <sam at crawley.nz>
> Date: Fri, 6 Nov 2020 20:57:14 +1300
> Subject: [PATCH 2/4] Created initial test for compare function
> 
> Runs the compare via a command line, and then compared the output to the
> expected result. Required adding a script to do the comparison, so that
> the timestamps on changes in the lyx file are ignored.


Scott/Kornel will presumably review/check the testing part.

Cheers,
Pavel