From: Mark Jamsek Subject: Re: tog test harness To: Game of Trees Date: Fri, 14 Apr 2023 22:49:01 +1000 On 23-04-14 02:06PM, Stefan Sperling wrote: > On Thu, Apr 13, 2023 at 06:34:05PM +1000, Mark Jamsek wrote: > > On 23-04-13 12:01AM, Mark Jamsek wrote: > > > Yes, we can definitely open it before calling pledge(). I was about to > > > zzz so will send this now but will cook a diff tomorrow to see if that > > > fixes the pledge issue. Thanks :) > > > > Unfortunately, my initial suspicions were valid: opening the screendump > > file before pledging still doesn't work. I thought you might be onto > > something because earlier I was calling init_mock_term() from main > > before pledge() and it was aborting--but I wasn't opening the screendump > > file then. > > > > I checked-out that revision and moved the screendump file opening to > > init_mock_term() before pledge, but no dice. > > > > It seems every curses ioctl() request SIGABRTs. When we init_curses(), > > we uncook the terminal with cbreak(), which calls ioctl(): > > The reason for the aborts is that pledge will not permit ioctls > related to TTYs on non-tty files. Which makes sense. > > I do believe we should run tests under pledge if at all possible. > Because otherwise our tests will not catch code which introduces > pledge violations. Yes, definitely! I think tog regress should run under pledge. > The patch below makes the existing tog tests pass with pledge active. > What do you think? Very nice, ok. I was actually working on a similar diff with minor changes but I like this better; I was also calling init_curses() from main. I've added a patch below with one of the differences though, which applies on top of yours. It just consolidates all the tog_io_close() calls at the end of main(). We may also need to make future changes as test coverage grows but this is great having regress pledged. Thank you! diff /home/mark/src/got commit - 324ddda19751f233f3f99b1dba4e950e955d8ae1 path + /home/mark/src/got blob - 45cbc985724f5499495edf465cbddf52611fbae8 file + tog/tog.c --- tog/tog.c +++ tog/tog.c @@ -4189,10 +4189,7 @@ init_mock_term(const char *test_script_path) goto done; } - /* - * XXX Perhaps we should define "xterm" as the terminal - * type for standardised testing instead of using $TERM? - */ + /* 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"); @@ -4274,7 +4271,7 @@ cmd_log(int argc, char *argv[]) static const struct got_error * cmd_log(int argc, char *argv[]) { - const struct got_error *io_err, *error; + const struct got_error *error; struct got_repository *repo = NULL; struct got_worktree *worktree = NULL; struct got_object_id *start_id = NULL; @@ -4412,11 +4409,6 @@ done: if (error == NULL) error = pack_err; } - if (using_mock_io) { - io_err = tog_io_close(); - if (error == NULL) - error = io_err; - } tog_free_refs(); return error; } @@ -5786,7 +5778,7 @@ cmd_diff(int argc, char *argv[]) static const struct got_error * cmd_diff(int argc, char *argv[]) { - const struct got_error *io_err, *error; + const struct got_error *error; struct got_repository *repo = NULL; struct got_worktree *worktree = NULL; struct got_object_id *id1 = NULL, *id2 = NULL; @@ -5912,11 +5904,6 @@ done: if (error == NULL) error = pack_err; } - if (using_mock_io) { - io_err = tog_io_close(); - if (error == NULL) - error = io_err; - } tog_free_refs(); return error; } @@ -6891,7 +6878,7 @@ cmd_blame(int argc, char *argv[]) static const struct got_error * cmd_blame(int argc, char *argv[]) { - const struct got_error *io_err, *error; + const struct got_error *error; struct got_repository *repo = NULL; struct got_worktree *worktree = NULL; char *cwd = NULL, *repo_path = NULL, *in_repo_path = NULL; @@ -7028,11 +7015,6 @@ done: if (error == NULL) error = pack_err; } - if (using_mock_io) { - io_err = tog_io_close(); - if (error == NULL) - error = io_err; - } tog_free_refs(); return error; } @@ -7865,7 +7847,7 @@ cmd_tree(int argc, char *argv[]) static const struct got_error * cmd_tree(int argc, char *argv[]) { - const struct got_error *io_err, *error; + const struct got_error *error; struct got_repository *repo = NULL; struct got_worktree *worktree = NULL; char *cwd = NULL, *repo_path = NULL, *in_repo_path = NULL; @@ -8005,11 +7987,6 @@ done: if (error == NULL) error = pack_err; } - if (using_mock_io) { - io_err = tog_io_close(); - if (error == NULL) - error = io_err; - } tog_free_refs(); return error; } @@ -8757,7 +8734,7 @@ cmd_ref(int argc, char *argv[]) static const struct got_error * cmd_ref(int argc, char *argv[]) { - const struct got_error *io_err, *error; + const struct got_error *error; struct got_repository *repo = NULL; struct got_worktree *worktree = NULL; char *cwd = NULL, *repo_path = NULL; @@ -8851,11 +8828,6 @@ done: if (error == NULL) error = pack_err; } - if (using_mock_io) { - io_err = tog_io_close(); - if (error == NULL) - error = io_err; - } tog_free_refs(); return error; } @@ -9722,7 +9694,7 @@ main(int argc, char *argv[]) int main(int argc, char *argv[]) { - const struct got_error *error = NULL; + const struct got_error *io_err, *error = NULL; const struct tog_cmd *cmd = NULL; int ch, hflag = 0, Vflag = 0; char **cmd_argv = NULL; @@ -9821,6 +9793,11 @@ main(int argc, char *argv[]) error = cmd->cmd_main(argc, cmd_argv ? cmd_argv : argv); } + if (using_mock_io) { + io_err = tog_io_close(); + if (error == NULL) + error = io_err; + } endwin(); if (cmd_argv) { int i; -- Mark Jamsek GPG: F2FF 13DE 6A06 C471 CA80 E6E2 2930 DC66 86EE CF68