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