lib-httpd.sh: print error.log on error
PATCH 1/1 lib-httpd.sh: print error.log on error
Failure to bring up httpd for testing is not considered an error, so the trash directory, which contains this error.log file, is removed and we don't know what made httpd fail to start. Improve the situation a bit.
--- t/lib-httpd.sh | 1 + 1 file changed, 1 insertion(+)
t/lib-httpd.sh +1 -0
@@ -180,6 +180,7 @@ start_httpd() { if test $? -ne 0 then trap 'die' EXIT + cat "$HTTPD_ROOT_PATH"/error.log 2>/dev/null test_skip_or_die $GIT_TEST_HTTPD "web server setup failed" fi }
Signature · 1 line
2.8.2.524.g6ff3d78
Quoted 14 lines · Nguyễn Thái Ngọc Duy
Failure to bring up httpd for testing is not considered an error, so the trash directory, which contains this error.log file, is removed and we don't know what made httpd fail to start. Improve the situation a bit.
Signed-off-by Nguyễn Thái Ngọc Duy <[email protected]>--- t/lib-httpd.sh | 1 + 1 file changed, 1 insertion(+)t/lib-httpd.sh +0
-0
t/lib-httpd.sh start_httpd() { · line 185
Quoted 6 lines
t/lib-httpd.sh +1
-0@@ -180,6 +180,7 @@ start_httpd() { if test $? -ne 0 then trap 'die' EXIT + cat "$HTTPD_ROOT_PATH"/error.log 2>/dev/null test_skip_or_die $GIT_TEST_HTTPD "web server setup failed" fi
I like the idea of giving more data on error, but I think this will break the TAP output and confuse anything parsing the output of the tests, like prove (I think arbitrary output should have "#" prepended).
Also (or alternatively), it should probably only happen when we are in verbose mode (it's not taken care of for us as usual because tests call start_httpd outside of a test_expect_ block). I think this eliminates the need to deal with the TAP thing (because our usual "-v" output is not TAP-compliant).
-Peff
Quoted 27 lines · Jeff King
Quoted 14 lines · Nguyễn Thái Ngọc Duy
Failure to bring up httpd for testing is not considered an error, so the trash directory, which contains this error.log file, is removed and we don't know what made httpd fail to start. Improve the situation a bit.
Signed-off-by Nguyễn Thái Ngọc Duy <[email protected]>--- t/lib-httpd.sh | 1 + 1 file changed, 1 insertion(+)t/lib-httpd.sh +0
-0t/lib-httpd.sh start_httpd() { · line 185
Quoted 6 lines
t/lib-httpd.sh +1
-0@@ -180,6 +180,7 @@ start_httpd() { if test $? -ne 0 then trap 'die' EXIT + cat "$HTTPD_ROOT_PATH"/error.log 2>/dev/null test_skip_or_die $GIT_TEST_HTTPD "web server setup failed" fiI like the idea of giving more data on error, but I think this will break the TAP output and confuse anything parsing the output of the tests, like prove (I think arbitrary output should have "#" prepended).
Quoted 5 lines
Also (or alternatively), it should probably only happen when we are in verbose mode (it's not taken care of for us as usual because tests call
start_httpdoutside of atest_expect_block). I think this eliminates the need to deal with the TAP thing (because our usual "-v" output is not TAP-compliant).
I like the verbose route, so here's v2
-- 8< --
Subject: [PATCH v2] lib-httpd.sh: print error.log on error
Failure to bring up httpd for testing is not considered an error, so the trash directory, which contains this error.log file, is removed and we don't know why httpd failed. Improve the situation a bit, print error.log but only in verbose mode.
--- t/lib-httpd.sh | 2 ++ 1 file changed, 2 insertions(+)
t/lib-httpd.sh +2 -0
@@ -180,6 +180,8 @@ start_httpd() { if test $? -ne 0 then trap 'die' EXIT + test "$verbose" = t && \ + cat "$HTTPD_ROOT_PATH"/error.log 2>/dev/null test_skip_or_die $GIT_TEST_HTTPD "web server setup failed" fi }
Signature · 2 lines
2.8.2.524.g6ff3d78 -- 8< --
Quoted 1 line · Duy Nguyen
I like the verbose route, so here's v2
I think this is OK, though...
t/lib-httpd.sh start_httpd() { · line 184
Quoted 10 lines
t/lib-httpd.sh +2
-0@@ -180,6 +180,8 @@ start_httpd() { if test $? -ne 0 then trap 'die' EXIT + test "$verbose" = t && \ + cat "$HTTPD_ROOT_PATH"/error.log 2>/dev/null
I think you can just spell this:
cat >&4 ...
(I had originally thought that we set up those fds only inside the test_expect blocks, but it is the redirection 2>&4 that we set up there; you can always use fd 4 as necessary).
It does incur a useless "cat" when we are not verbose, but I don't think that's a big deal for the error path like this.
-Peff
PATCH v3 1/1 lib-httpd.sh: print error.log on error
Failure to bring up httpd for testing is not considered an error, so the trash directory, which contains this error.log file, is removed and we don't know what made httpd fail to start. Improve the situation a bit, print error.log but only in verbose mode.
---
Redirect stdout to "verbose stderr" instead of testing $verbose.
t/lib-httpd.sh | 1 + 1 file changed, 1 insertion(+)
t/lib-httpd.sh +1 -0
@@ -180,6 +180,7 @@ start_httpd() { if test $? -ne 0 then trap 'die' EXIT + cat "$HTTPD_ROOT_PATH"/error.log >&4 2>/dev/null test_skip_or_die $GIT_TEST_HTTPD "web server setup failed" fi }
Signature · 1 line
2.8.2.524.g6ff3d78
Quoted 6 lines · Nguyễn Thái Ngọc Duy
Failure to bring up httpd for testing is not considered an error, so the trash directory, which contains this error.log file, is removed and we don't know what made httpd fail to start. Improve the situation a bit, print error.log but only in verbose mode.
Signed-off-by Nguyễn Thái Ngọc Duy <[email protected]>
Looks good to me. Thanks.
-Peff