From ba3e6e2bca97df14920375b0a1ebf4eab95b78b5 Mon Sep 17 00:00:00 2001 From: Andrew Dunstan Date: Tue, 23 Apr 2024 15:27:40 -0400 Subject: [PATCH] Post review fixes for test_json_parser test module . Add missing copytight notices . improve code coverage . put work files in a temp directory in the standard location . improve error checking in C code . indent perl files with perltidy . add some comments per comments from Michael Paquier Discussion: https://postgr.es/m/ZiC3-cdFys4-6xSk@paquier.xyz --- .../t/001_test_json_parser_incremental.pl | 15 ++- .../modules/test_json_parser/t/002_inline.pl | 93 ++++++++++++++----- .../test_json_parser/t/003_test_semantic.pl | 15 ++- .../t/004_test_parser_perf.pl | 18 +++- .../test_json_parser_incremental.c | 25 +++-- .../test_json_parser/test_json_parser_perf.c | 13 ++- src/test/modules/test_json_parser/tiny.json | 1 + src/test/modules/test_json_parser/tiny.out | 1 + 8 files changed, 133 insertions(+), 48 deletions(-) diff --git a/src/test/modules/test_json_parser/t/001_test_json_parser_incremental.pl b/src/test/modules/test_json_parser/t/001_test_json_parser_incremental.pl index 24f665c021..30506be033 100644 --- a/src/test/modules/test_json_parser/t/001_test_json_parser_incremental.pl +++ b/src/test/modules/test_json_parser/t/001_test_json_parser_incremental.pl @@ -1,4 +1,9 @@ +# Copyright (c) 2024, PostgreSQL Global Development Group + +# Test the incremental (table-driven) json parser. + + use strict; use warnings; @@ -6,15 +11,19 @@ use PostgreSQL::Test::Utils; use Test::More; use FindBin; -use File::Temp qw(tempfile); - my $test_file = "$FindBin::RealBin/../tiny.json"; my $exe = "test_json_parser_incremental"; +# Test the usage error +my ($stdout, $stderr) = run_command([ $exe, "-c", 10 ]); +like($stderr, qr/Usage:/, 'error message if not enough arguments'); + +# Test that we get success for small chunk sizes from 64 down to 1. + for (my $size = 64; $size > 0; $size--) { - my ($stdout, $stderr) = run_command( [$exe, "-c", $size, $test_file] ); + ($stdout, $stderr) = run_command([ $exe, "-c", $size, $test_file ]); like($stdout, qr/SUCCESS/, "chunk size $size: test succeeds"); is($stderr, "", "chunk size $size: no error output"); diff --git a/src/test/modules/test_json_parser/t/002_inline.pl b/src/test/modules/test_json_parser/t/002_inline.pl index e63d36a6a8..b95cb6b16a 100644 --- a/src/test/modules/test_json_parser/t/002_inline.pl +++ b/src/test/modules/test_json_parser/t/002_inline.pl @@ -1,3 +1,9 @@ + +# Copyright (c) 2021-2024, PostgreSQL Global Development Group + +# Test success or failure of the incremental (table-driven) JSON parser +# for a variety of small inputs. + use strict; use warnings; @@ -6,6 +12,8 @@ use Test::More; use File::Temp qw(tempfile); +my $dir = PostgreSQL::Test::Utils::tempdir; + sub test { local $Test::Builder::Level = $Test::Builder::Level + 1; @@ -14,27 +22,32 @@ sub test my $exe = "test_json_parser_incremental"; my $chunk = length($json); + # Test the input with chunk sizes from max(input_size, 64) down to 1 + if ($chunk > 64) { $chunk = 64; } - my ($fh, $fname) = tempfile(UNLINK => 1); + my ($fh, $fname) = tempfile(DIR => $dir); print $fh "$json"; close($fh); - foreach my $size (reverse(1..$chunk)) + foreach my $size (reverse(1 .. $chunk)) { - my ($stdout, $stderr) = run_command( [$exe, "-c", $size, $fname] ); + my ($stdout, $stderr) = run_command([ $exe, "-c", $size, $fname ]); if (defined($params{error})) { - unlike($stdout, qr/SUCCESS/, "$name, chunk size $size: test fails"); - like($stderr, $params{error}, "$name, chunk size $size: correct error output"); + unlike($stdout, qr/SUCCESS/, + "$name, chunk size $size: test fails"); + like($stderr, $params{error}, + "$name, chunk size $size: correct error output"); } else { - like($stdout, qr/SUCCESS/, "$name, chunk size $size: test succeeds"); + like($stdout, qr/SUCCESS/, + "$name, chunk size $size: test succeeds"); is($stderr, "", "$name, chunk size $size: no error output"); } } @@ -58,26 +71,60 @@ test("serial escapes", '"\\\\\\\\\\\\\\\\"'); test("interrupted escapes", '"\\\\\\"\\\\\\\\\\"\\\\"'); test("whitespace", ' "" '); -test("unclosed empty object", "{", error => qr/input string ended unexpectedly/); +test("unclosed empty object", + "{", error => qr/input string ended unexpectedly/); test("bad key", "{{", error => qr/Expected string or "}", but found "\{"/); test("bad key", "{{}", error => qr/Expected string or "}", but found "\{"/); -test("numeric key", "{1234: 2}", error => qr/Expected string or "}", but found "1234"/); -test("second numeric key", '{"a": "a", 1234: 2}', error => qr/Expected string, but found "1234"/); -test("unclosed object with pair", '{"key": "value"', error => qr/input string ended unexpectedly/); -test("missing key value", '{"key": }', error => qr/Expected JSON value, but found "}"/); -test("missing colon", '{"key" 12345}', error => qr/Expected ":", but found "12345"/); -test("missing comma", '{"key": 12345 12345}', error => qr/Expected "," or "}", but found "12345"/); -test("overnested array", "[" x 6401, error => qr/maximum permitted depth is 6400/); -test("overclosed array", "[]]", error => qr/Expected end of input, but found "]"/); -test("unexpected token in array", "[ }}} ]", error => qr/Expected array element or "]", but found "}"/); +test("numeric key", "{1234: 2}", + error => qr/Expected string or "}", but found "1234"/); +test( + "second numeric key", + '{"a": "a", 1234: 2}', + error => qr/Expected string, but found "1234"/); +test( + "unclosed object with pair", + '{"key": "value"', + error => qr/input string ended unexpectedly/); +test("missing key value", + '{"key": }', error => qr/Expected JSON value, but found "}"/); +test( + "missing colon", + '{"key" 12345}', + error => qr/Expected ":", but found "12345"/); +test( + "missing comma", + '{"key": 12345 12345}', + error => qr/Expected "," or "}", but found "12345"/); +test("overnested array", + "[" x 6401, error => qr/maximum permitted depth is 6400/); +test("overclosed array", + "[]]", error => qr/Expected end of input, but found "]"/); +test("unexpected token in array", + "[ }}} ]", error => qr/Expected array element or "]", but found "}"/); test("junk punctuation", "[ ||| ]", error => qr/Token "|" is invalid/); -test("missing comma in array", "[123 123]", error => qr/Expected "," or "]", but found "123"/); +test("missing comma in array", + "[123 123]", error => qr/Expected "," or "]", but found "123"/); test("misspelled boolean", "tru", error => qr/Token "tru" is invalid/); -test("misspelled boolean in array", "[tru]", error => qr/Token "tru" is invalid/); -test("smashed top-level scalar", "12zz", error => qr/Token "12zz" is invalid/); -test("smashed scalar in array", "[12zz]", error => qr/Token "12zz" is invalid/); -test("unknown escape sequence", '"hello\vworld"', error => qr/Escape sequence "\\v" is invalid/); -test("unescaped control", "\"hello\tworld\"", error => qr/Character with value 0x09 must be escaped/); -test("incorrect escape count", '"\\\\\\\\\\\\\\"', error => qr/Token ""\\\\\\\\\\\\\\"" is invalid/); +test( + "misspelled boolean in array", + "[tru]", + error => qr/Token "tru" is invalid/); +test("smashed top-level scalar", "12zz", + error => qr/Token "12zz" is invalid/); +test( + "smashed scalar in array", + "[12zz]", + error => qr/Token "12zz" is invalid/); +test( + "unknown escape sequence", + '"hello\vworld"', + error => qr/Escape sequence "\\v" is invalid/); +test("unescaped control", + "\"hello\tworld\"", + error => qr/Character with value 0x09 must be escaped/); +test( + "incorrect escape count", + '"\\\\\\\\\\\\\\"', + error => qr/Token ""\\\\\\\\\\\\\\"" is invalid/); done_testing(); diff --git a/src/test/modules/test_json_parser/t/003_test_semantic.pl b/src/test/modules/test_json_parser/t/003_test_semantic.pl index 7b4941b7f9..7d3e07e750 100644 --- a/src/test/modules/test_json_parser/t/003_test_semantic.pl +++ b/src/test/modules/test_json_parser/t/003_test_semantic.pl @@ -1,3 +1,9 @@ + +# Copyright (c) 2021-2024, PostgreSQL Global Development Group + +# Test the incremental JSON parser with semantic routines, and compare the +# output with the expected output. + use strict; use warnings; @@ -12,17 +18,18 @@ my $test_out = "$FindBin::RealBin/../tiny.out"; my $exe = "test_json_parser_incremental"; -my ($stdout, $stderr) = run_command( [$exe, "-s", $test_file] ); +my ($stdout, $stderr) = run_command([ $exe, "-s", $test_file ]); is($stderr, "", "no error output"); -my ($fh, $fname) = tempfile(); +my $dir = PostgreSQL::Test::Utils::tempdir; +my ($fh, $fname) = tempfile(DIR => $dir); -print $fh $stdout,"\n"; +print $fh $stdout, "\n"; close($fh); -($stdout, $stderr) = run_command(["diff", "-u", $fname, $test_out]); +($stdout, $stderr) = run_command([ "diff", "-u", $fname, $test_out ]); is($stdout, "", "no output diff"); is($stderr, "", "no diff error"); diff --git a/src/test/modules/test_json_parser/t/004_test_parser_perf.pl b/src/test/modules/test_json_parser/t/004_test_parser_perf.pl index ec322c1391..c61fdf898a 100644 --- a/src/test/modules/test_json_parser/t/004_test_parser_perf.pl +++ b/src/test/modules/test_json_parser/t/004_test_parser_perf.pl @@ -1,4 +1,11 @@ +# Copyright (c) 2021-2024, PostgreSQL Global Development Group + +# Test the JSON parser performance tester. Here we are just checking that +# the performance tester can run, both with the standard parser and the +# incremental parser. An actual performance test will run with thousands +# of iterations onstead of just one. + use strict; use warnings; @@ -14,21 +21,22 @@ my $exe = "test_json_parser_perf"; my $contents = slurp_file($test_file); -my ($fh, $fname) = tempfile(UNLINK => 1); +my $dir = PostgreSQL::Test::Utils::tempdir; +my ($fh, $fname) = tempfile(DIR => $dir); # repeat the input json file 50 times in an array -print $fh, '[', $contents , ",$contents" x 49 , ']'; +print $fh, '[', $contents, ",$contents" x 49, ']'; close($fh); # but only do one iteration -my ($result) = run_log([ $exe, "1", $fname ] ); +my ($result) = run_log([ $exe, "1", $fname ]); -ok($result == 0, "perf test runs with RD parser"); +ok($result == 0, "perf test runs with recursive descent parser"); -$result = run_log([ $exe, "-i" , "1", $fname ]); +$result = run_log([ $exe, "-i", "1", $fname ]); ok($result == 0, "perf test runs with table driven parser"); diff --git a/src/test/modules/test_json_parser/test_json_parser_incremental.c b/src/test/modules/test_json_parser/test_json_parser_incremental.c index 835cdc9efd..b2195cb811 100644 --- a/src/test/modules/test_json_parser/test_json_parser_incremental.c +++ b/src/test/modules/test_json_parser/test_json_parser_incremental.c @@ -15,6 +15,9 @@ * If the "-c SIZE" option is provided, that chunk size is used instead * of the default of 60. * + * If the -s flag is given, the program does semantic processing. This should + * just mirror back the json, albeit with white space changes. + * * The argument specifies the file containing the JSON input. * *------------------------------------------------------------------------- @@ -28,6 +31,7 @@ #include #include "common/jsonapi.h" +#include "common/logging.h" #include "lib/stringinfo.h" #include "mb/pg_wchar.h" #include "pg_getopt.h" @@ -92,10 +96,7 @@ main(int argc, char **argv) case 'c': /* chunksize */ sscanf(optarg, "%zu", &chunk_size); if (chunk_size > BUFSIZE) - { - fprintf(stderr, "chunk size cannot exceed %d\n", BUFSIZE); - exit(1); - } + pg_fatal("chunk size cannot exceed %d", BUFSIZE); break; case 's': /* do semantic processing */ testsem = &sem; @@ -121,13 +122,25 @@ main(int argc, char **argv) makeJsonLexContextIncremental(&lex, PG_UTF8, need_strings); initStringInfo(&json); - json_file = fopen(testfile, "r"); - fstat(fileno(json_file), &statbuf); + if ((json_file = fopen(testfile, "r")) == NULL) + pg_fatal("error opening input: %m"); + + if (fstat(fileno(json_file), &statbuf) != 0) + pg_fatal("error statting input: %m"); + bytes_left = statbuf.st_size; for (;;) { + /* We will break when there's nothing left to read */ + + if (bytes_left < chunk_size) + chunk_size = bytes_left; + n_read = fread(buff, 1, chunk_size, json_file); + if (n_read < chunk_size) + pg_fatal("error reading input file: %d", ferror(json_file)); + appendBinaryStringInfo(&json, buff, n_read); /* diff --git a/src/test/modules/test_json_parser/test_json_parser_perf.c b/src/test/modules/test_json_parser/test_json_parser_perf.c index c463046848..f5c0e8dd9a 100644 --- a/src/test/modules/test_json_parser/test_json_parser_perf.c +++ b/src/test/modules/test_json_parser/test_json_parser_perf.c @@ -23,6 +23,7 @@ #include "postgres_fe.h" #include "common/jsonapi.h" +#include "common/logging.h" #include "lib/stringinfo.h" #include "mb/pg_wchar.h" #include @@ -52,7 +53,9 @@ main(int argc, char **argv) sscanf(argv[1], "%d", &iter); - json_file = fopen(argv[2], "r"); + if ((json_file = fopen(argv[2], "r")) == NULL) + pg_fatal("Could not open input file '%s': %m", argv[2]); + while ((n_read = fread(buff, 1, 6000, json_file)) > 0) { appendBinaryStringInfo(&json, buff, n_read); @@ -76,12 +79,8 @@ main(int argc, char **argv) freeJsonLexContext(lex); } if (result != JSON_SUCCESS) - { - fprintf(stderr, - "unexpected result %d (expecting %d) on parse\n", - result, JSON_SUCCESS); - exit(1); - } + pg_fatal("unexpected result %d (expecting %d) on parse", + result, JSON_SUCCESS); } exit(0); } diff --git a/src/test/modules/test_json_parser/tiny.json b/src/test/modules/test_json_parser/tiny.json index 5f9c9346a1..361f692236 100644 --- a/src/test/modules/test_json_parser/tiny.json +++ b/src/test/modules/test_json_parser/tiny.json @@ -1,4 +1,5 @@ [{ +"teststring": "\u0001\b\f\n\r\t\\blurfl", "updated": "Tue, 08 Sep 2009 23:28:55 +0000", "links": [{"href" : "http://example.com/broadway/", diff --git a/src/test/modules/test_json_parser/tiny.out b/src/test/modules/test_json_parser/tiny.out index 4d82895c48..4608ec44cd 100644 --- a/src/test/modules/test_json_parser/tiny.out +++ b/src/test/modules/test_json_parser/tiny.out @@ -1,5 +1,6 @@ [ { +"teststring": "\u0001\b\f\n\r\t\\blurfl", "updated": "Tue, 08 Sep 2009 23:28:55 +0000", "links": [ {