Hi hackers, When I wrote patch of ecpg command notice bug, I recognized needs of regression tests for ecpg command notices and I say that I write the tests.
https://commitfest.postgresql.org/52/5497/ https://www.postgresql.org/message-id/0efab1f6-5d8d-451f-a7dc-ef9c73ba9e02%40oss.nttdata.com This mail is about patch of the tests. Patch is attached to this mail : ecpg_cmd_notice_regress_test.patch This patch passed CI. https://cirrus-ci.com/build/4861827500212224 And corresponding diff is below. https://github.com/ryogrid/postgres/compare/0ec3c295e7594ed3af86bca1a4b4be269c2f069d...72329311a75630594bcaa38248255360b7e8e525 I explain about implementation of this patch. What is this patch - add regression tests which test ecpg command notices such as warning and errors - test notices implemented in ecpg.addons file Basic policy on implementation - do in a way that matches the method using the existing pg_regress command as much as possible - avoid methods that increase the scope of influence Next, I list answers to points that are likely to be pointed out in advance below :) - shell scripts and bat files is used due to ... avoid non zero exit code of ecpg command makes tests failure avoid increasing C code for executing binary which cares cross platform - python code is used because I couldn't write meson.build appropriately describe dependency about materials which is used on tests without it. please help me... - as you said, kick this kind of tests by pg_regress accompanied with needless PG server process execution. but pg_regress doesn't execute test without it and making pg_regress require modification which has not small scope of influence --- Great regards, Ryo Kanbayashi https://github.com/ryogrid
diff --git a/src/interfaces/ecpg/test/ecpg_schedule b/src/interfaces/ecpg/test/ecpg_schedule index 254a0bacc75..e1cc61d8c52 100644 --- a/src/interfaces/ecpg/test/ecpg_schedule +++ b/src/interfaces/ecpg/test/ecpg_schedule @@ -31,6 +31,8 @@ test: preproc/variable test: preproc/outofscope test: preproc/whenever test: preproc/whenever_do_continue +test: preproc/notice_check +test: preproc/notice_informix_check test: sql/array test: sql/binary test: sql/bytea diff --git a/src/interfaces/ecpg/test/expected/preproc-check_cmd_notice.stderr b/src/interfaces/ecpg/test/expected/preproc-check_cmd_notice.stderr new file mode 100644 index 00000000000..31bfbc9f3bd --- /dev/null +++ b/src/interfaces/ecpg/test/expected/preproc-check_cmd_notice.stderr @@ -0,0 +1,13 @@ +preproc/notice.pgc:16: ERROR: AT option not allowed in CONNECT statement +preproc/notice.pgc:17: ERROR: AT option not allowed in DISCONNECT statement +preproc/notice.pgc:18: ERROR: AT option not allowed in SET CONNECTION statement +preproc/notice.pgc:21: WARNING: COPY FROM STDIN is not implemented +preproc/notice.pgc:25: ERROR: using variable "cursor_var" in different declare statements is not supported +preproc/notice.pgc:28: ERROR: cursor "duplicate_cursor" is already defined +preproc/notice.pgc:31: ERROR: SHOW ALL is not implemented +preproc/notice.pgc:33: ERROR: AT option not allowed in WHENEVER statement +preproc/notice.pgc:36: WARNING: no longer supported LIMIT #,# syntax passed to server +preproc/notice.pgc:39: WARNING: cursor "duplicate_cursor" has been declared but not opened +preproc/notice.pgc:39: WARNING: cursor "duplicate_cursor" has been declared but not opened +preproc/notice.pgc:39: WARNING: cursor ":cursor_var" has been declared but not opened +preproc/notice.pgc:39: WARNING: cursor ":cursor_var" has been declared but not opened diff --git a/src/interfaces/ecpg/test/expected/preproc-check_cmd_notice_informix.stderr b/src/interfaces/ecpg/test/expected/preproc-check_cmd_notice_informix.stderr new file mode 100644 index 00000000000..0b9013c126e --- /dev/null +++ b/src/interfaces/ecpg/test/expected/preproc-check_cmd_notice_informix.stderr @@ -0,0 +1,2 @@ +preproc/notice_informix.pgc:11: ERROR: AT option not allowed in CLOSE DATABASE statement +preproc/notice_informix.pgc:14: ERROR: "database" cannot be used as cursor name in INFORMIX mode diff --git a/src/interfaces/ecpg/test/expected/preproc-notice_check.stderr b/src/interfaces/ecpg/test/expected/preproc-notice_check.stderr new file mode 100644 index 00000000000..b31076a2527 --- /dev/null +++ b/src/interfaces/ecpg/test/expected/preproc-notice_check.stderr @@ -0,0 +1,15 @@ +notice.pgc:17: ERROR: AT option not allowed in CONNECT statement +notice.pgc:18: ERROR: AT option not allowed in DISCONNECT statement +notice.pgc:19: ERROR: AT option not allowed in SET CONNECTION statement +notice.pgc:20: ERROR: AT option not allowed in TYPE statement +notice.pgc:21: ERROR: AT option not allowed in WHENEVER statement +notice.pgc:22: ERROR: AT option not allowed in VAR statement +notice.pgc:25: WARNING: COPY FROM STDIN is not implemented +notice.pgc:29: ERROR: using variable "cursor_var" in different declare statements is not supported +notice.pgc:33: ERROR: cursor "duplicate_cursor" is already defined +notice.pgc:36: ERROR: SHOW ALL is not implemented +notice.pgc:39: WARNING: no longer supported LIMIT #,# syntax passed to server +notice.pgc:42: WARNING: cursor "duplicate_cursor" has been declared but not opened +notice.pgc:42: WARNING: cursor "duplicate_cursor" has been declared but not opened +notice.pgc:42: WARNING: cursor ":cursor_var" has been declared but not opened +notice.pgc:42: WARNING: cursor ":cursor_var" has been declared but not opened diff --git a/src/interfaces/ecpg/test/expected/preproc-notice_check.stdout b/src/interfaces/ecpg/test/expected/preproc-notice_check.stdout new file mode 100644 index 00000000000..e69de29bb2d diff --git a/src/interfaces/ecpg/test/expected/preproc-notice_informix_check.stderr b/src/interfaces/ecpg/test/expected/preproc-notice_informix_check.stderr new file mode 100644 index 00000000000..d6386aaa896 --- /dev/null +++ b/src/interfaces/ecpg/test/expected/preproc-notice_informix_check.stderr @@ -0,0 +1,2 @@ +notice_informix.pgc:12: ERROR: AT option not allowed in CLOSE DATABASE statement +notice_informix.pgc:15: ERROR: "database" cannot be used as cursor name in INFORMIX mode diff --git a/src/interfaces/ecpg/test/expected/preproc-notice_informix_check.stdout b/src/interfaces/ecpg/test/expected/preproc-notice_informix_check.stdout new file mode 100644 index 00000000000..e69de29bb2d diff --git a/src/interfaces/ecpg/test/pg_regress_ecpg.c b/src/interfaces/ecpg/test/pg_regress_ecpg.c index ba3477f9dd8..18d8b398a30 100644 --- a/src/interfaces/ecpg/test/pg_regress_ecpg.c +++ b/src/interfaces/ecpg/test/pg_regress_ecpg.c @@ -22,6 +22,13 @@ #include "lib/stringinfo.h" #include "pg_regress.h" +#define ECPG_CMD_NOTICE_TEST_PREFIX "preproc/notice" + +/* + * Tests whose prefix is ECPG_CMD_NOTICE_TEST_PREFIX check ecpg command's notice + * such as error or warning. Not checking of compiled ECPG programs behavior. + * So, we handle these tests differently. + */ /* * Create a filtered copy of sourcefile, removing any path @@ -115,6 +122,9 @@ ecpg_filter_stderr(const char *resultfile, const char *tmpfile) { char *p1 = strstr(linebuf.data, "connection to server "); + /* when ecpg command notice test */ + char *p3 = strstr(linebuf.data, "preproc"); + if (p1) { char *p2 = strstr(p1, "failed: "); @@ -125,6 +135,18 @@ ecpg_filter_stderr(const char *resultfile, const char *tmpfile) /* we don't bother to fix up linebuf.len */ } } + + if (p3) + { + char *p4 = strstr(p3, "notice"); + + if (p4) + { + memmove(p3, p4, strlen(p4) + 1); + /* we don't bother to fix up linebuf.len */ + } + } + fputs(linebuf.data, t); } @@ -162,9 +184,7 @@ ecpg_start_test(const char *testname, expectfile_source[MAXPGPATH]; char cmd[MAXPGPATH * 3]; char *appnameenv; - - snprintf(inprg, sizeof(inprg), "%s/%s", inputdir, testname); - snprintf(insource, sizeof(insource), "%s/%s.c", inputdir, testname); + bool is_ecpg_cmd_notice_test = false; /* make a version of the test name that has dashes in place of slashes */ initStringInfo(&testname_dash); @@ -175,15 +195,38 @@ ecpg_start_test(const char *testname, *c = '-'; } + /* check for ECPG_CMD_NOTICE_TEST_PREFIX literal in the beginning */ + if (strstr(testname, ECPG_CMD_NOTICE_TEST_PREFIX) == testname) + { + is_ecpg_cmd_notice_test = true; + } + + if (is_ecpg_cmd_notice_test) + { +#ifdef WIN32 + snprintf(inprg, sizeof(inprg), "%s/%s.bat", inputdir, testname); +#else + snprintf(inprg, sizeof(inprg), "%s/%s.sh", inputdir, testname); +#endif + } + else + { + snprintf(inprg, sizeof(inprg), "%s/%s", inputdir, testname); + snprintf(insource, sizeof(insource), "%s/%s.c", inputdir, testname); + } + snprintf(expectfile_stdout, sizeof(expectfile_stdout), "%s/expected/%s.stdout", expecteddir, testname_dash.data); snprintf(expectfile_stderr, sizeof(expectfile_stderr), "%s/expected/%s.stderr", expecteddir, testname_dash.data); - snprintf(expectfile_source, sizeof(expectfile_source), - "%s/expected/%s.c", - expecteddir, testname_dash.data); + if (!is_ecpg_cmd_notice_test) + { + snprintf(expectfile_source, sizeof(expectfile_source), + "%s/expected/%s.c", + expecteddir, testname_dash.data); + } snprintf(outfile_stdout, sizeof(outfile_stdout), "%s/results/%s.stdout", @@ -191,9 +234,12 @@ ecpg_start_test(const char *testname, snprintf(outfile_stderr, sizeof(outfile_stderr), "%s/results/%s.stderr", outputdir, testname_dash.data); - snprintf(outfile_source, sizeof(outfile_source), - "%s/results/%s.c", - outputdir, testname_dash.data); + if (!is_ecpg_cmd_notice_test) + { + snprintf(outfile_source, sizeof(outfile_source), + "%s/results/%s.c", + outputdir, testname_dash.data); + } add_stringlist_item(resultfiles, outfile_stdout); add_stringlist_item(expectfiles, expectfile_stdout); @@ -203,11 +249,18 @@ ecpg_start_test(const char *testname, add_stringlist_item(expectfiles, expectfile_stderr); add_stringlist_item(tags, "stderr"); - add_stringlist_item(resultfiles, outfile_source); - add_stringlist_item(expectfiles, expectfile_source); - add_stringlist_item(tags, "source"); + if (!is_ecpg_cmd_notice_test) + { + add_stringlist_item(resultfiles, outfile_source); + add_stringlist_item(expectfiles, expectfile_source); + add_stringlist_item(tags, "source"); + + ecpg_filter_source(insource, outfile_source); + } - ecpg_filter_source(insource, outfile_source); + appnameenv = psprintf("ecpg/%s", testname_dash.data); + setenv("PGAPPNAME", appnameenv, 1); + free(appnameenv); snprintf(cmd, sizeof(cmd), "\"%s\" >\"%s\" 2>\"%s\"", @@ -215,10 +268,6 @@ ecpg_start_test(const char *testname, outfile_stdout, outfile_stderr); - appnameenv = psprintf("ecpg/%s", testname_dash.data); - setenv("PGAPPNAME", appnameenv, 1); - free(appnameenv); - pid = spawn_process(cmd); if (pid == INVALID_PID) diff --git a/src/interfaces/ecpg/test/preproc/copy_files.py b/src/interfaces/ecpg/test/preproc/copy_files.py new file mode 100644 index 00000000000..8678c86e1cd --- /dev/null +++ b/src/interfaces/ecpg/test/preproc/copy_files.py @@ -0,0 +1,21 @@ +import shutil +import os +import sys + +def main(): + if len(sys.argv) < 4: + print("Usage: python copy_files.py <src_base> <destination_base> <file1> <file2> ...") + sys.exit(1) + + src = sys.argv[1] + destination = sys.argv[2] + files = sys.argv[3:] + + for file in files: + src_path = os.path.join(src, os.path.basename(file)) + dest_path = os.path.join(destination, os.path.basename(file)) + + shutil.copy2(src_path, dest_path) + +if __name__ == "__main__": + main() \ No newline at end of file diff --git a/src/interfaces/ecpg/test/preproc/meson.build b/src/interfaces/ecpg/test/preproc/meson.build index 775502e0102..26bef1f2e73 100644 --- a/src/interfaces/ecpg/test/preproc/meson.build +++ b/src/interfaces/ecpg/test/preproc/meson.build @@ -37,3 +37,31 @@ foreach pgc_file : pgc_files kwargs: ecpg_test_exec_kw, ) endforeach + +# copy materials for ecpg command notice test +copy_materials = [ + 'notice.pgc', + 'notice_informix.pgc', + 'notice_check.bat', + 'notice_check.sh', + 'notice_informix_check.bat', + 'notice_informix_check.sh', +] + +copy_cmdline = [ + python, + meson.current_source_dir() / 'copy_files.py', + meson.current_source_dir(), + meson.current_build_dir() +] + +foreach file : copy_materials + copy_cmdline += file +endforeach + +ecpg_test_dependencies += custom_target('copy_ecpg_test_materials', + input: meson.current_source_dir() / 'copy_files.py', + output: copy_materials, + command: copy_cmdline, + build_by_default: false, +) \ No newline at end of file diff --git a/src/interfaces/ecpg/test/preproc/notice.pgc b/src/interfaces/ecpg/test/preproc/notice.pgc new file mode 100644 index 00000000000..03a4f60d5e2 --- /dev/null +++ b/src/interfaces/ecpg/test/preproc/notice.pgc @@ -0,0 +1,42 @@ +/* Test ECPG notice/warning/error messages */ + +#include <stdlib.h> + +int +main(void) +{ + EXEC SQL BEGIN DECLARE SECTION; + char *cursor_var = "mycursor"; + short a; + EXEC SQL END DECLARE SECTION; + + /* For consistency with other tests */ + EXEC SQL CONNECT TO testdb AS con1; + + /* Test AT option errors */ + EXEC SQL AT con1 CONNECT TO testdb2; + EXEC SQL AT con1 DISCONNECT; + EXEC SQL AT con1 SET CONNECTION TO testdb2; + EXEC SQL AT con1 TYPE string IS char[11]; + EXEC SQL AT con1 WHENEVER NOT FOUND CONTINUE; + EXEC SQL AT con1 VAR a IS int; + + /* Test COPY FROM STDIN warning */ + EXEC SQL COPY test FROM stdin; + + /* Test same variable in multi declare statement */ + EXEC SQL DECLARE :cursor_var CURSOR FOR SELECT * FROM test; + EXEC SQL DECLARE :cursor_var CURSOR FOR SELECT * FROM test; + + /* Test duplicate cursor declarations */ + EXEC SQL DECLARE duplicate_cursor CURSOR FOR SELECT * FROM test; + EXEC SQL DECLARE duplicate_cursor CURSOR FOR SELECT * FROM test; + + /* Test SHOW ALL error */ + EXEC SQL SHOW ALL; + + /* Test deprecated LIMIT syntax warning */ + EXEC SQL SELECT * FROM test LIMIT 10, 5; + + return 0; +} \ No newline at end of file diff --git a/src/interfaces/ecpg/test/preproc/notice_check.bat b/src/interfaces/ecpg/test/preproc/notice_check.bat new file mode 100644 index 00000000000..172950897db --- /dev/null +++ b/src/interfaces/ecpg/test/preproc/notice_check.bat @@ -0,0 +1,7 @@ +@echo off + +PATH=..\preproc\;%PATH% +ecpg -o preproc\notice.c preproc\notice.pgc + +REM always return 0 for test purposes +exit /b 0 \ No newline at end of file diff --git a/src/interfaces/ecpg/test/preproc/notice_check.sh b/src/interfaces/ecpg/test/preproc/notice_check.sh new file mode 100755 index 00000000000..11338bb3ede --- /dev/null +++ b/src/interfaces/ecpg/test/preproc/notice_check.sh @@ -0,0 +1,7 @@ +#!/bin/sh + +PATH=../preproc/:$PATH +ecpg -c -o preproc/notice.c preproc/notice.pgc + +# always return 0 for testing purposes +exit 0 \ No newline at end of file diff --git a/src/interfaces/ecpg/test/preproc/notice_informix.pgc b/src/interfaces/ecpg/test/preproc/notice_informix.pgc new file mode 100644 index 00000000000..18281f3b9ee --- /dev/null +++ b/src/interfaces/ecpg/test/preproc/notice_informix.pgc @@ -0,0 +1,18 @@ +/* Test ECPG notice/warning/error messages */ + +#include <stdlib.h> + +int +main(void) +{ + /* For consistency with other tests */ + $CONNECT TO testdb AS con1; + + /* Test AT option usage at CLOSE statement in INFORMIX mode */ + $AT con1 CLOSE database; + + /* Test cursor name errors in INFORMIX mode */ + $DECLARE database CURSOR FOR SELECT * FROM test; + + return 0; +} diff --git a/src/interfaces/ecpg/test/preproc/notice_informix_check.bat b/src/interfaces/ecpg/test/preproc/notice_informix_check.bat new file mode 100644 index 00000000000..fd66da3b67e --- /dev/null +++ b/src/interfaces/ecpg/test/preproc/notice_informix_check.bat @@ -0,0 +1,7 @@ +@echo off + +PATH=..\preproc\;%PATH% +ecpg -C INFORMIX -o preproc\notice_informix.c preproc\notice_informix.pgc + +REM always return 0 for test purposes +exit /b 0 \ No newline at end of file diff --git a/src/interfaces/ecpg/test/preproc/notice_informix_check.sh b/src/interfaces/ecpg/test/preproc/notice_informix_check.sh new file mode 100755 index 00000000000..ef543e1082e --- /dev/null +++ b/src/interfaces/ecpg/test/preproc/notice_informix_check.sh @@ -0,0 +1,7 @@ +#!/bin/sh + +PATH=../preproc/:$PATH +ecpg -C INFORMIX -o preproc/notice_informix.c preproc/notice_informix.pgc + +# always return 0 for testing purposes +exit 0 \ No newline at end of file