Allow finish() to throw an exception (yugabyte#227)

Add a boolean argument to finish() that, when true, will cause it to throw an exception if any errors occurred. Requested by @singpolyma, most of the work done by @rodo.

Closes yugabyte#80, yugabyte#104.

Makefile

@@ -43,13 +43,16 @@ EXTRA_CLEAN += $(SCHEDULE_DEST_FILES)
 SCHEDULE_FILES = $(wildcard test/schedule/*.sch)
 
 # These are our actual regression tests
-TEST_FILES 	= $(filter-out $(SCHEDULE_DEST_FILES),$(wildcard test/sql/*.sql))
+TEST_FILES 	?= $(filter-out $(SCHEDULE_DEST_FILES),$(wildcard test/sql/*.sql))
 
 # Plain test names
 ALL_TESTS	= $(notdir $(TEST_FILES:.sql=))
 
 # Some tests fail when run in parallel
-SERIAL_TESTS = coltap hastap
+SERIAL_TESTS ?= coltap hastap
+
+# Remove tests from SERIAL_TESTS that do not appear in ALL_TESTS
+SERIAL_TESTS := $(foreach test,$(SERIAL_TESTS),$(findstring $(test),$(ALL_TESTS)))
 
 # Some tests fail when run by pg_prove
 # TODO: The first 2 of these fail because they have tests that intentionally
@@ -336,7 +339,7 @@ test: test-serial test-parallel
 # dependencies (such as excluded tests) have changed since the last time we
 # ran.
 TB_DIR = test/build
-GENERATED_SCHEDULE_DEPS = $(TB_DIR)/tests $(TB_DIR)/exclude_tests
+GENERATED_SCHEDULE_DEPS = $(TB_DIR)/all_tests $(TB_DIR)/exclude_tests
 REGRESS = --schedule $(TB_DIR)/run.sch # Set this again just to be safe
 REGRESS_OPTS = --inputdir=test --load-language=plpgsql --max-connections=$(PARALLEL_CONN) --schedule $(SETUP_SCH) $(REGRESS_CONF)
 SETUP_SCH = test/schedule/main.sch # schedule to use for test setup; this can be forcibly changed by some targets!
@@ -372,12 +375,12 @@ $(TB_DIR)/which_schedule: $(TB_DIR)/ set_parallel_conn
 	@[ "`cat $@ 2>/dev/null`" = "$(SCHEDULE)" ] || (echo "Schedule changed to $(SCHEDULE)"; echo "$(SCHEDULE)" > $@)
 
 # Generated schedule files, one for serial one for parallel
-.PHONY: $(TB_DIR)/tests # Need this target to force schedule rebuild if $(TEST) changes
-$(TB_DIR)/tests: $(TB_DIR)/
-	@[ "`cat $@ 2>/dev/null`" = "$(TEST)" ] || (echo "Rebuilding $@"; echo "$(TEST)" > $@)
+.PHONY: $(TB_DIR)/all_tests # Need this target to force schedule rebuild if $(ALL_TESTS) changes
+$(TB_DIR)/all_tests: $(TB_DIR)/
+	@[ "`cat $@ 2>/dev/null`" = "$(ALL_TESTS)" ] || (echo "Rebuilding $@"; echo "$(ALL_TESTS)" > $@)
 
 .PHONY: $(TB_DIR)/exclude_tests # Need this target to force schedule rebuild if $(EXCLUDE_TEST) changes
-$(TB_DIR)/exclude_tests: $(TB_DIR)/
+$(TB_DIR)/exclude_tests: $(TB_DIR)/ $(TB_DIR)/all_tests
 	@[ "`cat $@ 2>/dev/null`" = "$(EXCLUDE_TEST)" ] || (echo "Rebuilding $@"; echo "$(EXCLUDE_TEST)" > $@)
 
 $(TB_DIR)/serial.sch: $(GENERATED_SCHEDULE_DEPS)

doc/pgtap.mmd

@@ -300,6 +300,11 @@ discrepancy between the planned number of tests and the number actually run:
 
     SELECT * FROM finish();
 
+If you need to throw an exception if some test failed, you can pass an
+option to `finish()`.
+
+    SELECT * FROM finish(true);    
+
 What a sweet unit!
 ------------------
 

sql/pgtap--1.0.0--1.1.0.sql

@@ -128,3 +128,59 @@ RETURNS SETOF NAME[] AS $$
   ORDER BY 1
 $$ LANGUAGE sql;
 
+
+-- finish
+DROP FUNCTION finish();
+DROP FUNCTION _finish (INTEGER, INTEGER, INTEGER);
+
+CREATE OR REPLACE FUNCTION _finish (INTEGER, INTEGER, INTEGER, BOOLEAN DEFAULT NULL)
+RETURNS SETOF TEXT AS $$
+DECLARE
+    curr_test ALIAS FOR $1;
+    exp_tests INTEGER := $2;
+    num_faild ALIAS FOR $3;
+    plural    CHAR;
+    raise_ex  ALIAS FOR $4;
+BEGIN
+    plural    := CASE exp_tests WHEN 1 THEN '' ELSE 's' END;
+
+    IF curr_test IS NULL THEN
+        RAISE EXCEPTION '# No tests run!';
+    END IF;
+
+    IF exp_tests = 0 OR exp_tests IS NULL THEN
+         -- No plan. Output one now.
+        exp_tests = curr_test;
+        RETURN NEXT '1..' || exp_tests;
+    END IF;
+
+    IF curr_test <> exp_tests THEN
+        RETURN NEXT diag(
+            'Looks like you planned ' || exp_tests || ' test' ||
+            plural || ' but ran ' || curr_test
+        );
+    ELSIF num_faild > 0 THEN
+        IF raise_ex THEN
+            RAISE EXCEPTION  '% test% failed of %', num_faild, CASE num_faild WHEN 1 THEN '' ELSE 's' END, exp_tests;
+        END IF;
+        RETURN NEXT diag(
+            'Looks like you failed ' || num_faild || ' test' ||
+            CASE num_faild WHEN 1 THEN '' ELSE 's' END
+            || ' of ' || exp_tests
+        );
+    ELSE
+
+    END IF;
+    RETURN;
+END;
+$$ LANGUAGE plpgsql;
+
+CREATE OR REPLACE FUNCTION finish (exception_on_failure BOOLEAN DEFAULT NULL)
+RETURNS SETOF TEXT AS $$
+    SELECT * FROM _finish(
+        _get('curr_test'),
+        _get('plan'),
+        num_failed(),
+        $1
+    );
+$$ LANGUAGE sql;

sql/pgtap.sql.in

@@ -180,13 +180,14 @@ RETURNS INTEGER AS $$
     SELECT _get('failed');
 $$ LANGUAGE SQL strict;
 
-CREATE OR REPLACE FUNCTION _finish (INTEGER, INTEGER, INTEGER)
+CREATE OR REPLACE FUNCTION _finish (INTEGER, INTEGER, INTEGER, BOOLEAN DEFAULT NULL)
 RETURNS SETOF TEXT AS $$
 DECLARE
     curr_test ALIAS FOR $1;
     exp_tests INTEGER := $2;
     num_faild ALIAS FOR $3;
     plural    CHAR;
+    raise_ex  ALIAS FOR $4;
 BEGIN
     plural    := CASE exp_tests WHEN 1 THEN '' ELSE 's' END;
 
@@ -206,6 +207,9 @@ BEGIN
             plural || ' but ran ' || curr_test
         );
     ELSIF num_faild > 0 THEN
+        IF raise_ex THEN
+            RAISE EXCEPTION  '% test% failed of %', num_faild, CASE num_faild WHEN 1 THEN '' ELSE 's' END, exp_tests;
+        END IF;
         RETURN NEXT diag(
             'Looks like you failed ' || num_faild || ' test' ||
             CASE num_faild WHEN 1 THEN '' ELSE 's' END
@@ -218,12 +222,13 @@ BEGIN
 END;
 $$ LANGUAGE plpgsql;
 
-CREATE OR REPLACE FUNCTION finish ()
+CREATE OR REPLACE FUNCTION finish (exception_on_failure BOOLEAN DEFAULT NULL)
 RETURNS SETOF TEXT AS $$
     SELECT * FROM _finish(
         _get('curr_test'),
         _get('plan'),
-        num_failed()
+        num_failed(),
+        $1
     );
 $$ LANGUAGE sql;
 

test/expected/moretap.out

@@ -1,48 +1,56 @@
 \unset ECHO
-1..46
-ok 1 - My pass() passed, w00t!
-ok 2 - Testing fail()
-ok 3 - We should get the proper output from fail()
-ok 4 - The output of finish() should reflect the test failure
-ok 5 - We should have one failure
-ok 6 - We should now have no failures
-ok 7 - diag() should work properly
-ok 8 - multiline diag() should work properly
-ok 9 - multiline diag() should work properly with existing comments
-ok 10 - diag(int)
-ok 11 - diag(numeric)
-ok 12 - diag(timestamptz)
-ok 13 - variadic text
-ok 14 - variadic int
-ok 15 - variadic unknown
-ok 16 - no_plan() should have stored a plan of 0
-ok 17 - Set the plan to 4000
-ok 18 - The output of finish() should reflect a high test plan
-ok 19 - Set the plan to 4
-ok 20 - The output of finish() should reflect a low test plan
-ok 21 - Reset the plan
-ok 22 - plan() should have stored the test count
-ok 23 - ok(true) should pass
-ok 24 - ok(true) should have the proper description
-ok 25 - ok(true) should have the proper diagnostics
-ok 26 - ok(true, '') should pass
-ok 27 - ok(true, '') should have the proper description
-ok 28 - ok(true, '') should have the proper diagnostics
-ok 29 - ok(true, 'foo') should pass
-ok 30 - ok(true, 'foo') should have the proper description
-ok 31 - ok(true, 'foo') should have the proper diagnostics
-ok 32 - ok(false) should fail
-ok 33 - ok(false) should have the proper description
-ok 34 - ok(false) should have the proper diagnostics
-ok 35 - ok(false, '') should fail
-ok 36 - ok(false, '') should have the proper description
-ok 37 - ok(false, '') should have the proper diagnostics
-ok 38 - ok(false, 'foo') should fail
-ok 39 - ok(false, 'foo') should have the proper description
-ok 40 - ok(false, 'foo') should have the proper diagnostics
-ok 41 - ok(NULL, 'null') should fail
-ok 42 - ok(NULL, 'null') should have the proper description
-ok 43 - ok(NULL, 'null') should have the proper diagnostics
-ok 44 - multiline desc should pass
-ok 45 - multiline desc should have the proper description
-ok 46 - multiline desc should have the proper diagnostics
+1..54
+ok 1 - Modify internal plan value
+ok 2 - My pass() passed, w00t!
+ok 3 - Testing fail()
+ok 4 - We should get the proper output from fail()
+ok 5 - The output of finish() should reflect the test failure
+ok 6 - Increase internal plan value after testing finish
+ok 7 - The output of finish(false) should reflect the test failure
+ok 8 - Increase internal plan value after testing finish
+ok 9 - The output of finish(NULL) should reflect the test failure
+ok 10 - Increase internal plan value after testing finish
+ok 11 - finish(true) should throw an exception
+ok 12 - We should have one failure
+ok 13 - Reset internal failure count
+ok 14 - We should now have no failures
+ok 15 - diag() should work properly
+ok 16 - multiline diag() should work properly
+ok 17 - multiline diag() should work properly with existing comments
+ok 18 - diag(int)
+ok 19 - diag(numeric)
+ok 20 - diag(timestamptz)
+ok 21 - variadic text
+ok 22 - variadic int
+ok 23 - variadic unknown
+ok 24 - no_plan() should have stored a plan of 0
+ok 25 - Set the plan to 4000
+ok 26 - The output of finish() should reflect a high test plan
+ok 27 - Set the plan to 4
+ok 28 - The output of finish() should reflect a low test plan
+ok 29 - Reset the plan
+ok 30 - plan() should have stored the test count
+ok 31 - ok(true) should pass
+ok 32 - ok(true) should have the proper description
+ok 33 - ok(true) should have the proper diagnostics
+ok 34 - ok(true, '') should pass
+ok 35 - ok(true, '') should have the proper description
+ok 36 - ok(true, '') should have the proper diagnostics
+ok 37 - ok(true, 'foo') should pass
+ok 38 - ok(true, 'foo') should have the proper description
+ok 39 - ok(true, 'foo') should have the proper diagnostics
+ok 40 - ok(false) should fail
+ok 41 - ok(false) should have the proper description
+ok 42 - ok(false) should have the proper diagnostics
+ok 43 - ok(false, '') should fail
+ok 44 - ok(false, '') should have the proper description
+ok 45 - ok(false, '') should have the proper diagnostics
+ok 46 - ok(false, 'foo') should fail
+ok 47 - ok(false, 'foo') should have the proper description
+ok 48 - ok(false, 'foo') should have the proper diagnostics
+ok 49 - ok(NULL, 'null') should fail
+ok 50 - ok(NULL, 'null') should have the proper description
+ok 51 - ok(NULL, 'null') should have the proper diagnostics
+ok 52 - multiline desc should pass
+ok 53 - multiline desc should have the proper description
+ok 54 - multiline desc should have the proper diagnostics

test/sql/istap.sql

@@ -147,5 +147,5 @@ SELECT * FROM test_records();
 
 /****************************************************************************/
 -- Finish the tests and clean up.
-SELECT * FROM finish();
+SELECT * FROM finish(false); -- Arbitrarily decided to test `finish(false)` here... :)
 ROLLBACK;

test/sql/moretap.sql

@@ -1,35 +1,64 @@
 \unset ECHO
 \i test/setup.sql
 
-\set numb_tests 46
+\set numb_tests 54
 SELECT plan(:numb_tests);
 
 -- Replace the internal record of the plan for a few tests.
-UPDATE  __tcache__ SET value = 3 WHERE label = 'plan';
+SELECT is( _set('plan', 4), 4, 'Modify internal plan value');
 
 /****************************************************************************/
 -- Test pass().
 SELECT pass( 'My pass() passed, w00t!' );
 
 -- Test fail().
-\set fail_numb 2
+\set fail_numb 3
 \echo ok :fail_numb - Testing fail()
 SELECT is(
-       fail('oops'),
-       'not ok 2 - oops
-# Failed test 2: "oops"', 'We should get the proper output from fail()');
+    fail('oops'),
+    format( E'not ok %1$s - oops\n# Failed test %1$s: "oops"', :fail_numb ),
+    'We should get the proper output from fail()'
+);
+
+/*
+ * NOTE: From this point until we call _set('failed') below we should always
+ * have *one* test failure, *BUT* if the tests themselves start failing then
+ * you'll have extra failures which will throw off all the successive counts!
+ */
 
--- Check the finish() output.
+-- Check the finish() output with no value.
 SELECT is(
     (SELECT * FROM finish() LIMIT 1),
-    '# Looks like you failed 1 test of 3',
+    '# Looks like you failed 1 test of 4',
     'The output of finish() should reflect the test failure'
 );
 
+-- Make sure that false and NULL work as well
+SELECT is( _set('plan', 6), 6, 'Increase internal plan value after testing finish' );
+SELECT is(
+    (SELECT * FROM finish(false) LIMIT 1),
+    '# Looks like you failed 1 test of 6',
+    'The output of finish(false) should reflect the test failure'
+);
+SELECT is( _set('plan', 8), 8, 'Increase internal plan value after testing finish' );
+SELECT is(
+    (SELECT * FROM finish(NULL) LIMIT 1),
+    '# Looks like you failed 1 test of 8',
+    'The output of finish(NULL) should reflect the test failure'
+);
+
+-- Verify that finish(true) works
+SELECT is( _set('plan', 10), 10, 'Increase internal plan value after testing finish' );
+SELECT throws_ok(
+    $$SELECT finish(true)$$,
+    '1 test failed of 10',
+    'finish(true) should throw an exception'
+);
+
 /****************************************************************************/
 -- Check num_failed
 SELECT is( num_failed(), 1, 'We should have one failure' );
-UPDATE  __tcache__ SET value = 0 WHERE label = 'failed';
+SELECT is( _set('failed', 0), 0, 'Reset internal failure count' );
 SELECT is( num_failed(), 0, 'We should now have no failures' );
 
 /****************************************************************************/
@@ -69,16 +98,14 @@ SELECT * FROM test_variadic();
 -- Check no_plan.
 DELETE FROM __tcache__ WHERE label = 'plan';
 SELECT * FROM no_plan();
-SELECT is( value, 0, 'no_plan() should have stored a plan of 0' )
-  FROM __tcache__
- WHERE label = 'plan';
+SELECT is( _get('plan'), 0, 'no_plan() should have stored a plan of 0' );
 
 -- Set the plan to a high number.
 DELETE FROM __tcache__ WHERE label = 'plan';
 SELECT is( plan(4000), '1..4000', 'Set the plan to 4000' );
 SELECT is(
     (SELECT * FROM finish() LIMIT 1),
-    '# Looks like you planned 4000 tests but ran 17',
+    '# Looks like you planned 4000 tests but ran 25',
     'The output of finish() should reflect a high test plan'
 );
 
@@ -87,16 +114,14 @@ DELETE FROM __tcache__ WHERE label = 'plan';
 SELECT is( plan(4), '1..4', 'Set the plan to 4' );
 SELECT is(
     (SELECT * FROM finish() LIMIT 1),
-    '# Looks like you planned 4 tests but ran 19',
+    '# Looks like you planned 4 tests but ran 27',
     'The output of finish() should reflect a low test plan'
 );
 
 -- Reset the original plan.
 DELETE FROM __tcache__ WHERE label = 'plan';
 SELECT is( plan(:numb_tests), '1..' || :numb_tests, 'Reset the plan' );
-SELECT is( value, :numb_tests, 'plan() should have stored the test count' )
-  FROM __tcache__
- WHERE label = 'plan';
+SELECT is( _get('plan'), :numb_tests, 'plan() should have stored the test count' );
 
 /****************************************************************************/
 -- Test ok()