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

Reply via email to