On 23-04-15 09:07PM, Mark Jamsek wrote:
> On 23-04-15 11:12AM, Stefan Sperling wrote:
> > On Sat, Apr 15, 2023 at 06:59:49PM +1000, Mark Jamsek wrote:
> > > This is a rebased diff on top of main with the basic diff and blame
> > > regress, and your TOG_TEST_SCRIPT suggestion.
> > 
> > Great! ok.
> > 
> > > +	if (strncasecmp(line, "SLEEP ", 6) == 0 &&
> > > +	    isdigit((unsigned char)line[6])) {
> > > +		sleep(line[6] - '0');
> > > +		*ch = -1;
> > 
> > Instead of sleeping, or skipping interactive display based on using_mock_io,
> > we could have scripting instructions that wait until a certain event occurs.
> > Such as "wait until the blame thread signals that it is done".
> > That would also fix the problem and be generally applicable to similar
> > cases such as when the log thread is involved. But perhaps checking
> > using_mock_io is simpler? I am not sure.
> > 
> > Interactive use depends on the main thread sleeping in getch(), so in
> > that case the main thread should never block for long. But tests probably
> > won't need that behaviour?
> 
> Hmm...Yes, both are good options. Let me think about this a bit and come
> up with something. I think I'm leaning to the new instruction to
> optionally wait on an event. Although either approach is an improvement,
> I think that way offers a more suitable solution. Thanks!

That was actually a simple yet brilliant idea you had!

The below diff (which applies on top of main) adds a new WAIT_FOR_UI
instruction to the test harness, and a basic blame regress test that
demonstrates its use.  When the instruction is received a flag is set,
which delays processing further instructions till it is unset, which, in
this case, is when the blame process is complete. This can easily be
extrapolated to future cases that require waiting till the UI has been
drawn.

diff /home/mark/src/got
commit - 5bde47590bc2803fac92ad9b3362511f427d3c6a
path + /home/mark/src/got
blob - ac485dd3c4b41a848f35a558031031452fc7aa5e
file + regress/tog/Makefile
--- regress/tog/Makefile
+++ regress/tog/Makefile
@@ -1,4 +1,4 @@
-REGRESS_TARGETS=log diff
+REGRESS_TARGETS=log diff blame
 NOOBJ=Yes
 
 GOT_TEST_ROOT=/tmp
@@ -9,4 +9,7 @@ diff:
 diff:
 	./diff.sh -q -r "$(GOT_TEST_ROOT)"
 
+blame:
+	./blame.sh -q -r "$(GOT_TEST_ROOT)"
+
 .include <bsd.regress.mk>
blob - /dev/null
file + regress/tog/blame.sh (mode 755)
--- /dev/null
+++ regress/tog/blame.sh
@@ -0,0 +1,80 @@
+#!/bin/sh
+#
+# Copyright (c) 2023 Mark Jamsek <mark@jamsek.dev>
+#
+# Permission to use, copy, modify, and distribute this software for any
+# purpose with or without fee is hereby granted, provided that the above
+# copyright notice and this permission notice appear in all copies.
+#
+# THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+# WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+# MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+# ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+# WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
+# ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+# OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+
+. ./common.sh
+
+test_blame_basic()
+{
+	test_init blame_basic 80 8
+
+	local commit_id1=`git_show_head $testroot/repo`
+
+	got checkout $testroot/repo $testroot/wt > /dev/null
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		test_done "$testroot" "$ret"
+		return 1
+	fi
+
+	echo aaaa >> $testroot/wt/alpha
+	(cd $testroot/wt && got commit -m "a change" > /dev/null)
+	local commit_id2=`git_show_head $testroot/repo`
+
+	echo bbbb >> $testroot/wt/alpha
+	(cd $testroot/wt && got commit -m "b change" > /dev/null)
+	local commit_id3=`git_show_head $testroot/repo`
+
+	echo cccc >> $testroot/wt/alpha
+	(cd $testroot/wt && got commit -m "c change" > /dev/null)
+	local commit_id4=`git_show_head $testroot/repo`
+	local author_time=`git_show_author_time $testroot/repo`
+	local ymd=`date -u -r $author_time +"%G-%m-%d"`
+
+	cat <<EOF >$TOG_TEST_SCRIPT
+WAIT_FOR_UI	wait for blame to finish
+SCREENDUMP
+EOF
+
+	local commit_id1_short=`trim_obj_id 32 $commit_id1`
+	local commit_id2_short=`trim_obj_id 32 $commit_id2`
+	local commit_id3_short=`trim_obj_id 32 $commit_id3`
+	local commit_id4_short=`trim_obj_id 32 $commit_id4`
+
+	cat <<EOF >$testroot/view.expected
+commit $commit_id4
+[1/4] /alpha
+$commit_id1_short alpha
+$commit_id2_short aaaa
+$commit_id3_short bbbb
+$commit_id4_short cccc
+
+
+EOF
+
+	cd $testroot/wt && tog blame alpha
+	cmp -s $testroot/view.expected $testroot/view
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		diff -u $testroot/view.expected $testroot/view
+		test_done "$testroot" "$ret"
+		return 1
+	fi
+
+	test_done "$testroot" "$ret"
+}
+
+test_parseargs "$@"
+run_test test_blame_basic
blob - b1b624c8f817efab9f0fca62e40bd69dab642202
file + tog/tog.c
--- tog/tog.c
+++ tog/tog.c
@@ -617,6 +617,7 @@ struct tog_io {
 	FILE	*cin;
 	FILE	*cout;
 	FILE	*f;
+	int	 wait_for_ui;
 } tog_io;
 static int using_mock_io;
 
@@ -1629,6 +1630,11 @@ tog_read_script_key(FILE *script, int *ch, int *done)
 	char			*line = NULL;
 	size_t			 linesz = 0;
 
+	*ch = -1;
+
+	if (tog_io.wait_for_ui)
+		goto done;
+
 	if (getline(&line, &linesz, script) == -1) {
 		if (feof(script)) {
 			*done = 1;
@@ -1637,7 +1643,10 @@ tog_read_script_key(FILE *script, int *ch, int *done)
 			err = got_ferror(script, GOT_ERR_IO);
 			goto done;
 		}
-	} else if (strncasecmp(line, "KEY_ENTER", 9) == 0)
+	}
+	if (strncasecmp(line, "WAIT_FOR_UI", 11) == 0)
+		tog_io.wait_for_ui = 1;
+	else if (strncasecmp(line, "KEY_ENTER", 9) == 0)
 		*ch = KEY_ENTER;
 	else if (strncasecmp(line, "KEY_RIGHT", 9) == 0)
 		*ch = KEY_RIGHT;
@@ -6112,6 +6121,11 @@ draw_blame(struct tog_view *view)
 
 	view_border(view);
 
+	if (tog_io.wait_for_ui) {
+		if (s->blame_complete)
+			tog_io.wait_for_ui = 0;
+	}
+
 	return NULL;
 }
 

-- 
Mark Jamsek <fnc.bsdbox.org|got.bsdbox.org>
GPG: F2FF 13DE 6A06 C471 CA80  E6E2 2930 DC66 86EE CF68