"GOT", but the "O" is a cute, smiling pufferfish. Index | Thread | Search

From:
Mark Jamsek <mark@jamsek.com>
Subject:
tog: teach test harness to count and basic tree test
To:
Game of Trees <gameoftrees@openbsd.org>
Date:
Thu, 20 Apr 2023 17:40:02 +1000

Download raw body.

Thread
The below diff teaches the tog test harness to read count modifier
instructions, and adds basic tree view regress tests--one of which
demonstrates the count mod instruction.

It applies on top of pending diffs: (1) naddy's tweaks to drop the
SCREENDUMP defines, unset TOG_* and set TERM=vt220 envvars, and
unconditionally write | and -; and (2) stsp's improvements for the wait
instruction to use pthread conditions (this has been tweaked since the
previous revision by renaming and making the pthread_cond_t condition
a member of the blame thread_args structure instead).

The diff might apply cleanly without them, and the tree/blame split view
regress may even work without (2), but just in case they are also
attached below for convenience.

-----------------------------------------------
commit e3a6e42f80f296c5aa342a76bf4e2034d84833bd (main)
from: Mark Jamsek <mark@jamsek.dev>
date: Thu Apr 20 07:37:40 2023 UTC
 
 tog: teach test harness to count and basic tree tests
 
 Add count instruction to the test harness to simulate count modifier compound
 keys (e.g., 11j), and add basic tests for the tree view.
 
 M  regress/tog/Makefile  |    4+  1-
 A  regress/tog/tree.sh   |  148+  0-
 M  tog/tog.c             |   17+  4-

3 files changed, 169 insertions(+), 5 deletions(-)

diff fcced8ec5faace6abc220a080cd29cdddbdbef41 e3a6e42f80f296c5aa342a76bf4e2034d84833bd
commit - fcced8ec5faace6abc220a080cd29cdddbdbef41
commit + e3a6e42f80f296c5aa342a76bf4e2034d84833bd
blob - eca41169193af38a1320fdb788385993cd078872
blob + 03259147951700e81a8aef5359da33d889dc0a6e
--- regress/tog/Makefile
+++ regress/tog/Makefile
@@ -1,4 +1,4 @@
-REGRESS_TARGETS=log diff blame
+REGRESS_TARGETS=log diff blame tree
 NOOBJ=Yes
 
 GOT_TEST_ROOT=/tmp
@@ -12,4 +12,7 @@ blame:
 blame:
 	./blame.sh -q -r "$(GOT_TEST_ROOT)"
 
+tree:
+	./tree.sh -q -r "$(GOT_TEST_ROOT)"
+
 .include <bsd.regress.mk>
blob - /dev/null
blob + 767a816949acdc89e85319bb9909f115024b5579 (mode 755)
--- /dev/null
+++ regress/tog/tree.sh
@@ -0,0 +1,148 @@
+#!/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_tree_basic()
+{
+	test_init tree_basic 48 8
+
+	local head_id=`git_show_head $testroot/repo`
+
+	cat <<EOF >$TOG_TEST_SCRIPT
+SCREENDUMP
+EOF
+
+	cat <<EOF >$testroot/view.expected
+commit $head_id
+[1/4] /
+
+  alpha
+  beta
+  epsilon/
+  gamma/
+
+EOF
+
+	cd $testroot/repo && tog tree
+	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_tree_vsplit_blame()
+{
+	test_init tree_vsplit_blame 120 8
+
+	local head_id=`git_show_head $testroot/repo`
+	local head_id_truncated=`trim_obj_id 8 $head_id`
+	local head_id_short=`trim_obj_id 32 $head_id`
+
+	cat <<EOF >$TOG_TEST_SCRIPT
+KEY_ENTER
+WAIT_FOR_UI	wait for blame to finish
+SCREENDUMP
+EOF
+
+	cat <<EOF >$testroot/view.expected
+commit $head_id_truncated|commit $head_id
+[1/4] /                                |[1/1] /alpha
+                                       |$head_id_short alpha
+  alpha                                |
+  beta                                 |
+  epsilon/                             |
+  gamma/                               |
+                                       |
+EOF
+
+	cd $testroot/repo && tog tree
+	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_tree_hsplit_blame()
+{
+	test_init tree_hsplit_blame 48 24
+
+	local head_id=`git_show_head $testroot/repo`
+	local head_id_truncated=`trim_obj_id 8 $head_id`
+	local head_id_short=`trim_obj_id 32 $head_id`
+
+	cat <<EOF >$TOG_TEST_SCRIPT
+j
+KEY_ENTER
+S		toggle horizontal split
+4-		4x decrease blame split
+WAIT_FOR_UI	wait for blame to finish
+SCREENDUMP
+EOF
+
+	cat <<EOF >$testroot/view.expected
+commit $head_id
+[2/4] /
+
+  alpha
+  beta
+  epsilon/
+  gamma/
+
+
+
+------------------------------------------------
+commit $head_id
+[1/1] /beta
+$head_id_short beta
+
+
+
+
+
+
+
+
+
+
+EOF
+
+	cd $testroot/repo && tog tree
+	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_tree_basic
+run_test test_tree_vsplit_blame
+run_test test_tree_hsplit_blame
blob - d9961429f19e9acc49150d85407f5f73815a445b
blob + 358a1fdc5e00738c6e5e68459f2d9e7cf23e1404
--- tog/tog.c
+++ tog/tog.c
@@ -1623,13 +1623,17 @@ tog_read_script_key(FILE *script, int *ch, int *done)
  * key instruction to *ch. If at EOF, set the *done flag.
  */
 static const struct got_error *
-tog_read_script_key(FILE *script, int *ch, int *done)
+tog_read_script_key(FILE *script, struct tog_view *view, int *ch, int *done)
 {
 	const struct got_error	*err = NULL;
 	char			*line = NULL;
 	size_t			 linesz = 0;
 
-	*ch = -1;
+	if (view->count && --view->count) {
+		*ch = view->ch;
+		return NULL;
+	} else
+		*ch = -1;
 
 	if (getline(&line, &linesz, script) == -1) {
 		if (feof(script)) {
@@ -1655,7 +1659,16 @@ tog_read_script_key(FILE *script, int *ch, int *done)
 		*ch = KEY_UP;
 	else if (strncasecmp(line, "SCREENDUMP", 10) == 0)
 		*ch = TOG_KEY_SCRDUMP;
-	else
+	else if (isdigit((unsigned char)*line)) {
+		char *t = line;
+
+		while (isdigit((unsigned char)*t))
+			++t;
+		view->ch = *ch = *t;
+		*t = '\0';
+		/* ignore error, view->count is 0 if instruction is invalid */
+		view->count = strtonum(line, 0, INT_MAX, NULL);
+	} else
 		*ch = *line;
 
 done:
@@ -1703,7 +1716,7 @@ view_input(struct tog_view **new, int *done, struct to
 		return got_error_set_errno(errcode, "pthread_mutex_unlock");
 
 	if (using_mock_io) {
-		err = tog_read_script_key(tog_io.f, &ch, done);
+		err = tog_read_script_key(tog_io.f, view, &ch, done);
 		if (err) {
 			errcode = pthread_mutex_lock(&tog_mutex);
 			return err;

-----------------------------------------------
commit fcced8ec5faace6abc220a080cd29cdddbdbef41
from: Mark Jamsek <mark@jamsek.dev>
date: Thu Apr 20 07:37:39 2023 UTC
 
 tog regress: zap needless defines and use "vt220" TERM
 
 Also, overwrite - and | unconditionally when capturing screen dumps,
 and unset TOG_COLORS and TOG_DIFF_ALGORITHM in regress setup.
 Tweaks suggested by naddy@
 
 M  regress/tog/common.sh  |  3+   0-
 M  tog/tog.c              |  7+  10-

2 files changed, 10 insertions(+), 10 deletions(-)

diff 20eaeb6b3c3c5b3419685f94016bcd9c145d39d6 fcced8ec5faace6abc220a080cd29cdddbdbef41
commit - 20eaeb6b3c3c5b3419685f94016bcd9c145d39d6
commit + fcced8ec5faace6abc220a080cd29cdddbdbef41
blob - e03be4c0dcc342c5c4bee7432c3f25219682d085
blob + 5c67c9b6991543475718b5f280e03afe3d39a724
--- regress/tog/common.sh
+++ regress/tog/common.sh
@@ -17,8 +17,11 @@ unset TOG_VIEW_SPLIT_MODE
 
 . ../cmdline/common.sh
 
+unset TOG_COLORS
+unset TOG_DIFF_ALGORITHM
 unset TOG_VIEW_SPLIT_MODE
 unset LC_ALL
+export TERM=vt220
 export LC_ALL=C.UTF-8
 export COLUMNS=80
 export LINES=24
blob - 4f544396c2a7e9672cb82f64712cf9b3840d4881
blob + d9961429f19e9acc49150d85407f5f73815a445b
--- tog/tog.c
+++ tog/tog.c
@@ -622,9 +622,7 @@ static int using_mock_io;
 } tog_io;
 static int using_mock_io;
 
-#define TOG_SCREEN_DUMP		"SCREENDUMP"
-#define TOG_SCREEN_DUMP_LEN	(sizeof(TOG_SCREEN_DUMP) - 1)
-#define TOG_KEY_SCRDUMP		SHRT_MIN
+#define TOG_KEY_SCRDUMP	SHRT_MIN
 
 /*
  * We implement two types of views: parent views and child views.
@@ -1450,9 +1448,9 @@ strip_trailing_ws(char *str, int *n)
 
 /*
  * Extract visible substring of line y from the curses screen
- * and strip trailing whitespace. If vline is set and locale is
- * UTF-8, overwrite line[vline] with '|' because the ACS_VLINE
- * character is written out as 'x'. Write the line to file f.
+ * and strip trailing whitespace. If vline is set, overwrite
+ * line[vline] with '|' because the ACS_VLINE character is
+ * written out as 'x'. Write the line to file f.
  */
 static const struct got_error *
 view_write_line(FILE *f, int y, int vline)
@@ -1471,7 +1469,7 @@ view_write_line(FILE *f, int y, int vline)
 	 */
 	strip_trailing_ws(line, &r);
 
-	if (vline > 0 && got_locale_is_utf8())
+	if (vline > 0)
 		line[vline] = '|';
 
 	w = fprintf(f, "%s\n", line);
@@ -1521,7 +1519,7 @@ screendump(struct tog_view *view)
 			    view->child->begin_y : view->begin_y;
 
 		for (i = 0; i < view->lines; i++) {
-			if (hline && got_locale_is_utf8() && i == hline - 1) {
+			if (hline && i == hline - 1) {
 				int c;
 
 				/* ACS_HLINE writes out as 'q', overwrite it */
@@ -1655,7 +1653,7 @@ tog_read_script_key(FILE *script, int *ch, int *done)
 		*ch = KEY_DOWN;
 	else if (strncasecmp(line, "KEY_UP", 6) == 0)
 		*ch = KEY_UP;
-	else if (strncasecmp(line, TOG_SCREEN_DUMP, TOG_SCREEN_DUMP_LEN) == 0)
+	else if (strncasecmp(line, "SCREENDUMP", 10) == 0)
 		*ch = TOG_KEY_SCRDUMP;
 	else
 		*ch = *line;
@@ -4205,7 +4203,6 @@ init_mock_term(const char *test_script_path)
 		goto done;
 	}
 
-	/* use local TERM so we test in different environments */
 	if (newterm(NULL, tog_io.cout, tog_io.cin) == NULL)
 		err = got_error_msg(GOT_ERR_IO,
 		    "newterm: failed to initialise curses");

-----------------------------------------------
commit 20eaeb6b3c3c5b3419685f94016bcd9c145d39d6
from: Mark Jamsek <mark@jamsek.dev>
date: Thu Apr 20 07:37:39 2023 UTC
 
 tog: use pthread conditions for regress wait instruction
 
 As per stsp's suggestion, rather than busy wait the view loop, wait for the
 blame thread to signal completion before drawing the screen. We can add more
 pthread conditions for other views when needed, but by keeping the general
 WAIT_FOR_UI instruction and flag, this should make their use context-dependent
 so we won't need to add more test harness WAIT_* instructions.
 
 M  tog/tog.c  |  38+  8-

1 file changed, 38 insertions(+), 8 deletions(-)

diff eab8809f807c6efa50cb26be1c068d6ae680a28a 20eaeb6b3c3c5b3419685f94016bcd9c145d39d6
commit - eab8809f807c6efa50cb26be1c068d6ae680a28a
commit + 20eaeb6b3c3c5b3419685f94016bcd9c145d39d6
blob - 8c21271a99fe7416ffc1a6b97ce8ce11403a6f35
blob + 4f544396c2a7e9672cb82f64712cf9b3840d4881
--- tog/tog.c
+++ tog/tog.c
@@ -432,6 +432,7 @@ struct tog_blame_thread_args {
 	int *complete;
 	got_cancel_cb cancel_cb;
 	void *cancel_arg;
+	pthread_cond_t blame_complete;
 };
 
 struct tog_blame {
@@ -1632,9 +1633,6 @@ tog_read_script_key(FILE *script, int *ch, int *done)
 
 	*ch = -1;
 
-	if (tog_io.wait_for_ui)
-		goto done;
-
 	if (getline(&line, &linesz, script) == -1) {
 		if (feof(script)) {
 			*done = 1;
@@ -1644,6 +1642,7 @@ tog_read_script_key(FILE *script, int *ch, int *done)
 			goto done;
 		}
 	}
+
 	if (strncasecmp(line, "WAIT_FOR_UI", 11) == 0)
 		tog_io.wait_for_ui = 1;
 	else if (strncasecmp(line, "KEY_ENTER", 9) == 0)
@@ -4212,6 +4211,7 @@ done:
 		    "newterm: failed to initialise curses");
 
 	using_mock_io = 1;
+
 done:
 	if (err)
 		tog_io_close();
@@ -6002,6 +6002,16 @@ draw_blame(struct tog_view *view)
 	if (view->gline > blame->nlines)
 		view->gline = blame->nlines;
 
+	if (tog_io.wait_for_ui) {
+		struct tog_blame_thread_args *bta = &s->blame.thread_args;
+		int rc;
+
+		rc = pthread_cond_wait(&bta->blame_complete, &tog_mutex);
+		if (rc)
+			return got_error_set_errno(rc, "pthread_cond_wait");
+		tog_io.wait_for_ui = 0;
+	}
+
 	if (asprintf(&line, "[%d/%d] %s%s", view->gline ? view->gline :
 	    s->first_displayed_line - 1 + s->selected_line, blame->nlines,
 	    s->blame_complete ? "" : "annotating... ", s->path) == -1) {
@@ -6123,11 +6133,6 @@ 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;
 }
 
@@ -6225,6 +6230,13 @@ blame_thread(void *arg)
 	ta->repo = NULL;
 	*ta->complete = 1;
 
+	if (tog_io.wait_for_ui) {
+		errcode = pthread_cond_signal(&ta->blame_complete);
+		if (errcode && err == NULL)
+			err = got_error_set_errno(errcode,
+			    "pthread_cond_signal");
+	}
+
 	errcode = pthread_mutex_unlock(&tog_mutex);
 	if (errcode && err == NULL)
 		err = got_error_set_errno(errcode, "pthread_mutex_unlock");
@@ -6505,6 +6517,15 @@ open_blame_view(struct tog_view *view, char *path,
 	view->search_setup = search_setup_blame_view;
 	view->search_next = search_next_view_match;
 
+	if (using_mock_io) {
+		struct tog_blame_thread_args *bta = &s->blame.thread_args;
+		int rc;
+
+		rc = pthread_cond_init(&bta->blame_complete, NULL);
+		if (rc)
+			return got_error_set_errno(rc, "pthread_cond_init");
+	}
+
 	return run_blame(view);
 }
 
@@ -6524,6 +6545,15 @@ close_blame_view(struct tog_view *view)
 		got_object_qid_free(blamed_commit);
 	}
 
+	if (using_mock_io) {
+		struct tog_blame_thread_args *bta = &s->blame.thread_args;
+		int rc;
+
+		rc = pthread_cond_destroy(&bta->blame_complete);
+		if (rc && err == NULL)
+			err = got_error_set_errno(rc, "pthread_cond_destroy");
+	}
+
 	free(s->path);
 	free_colors(&s->colors);
 	return err;


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