From 02444f07be3afa3e7177660d7633aa30542e9833 Mon Sep 17 00:00:00 2001 From: Dmitriy Tverdiakov Date: Wed, 28 Jun 2023 11:16:56 +0100 Subject: [PATCH 01/13] Add tx termination tests These tests verify transaction and its results handling when the transaction is terminated by a failure. --- .../tx_run/scripts/tx_error_on_pull.script | 19 ++ ...cessful_and_failing_on_pull_streams.script | 29 +++ ...ccessful_and_failing_on_run_streams.script | 27 +++ tests/stub/tx_run/test_tx_run.py | 189 ++++++++++++++++++ 4 files changed, 264 insertions(+) create mode 100644 tests/stub/tx_run/scripts/tx_error_on_pull.script create mode 100644 tests/stub/tx_run/scripts/tx_successful_and_failing_on_pull_streams.script create mode 100644 tests/stub/tx_run/scripts/tx_successful_and_failing_on_run_streams.script diff --git a/tests/stub/tx_run/scripts/tx_error_on_pull.script b/tests/stub/tx_run/scripts/tx_error_on_pull.script new file mode 100644 index 000000000..acd8bf4de --- /dev/null +++ b/tests/stub/tx_run/scripts/tx_error_on_pull.script @@ -0,0 +1,19 @@ +!: BOLT 5.3 + +A: HELLO {"{}": "*"} +A: LOGON {"{}": "*"} +*: RESET +C: BEGIN {"{}": "*"} +S: SUCCESS {} +C: RUN "failing on pull" {"{}": "*"} {"{}": "*"} +S: SUCCESS {"fields": ["n"], "qid": 1} +{{ + C: PULL {"n": {"Z": "*"}, "[qid]": -1} +---- + C: PULL {"n": {"Z": "*"}, "qid": 1} +}} +S: RECORD [1] + RECORD [2] + FAILURE {"code": "Neo.ClientError.MadeUp.Code", "message": "message"} +*: RESET +?: GOODBYE diff --git a/tests/stub/tx_run/scripts/tx_successful_and_failing_on_pull_streams.script b/tests/stub/tx_run/scripts/tx_successful_and_failing_on_pull_streams.script new file mode 100644 index 000000000..5fa5f9a75 --- /dev/null +++ b/tests/stub/tx_run/scripts/tx_successful_and_failing_on_pull_streams.script @@ -0,0 +1,29 @@ +!: BOLT 5.3 + +A: HELLO {"{}": "*"} +A: LOGON {"{}": "*"} +*: RESET +C: BEGIN {"{}": "*"} +S: SUCCESS {} +C: RUN "RETURN 1 AS n" {"{}": "*"} {"{}": "*"} +S: SUCCESS {"fields": ["n"], "qid": 1} +{? + {{ + C: PULL {"n": {"Z": "*"}, "[qid]": -1} + ---- + C: PULL {"n": {"Z": "*"}, "qid": 1} + }} + S: RECORD [1] + RECORD [2] + SUCCESS {"has_more": true, "type": "r"} +?} +C: RUN "failing on pull" {"{}": "*"} {"{}": "*"} +S: SUCCESS {"fields": ["n"], "qid": 2} +{{ + C: PULL {"n": {"Z": "*"}, "[qid]": -1} +---- + C: PULL {"n": {"Z": "*"}, "qid": 2} +}} +S: FAILURE {"code": "Neo.ClientError.MadeUp.Code", "message": "message"} +*: RESET +?: GOODBYE diff --git a/tests/stub/tx_run/scripts/tx_successful_and_failing_on_run_streams.script b/tests/stub/tx_run/scripts/tx_successful_and_failing_on_run_streams.script new file mode 100644 index 000000000..d9c6ead28 --- /dev/null +++ b/tests/stub/tx_run/scripts/tx_successful_and_failing_on_run_streams.script @@ -0,0 +1,27 @@ +!: BOLT 5.3 + +A: HELLO {"{}": "*"} +A: LOGON {"{}": "*"} +*: RESET +C: BEGIN {"{}": "*"} +S: SUCCESS {} +C: RUN "RETURN 1 AS n" {"{}": "*"} {"{}": "*"} +S: SUCCESS {"fields": ["n"], "qid": 1} +{? + {{ + C: PULL {"n": {"Z": "*"}, "[qid]": -1} + ---- + C: PULL {"n": {"Z": "*"}, "qid": 1} + }} + S: RECORD [1] + RECORD [2] + SUCCESS {"has_more": true, "type": "r"} +?} +C: RUN "invalid" {"{}": "*"} {"{}": "*"} +S: FAILURE {"code": "Neo.ClientError.Statement.SyntaxError", "message": "Invalid input"} +{? + C: PULL {"n": {"Z": "*"}, "[qid]": -1} + S: IGNORED +?} +*: RESET +?: GOODBYE diff --git a/tests/stub/tx_run/test_tx_run.py b/tests/stub/tx_run/test_tx_run.py index 5dafd6808..8f9976a0a 100644 --- a/tests/stub/tx_run/test_tx_run.py +++ b/tests/stub/tx_run/test_tx_run.py @@ -258,3 +258,192 @@ def test_failed_tx_run_allows_rollback(self): def test_failed_tx_run_allows_skipping_rollback(self): self._test_failed_tx_run(rollback=False) + + def test_should_prevent_pull_after_transaction_termination_on_run(self): + # TODO remove this block once all languages work + if get_driver_name() in ["go", "javascript", "dotnet", "python"]: + self.skipTest("requires investigation") + + def _test(): + self._create_direct_driver() + script = "tx_successful_and_failing_on_run_streams.script" + self._server1.start(path=self.script_path(script)) + self._session = self._driver.session("r", fetch_size=2) + tx = self._session.begin_transaction() + res = tx.run("RETURN 1 AS n") + + # begin another stream that fails on RUN + with self.assertRaises(types.DriverError) as exc: + failed_res = tx.run("invalid") + if get_driver_name() in ["javascript"]: + failed_res.next() + self.assertEqual(exc.exception.code, + "Neo.ClientError.Statement.SyntaxError") + self._assert_is_client_exception(exc) + + # while already buffered records may be accessible, there must be + # no further PULL and an exception must be raised + with self.assertRaises(types.DriverError) as exc: + if iterate == "true": + for _i in range(0, 3): + res.next() + else: + fetch_all = types.Feature.OPT_RESULT_LIST_FETCH_ALL + if self.driver_supports_features(fetch_all): + res.list() + else: + # if the fetch all is not supported, only explicit + # iteration can be tested + list(res) + # the streaming result cursors surface the termination exception + self.assertEqual(exc.exception.code, + "Neo.ClientError.Statement.SyntaxError") + self._assert_is_client_exception(exc) + + tx.close() + self._session.close() + self._session = None + self._server1.done() + for iterate in ["true", "false"]: + with self.subTest(iterate=iterate): + _test() + self._server1.reset() + + def test_should_prevent_discard_after_transaction_termination_on_run(self): + # TODO remove this block once all languages work + if get_driver_name() in ["go", "javascript", "dotnet", "python"]: + self.skipTest("requires investigation") + + self._create_direct_driver() + script = "tx_successful_and_failing_on_run_streams.script" + self._server1.start(path=self.script_path(script)) + self._session = self._driver.session("r", fetch_size=2) + tx = self._session.begin_transaction() + res = tx.run("RETURN 1 AS n") + + # begin another stream that fails on RUN + with self.assertRaises(types.DriverError) as exc: + failed_res = tx.run("invalid") + if get_driver_name() in ["javascript"]: + failed_res.next() + self.assertEqual(exc.exception.code, + "Neo.ClientError.Statement.SyntaxError") + self._assert_is_client_exception(exc) + + with self.assertRaises(types.DriverError) as exc: + res.consume() + # the streaming result cursors surface the termination exception + self.assertEqual(exc.exception.code, + "Neo.ClientError.Statement.SyntaxError") + self._assert_is_client_exception(exc) + + tx.close() + self._session.close() + self._session = None + self._server1.done() + + def test_should_prevent_run_after_transaction_termination_on_pull(self): + # TODO remove this block once all languages work + if get_driver_name() in ["go", "javascript", "dotnet", "python"]: + self.skipTest("requires investigation") + + def _test(): + self._create_direct_driver() + script = "tx_error_on_pull.script" + self._server1.start(path=self.script_path(script)) + self._session = self._driver.session("r", fetch_size=2) + tx = self._session.begin_transaction() + res = tx.run("failing on pull") + + # streaming fails on PULL + with self.assertRaises(types.DriverError) as exc: + if iterate == "true": + for _i in range(0, 3): + res.next() + else: + fetch_all = types.Feature.OPT_RESULT_LIST_FETCH_ALL + if self.driver_supports_features(fetch_all): + res.list() + else: + # if the fetch all is not supported, only explicit + # iteration can be tested + list(res) + self.assertEqual(exc.exception.code, + "Neo.ClientError.MadeUp.Code") + self._assert_is_client_exception(exc) + + with self.assertRaises(types.DriverError) as exc: + tx.run("invalid") + # initiating actions of transaction throw terminated + self._assert_is_tx_terminated_exception(exc) + + tx.close() + self._session.close() + self._session = None + self._server1.done() + for iterate in ["true", "false"]: + with self.subTest(iterate=iterate): + _test() + self._server1.reset() + + def test_should_prevent_pull_after_transaction_termination_on_pull(self): + # TODO remove this block once all languages work + if get_driver_name() in ["go", "javascript", "dotnet", "python"]: + self.skipTest("requires investigation") + + def _test(): + self._create_direct_driver() + script = "tx_successful_and_failing_on_pull_streams.script" + self._server1.start(path=self.script_path(script)) + self._session = self._driver.session("r", fetch_size=2) + tx = self._session.begin_transaction() + res = tx.run("RETURN 1 AS n") + + # fail on PULL + with self.assertRaises(types.DriverError) as exc: + failed_res = tx.run("failing on pull") + failed_res.next() + self.assertEqual(exc.exception.code, + "Neo.ClientError.MadeUp.Code") + self._assert_is_client_exception(exc) + + # fail on iteration + with self.assertRaises(types.DriverError): + if iterate == "true": + for _i in range(0, 3): + res.next() + else: + fetch_all = types.Feature.OPT_RESULT_LIST_FETCH_ALL + if self.driver_supports_features(fetch_all): + res.list() + else: + # if the fetch all is not supported, only explicit + # iteration can be tested + list(res) + # the streaming result cursors surface the termination exception + self.assertEqual(exc.exception.code, + "Neo.ClientError.MadeUp.Code") + self._assert_is_client_exception(exc) + + tx.close() + self._session.close() + self._session = None + self._server1.done() + for iterate in ["true", "false"]: + with self.subTest(iterate=iterate): + _test() + self._server1.reset() + + def _assert_is_client_exception(self, e): + if get_driver_name() in ["java"]: + self.assertEqual( + "org.neo4j.driver.exceptions.ClientException", + e.exception.errorType + ) + + def _assert_is_tx_terminated_exception(self, e): + if get_driver_name() in ["java"]: + self.assertEqual( + "org.neo4j.driver.exceptions.TransactionTerminatedException", + e.exception.errorType + ) From ccdc377efd5c7106f5b9142a1a79ab18d57a6ce7 Mon Sep 17 00:00:00 2001 From: Dmitriy Tverdiakov Date: Mon, 3 Jul 2023 17:27:06 +0100 Subject: [PATCH 02/13] Remove skips on Testkit side --- tests/stub/tx_run/test_tx_run.py | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/tests/stub/tx_run/test_tx_run.py b/tests/stub/tx_run/test_tx_run.py index 8f9976a0a..e150abf97 100644 --- a/tests/stub/tx_run/test_tx_run.py +++ b/tests/stub/tx_run/test_tx_run.py @@ -260,10 +260,6 @@ def test_failed_tx_run_allows_skipping_rollback(self): self._test_failed_tx_run(rollback=False) def test_should_prevent_pull_after_transaction_termination_on_run(self): - # TODO remove this block once all languages work - if get_driver_name() in ["go", "javascript", "dotnet", "python"]: - self.skipTest("requires investigation") - def _test(): self._create_direct_driver() script = "tx_successful_and_failing_on_run_streams.script" @@ -310,10 +306,6 @@ def _test(): self._server1.reset() def test_should_prevent_discard_after_transaction_termination_on_run(self): - # TODO remove this block once all languages work - if get_driver_name() in ["go", "javascript", "dotnet", "python"]: - self.skipTest("requires investigation") - self._create_direct_driver() script = "tx_successful_and_failing_on_run_streams.script" self._server1.start(path=self.script_path(script)) @@ -343,10 +335,6 @@ def test_should_prevent_discard_after_transaction_termination_on_run(self): self._server1.done() def test_should_prevent_run_after_transaction_termination_on_pull(self): - # TODO remove this block once all languages work - if get_driver_name() in ["go", "javascript", "dotnet", "python"]: - self.skipTest("requires investigation") - def _test(): self._create_direct_driver() script = "tx_error_on_pull.script" @@ -387,10 +375,6 @@ def _test(): self._server1.reset() def test_should_prevent_pull_after_transaction_termination_on_pull(self): - # TODO remove this block once all languages work - if get_driver_name() in ["go", "javascript", "dotnet", "python"]: - self.skipTest("requires investigation") - def _test(): self._create_direct_driver() script = "tx_successful_and_failing_on_pull_streams.script" From ad7ad51311d1fa82672eb805f21f208122d4c896 Mon Sep 17 00:00:00 2001 From: Dmitriy Tverdiakov Date: Mon, 3 Jul 2023 17:30:54 +0100 Subject: [PATCH 03/13] Update assertion for Java driver in test_timeout_unmanaged_tx_should_fail_subsequent_usage_after_timeout --- .../configuration_hints/test_connection_recv_timeout_seconds.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/stub/configuration_hints/test_connection_recv_timeout_seconds.py b/tests/stub/configuration_hints/test_connection_recv_timeout_seconds.py index d0b375484..7e48ad987 100644 --- a/tests/stub/configuration_hints/test_connection_recv_timeout_seconds.py +++ b/tests/stub/configuration_hints/test_connection_recv_timeout_seconds.py @@ -68,7 +68,7 @@ def _assert_is_timeout_exception(self, e): def _assert_is_client_exception(self, e): if get_driver_name() in ["java"]: self.assertEqual( - "org.neo4j.driver.exceptions.ClientException", + "org.neo4j.driver.exceptions.TransactionTerminatedException", e.errorType ) elif get_driver_name() in ["ruby"]: From d25285e55dbf7c8c507d41771468034332a67953 Mon Sep 17 00:00:00 2001 From: Dmitriy Tverdiakov Date: Mon, 3 Jul 2023 19:37:11 +0100 Subject: [PATCH 04/13] Update script names --- ...ams.script => tx_res0_success_res1_error_on_pull.script} | 0 ...eams.script => tx_res0_success_res1_error_on_run.script} | 0 tests/stub/tx_run/test_tx_run.py | 6 +++--- 3 files changed, 3 insertions(+), 3 deletions(-) rename tests/stub/tx_run/scripts/{tx_successful_and_failing_on_pull_streams.script => tx_res0_success_res1_error_on_pull.script} (100%) rename tests/stub/tx_run/scripts/{tx_successful_and_failing_on_run_streams.script => tx_res0_success_res1_error_on_run.script} (100%) diff --git a/tests/stub/tx_run/scripts/tx_successful_and_failing_on_pull_streams.script b/tests/stub/tx_run/scripts/tx_res0_success_res1_error_on_pull.script similarity index 100% rename from tests/stub/tx_run/scripts/tx_successful_and_failing_on_pull_streams.script rename to tests/stub/tx_run/scripts/tx_res0_success_res1_error_on_pull.script diff --git a/tests/stub/tx_run/scripts/tx_successful_and_failing_on_run_streams.script b/tests/stub/tx_run/scripts/tx_res0_success_res1_error_on_run.script similarity index 100% rename from tests/stub/tx_run/scripts/tx_successful_and_failing_on_run_streams.script rename to tests/stub/tx_run/scripts/tx_res0_success_res1_error_on_run.script diff --git a/tests/stub/tx_run/test_tx_run.py b/tests/stub/tx_run/test_tx_run.py index e150abf97..c7c1550d6 100644 --- a/tests/stub/tx_run/test_tx_run.py +++ b/tests/stub/tx_run/test_tx_run.py @@ -262,7 +262,7 @@ def test_failed_tx_run_allows_skipping_rollback(self): def test_should_prevent_pull_after_transaction_termination_on_run(self): def _test(): self._create_direct_driver() - script = "tx_successful_and_failing_on_run_streams.script" + script = "tx_res0_success_res1_error_on_run.script" self._server1.start(path=self.script_path(script)) self._session = self._driver.session("r", fetch_size=2) tx = self._session.begin_transaction() @@ -307,7 +307,7 @@ def _test(): def test_should_prevent_discard_after_transaction_termination_on_run(self): self._create_direct_driver() - script = "tx_successful_and_failing_on_run_streams.script" + script = "tx_res0_success_res1_error_on_run.script" self._server1.start(path=self.script_path(script)) self._session = self._driver.session("r", fetch_size=2) tx = self._session.begin_transaction() @@ -377,7 +377,7 @@ def _test(): def test_should_prevent_pull_after_transaction_termination_on_pull(self): def _test(): self._create_direct_driver() - script = "tx_successful_and_failing_on_pull_streams.script" + script = "tx_res0_success_res1_error_on_pull.script" self._server1.start(path=self.script_path(script)) self._session = self._driver.session("r", fetch_size=2) tx = self._session.begin_transaction() From c7feb5a1fa18ee3569b0d334279616238b17527c Mon Sep 17 00:00:00 2001 From: Dmitriy Tverdiakov Date: Mon, 3 Jul 2023 19:46:35 +0100 Subject: [PATCH 05/13] Update test names and comments --- tests/stub/tx_run/test_tx_run.py | 38 ++++++++++++++++---------------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/tests/stub/tx_run/test_tx_run.py b/tests/stub/tx_run/test_tx_run.py index c7c1550d6..e45d9f520 100644 --- a/tests/stub/tx_run/test_tx_run.py +++ b/tests/stub/tx_run/test_tx_run.py @@ -259,7 +259,7 @@ def test_failed_tx_run_allows_rollback(self): def test_failed_tx_run_allows_skipping_rollback(self): self._test_failed_tx_run(rollback=False) - def test_should_prevent_pull_after_transaction_termination_on_run(self): + def test_should_prevent_pull_after_tx_termination_on_run(self): def _test(): self._create_direct_driver() script = "tx_res0_success_res1_error_on_run.script" @@ -268,7 +268,7 @@ def _test(): tx = self._session.begin_transaction() res = tx.run("RETURN 1 AS n") - # begin another stream that fails on RUN + # initiate another stream that fails on RUN with self.assertRaises(types.DriverError) as exc: failed_res = tx.run("invalid") if get_driver_name() in ["javascript"]: @@ -277,8 +277,7 @@ def _test(): "Neo.ClientError.Statement.SyntaxError") self._assert_is_client_exception(exc) - # while already buffered records may be accessible, there must be - # no further PULL and an exception must be raised + # there must be no further PULL and an exception must be raised with self.assertRaises(types.DriverError) as exc: if iterate == "true": for _i in range(0, 3): @@ -288,10 +287,10 @@ def _test(): if self.driver_supports_features(fetch_all): res.list() else: - # if the fetch all is not supported, only explicit - # iteration can be tested + # only explicit iteration is tested if the fetch all is + # not supported list(res) - # the streaming result cursors surface the termination exception + # the streaming result surfaces the termination exception self.assertEqual(exc.exception.code, "Neo.ClientError.Statement.SyntaxError") self._assert_is_client_exception(exc) @@ -305,7 +304,7 @@ def _test(): _test() self._server1.reset() - def test_should_prevent_discard_after_transaction_termination_on_run(self): + def test_should_prevent_discard_after_tx_termination_on_run(self): self._create_direct_driver() script = "tx_res0_success_res1_error_on_run.script" self._server1.start(path=self.script_path(script)) @@ -313,7 +312,7 @@ def test_should_prevent_discard_after_transaction_termination_on_run(self): tx = self._session.begin_transaction() res = tx.run("RETURN 1 AS n") - # begin another stream that fails on RUN + # initiate another stream that fails on RUN with self.assertRaises(types.DriverError) as exc: failed_res = tx.run("invalid") if get_driver_name() in ["javascript"]: @@ -324,7 +323,7 @@ def test_should_prevent_discard_after_transaction_termination_on_run(self): with self.assertRaises(types.DriverError) as exc: res.consume() - # the streaming result cursors surface the termination exception + # the streaming result surfaces the termination exception self.assertEqual(exc.exception.code, "Neo.ClientError.Statement.SyntaxError") self._assert_is_client_exception(exc) @@ -334,7 +333,7 @@ def test_should_prevent_discard_after_transaction_termination_on_run(self): self._session = None self._server1.done() - def test_should_prevent_run_after_transaction_termination_on_pull(self): + def test_should_prevent_run_after_tx_termination_on_pull(self): def _test(): self._create_direct_driver() script = "tx_error_on_pull.script" @@ -343,7 +342,7 @@ def _test(): tx = self._session.begin_transaction() res = tx.run("failing on pull") - # streaming fails on PULL + # res fails on PULL with self.assertRaises(types.DriverError) as exc: if iterate == "true": for _i in range(0, 3): @@ -353,8 +352,8 @@ def _test(): if self.driver_supports_features(fetch_all): res.list() else: - # if the fetch all is not supported, only explicit - # iteration can be tested + # only explicit iteration is tested if the fetch all is + # not supported list(res) self.assertEqual(exc.exception.code, "Neo.ClientError.MadeUp.Code") @@ -362,7 +361,8 @@ def _test(): with self.assertRaises(types.DriverError) as exc: tx.run("invalid") - # initiating actions of transaction throw terminated + # new actions on the transaction result in a tx terminated + # exception, a subclass of the client exception self._assert_is_tx_terminated_exception(exc) tx.close() @@ -374,7 +374,7 @@ def _test(): _test() self._server1.reset() - def test_should_prevent_pull_after_transaction_termination_on_pull(self): + def test_should_prevent_pull_after_tx_termination_on_pull(self): def _test(): self._create_direct_driver() script = "tx_res0_success_res1_error_on_pull.script" @@ -383,7 +383,7 @@ def _test(): tx = self._session.begin_transaction() res = tx.run("RETURN 1 AS n") - # fail on PULL + # initiate another stream that fails on PULL with self.assertRaises(types.DriverError) as exc: failed_res = tx.run("failing on pull") failed_res.next() @@ -391,7 +391,7 @@ def _test(): "Neo.ClientError.MadeUp.Code") self._assert_is_client_exception(exc) - # fail on iteration + # there must be no further PULL and an exception must be raised with self.assertRaises(types.DriverError): if iterate == "true": for _i in range(0, 3): @@ -404,7 +404,7 @@ def _test(): # if the fetch all is not supported, only explicit # iteration can be tested list(res) - # the streaming result cursors surface the termination exception + # the streaming result surfaces the termination exception self.assertEqual(exc.exception.code, "Neo.ClientError.MadeUp.Code") self._assert_is_client_exception(exc) From e1dee24c4b1c9c49e6d2540ef5807d00e6c629f0 Mon Sep 17 00:00:00 2001 From: Dmitriy Tverdiakov Date: Tue, 4 Jul 2023 10:27:38 +0100 Subject: [PATCH 06/13] Expect at least one RESET after a FAILURE --- tests/stub/tx_run/scripts/tx_error_on_pull.script | 2 +- .../tx_run/scripts/tx_res0_success_res1_error_on_pull.script | 2 +- .../tx_run/scripts/tx_res0_success_res1_error_on_run.script | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/stub/tx_run/scripts/tx_error_on_pull.script b/tests/stub/tx_run/scripts/tx_error_on_pull.script index acd8bf4de..57840f45d 100644 --- a/tests/stub/tx_run/scripts/tx_error_on_pull.script +++ b/tests/stub/tx_run/scripts/tx_error_on_pull.script @@ -15,5 +15,5 @@ S: SUCCESS {"fields": ["n"], "qid": 1} S: RECORD [1] RECORD [2] FAILURE {"code": "Neo.ClientError.MadeUp.Code", "message": "message"} -*: RESET ++: RESET ?: GOODBYE diff --git a/tests/stub/tx_run/scripts/tx_res0_success_res1_error_on_pull.script b/tests/stub/tx_run/scripts/tx_res0_success_res1_error_on_pull.script index 5fa5f9a75..0381b622c 100644 --- a/tests/stub/tx_run/scripts/tx_res0_success_res1_error_on_pull.script +++ b/tests/stub/tx_run/scripts/tx_res0_success_res1_error_on_pull.script @@ -25,5 +25,5 @@ S: SUCCESS {"fields": ["n"], "qid": 2} C: PULL {"n": {"Z": "*"}, "qid": 2} }} S: FAILURE {"code": "Neo.ClientError.MadeUp.Code", "message": "message"} -*: RESET ++: RESET ?: GOODBYE diff --git a/tests/stub/tx_run/scripts/tx_res0_success_res1_error_on_run.script b/tests/stub/tx_run/scripts/tx_res0_success_res1_error_on_run.script index d9c6ead28..5648eb56e 100644 --- a/tests/stub/tx_run/scripts/tx_res0_success_res1_error_on_run.script +++ b/tests/stub/tx_run/scripts/tx_res0_success_res1_error_on_run.script @@ -23,5 +23,5 @@ S: FAILURE {"code": "Neo.ClientError.Statement.SyntaxError", "message": "Invalid C: PULL {"n": {"Z": "*"}, "[qid]": -1} S: IGNORED ?} -*: RESET ++: RESET ?: GOODBYE From 6b5bf154a253b0db4be9679b4b0aeafb656c317d Mon Sep 17 00:00:00 2001 From: Dmitriy Tverdiakov Date: Tue, 4 Jul 2023 10:30:57 +0100 Subject: [PATCH 07/13] Remove redundant type in SUCCESS --- .../tx_run/scripts/tx_res0_success_res1_error_on_pull.script | 2 +- .../tx_run/scripts/tx_res0_success_res1_error_on_run.script | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/stub/tx_run/scripts/tx_res0_success_res1_error_on_pull.script b/tests/stub/tx_run/scripts/tx_res0_success_res1_error_on_pull.script index 0381b622c..3de088016 100644 --- a/tests/stub/tx_run/scripts/tx_res0_success_res1_error_on_pull.script +++ b/tests/stub/tx_run/scripts/tx_res0_success_res1_error_on_pull.script @@ -15,7 +15,7 @@ S: SUCCESS {"fields": ["n"], "qid": 1} }} S: RECORD [1] RECORD [2] - SUCCESS {"has_more": true, "type": "r"} + SUCCESS {"has_more": true} ?} C: RUN "failing on pull" {"{}": "*"} {"{}": "*"} S: SUCCESS {"fields": ["n"], "qid": 2} diff --git a/tests/stub/tx_run/scripts/tx_res0_success_res1_error_on_run.script b/tests/stub/tx_run/scripts/tx_res0_success_res1_error_on_run.script index 5648eb56e..552fb1c57 100644 --- a/tests/stub/tx_run/scripts/tx_res0_success_res1_error_on_run.script +++ b/tests/stub/tx_run/scripts/tx_res0_success_res1_error_on_run.script @@ -15,7 +15,7 @@ S: SUCCESS {"fields": ["n"], "qid": 1} }} S: RECORD [1] RECORD [2] - SUCCESS {"has_more": true, "type": "r"} + SUCCESS {"has_more": true} ?} C: RUN "invalid" {"{}": "*"} {"{}": "*"} S: FAILURE {"code": "Neo.ClientError.Statement.SyntaxError", "message": "Invalid input"} From 88ad394a3cd49612e11b7a143a7d9b3022d6b6bd Mon Sep 17 00:00:00 2001 From: Dmitriy Tverdiakov Date: Tue, 4 Jul 2023 10:34:07 +0100 Subject: [PATCH 08/13] Remove JS conditions --- tests/stub/tx_run/test_tx_run.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/tests/stub/tx_run/test_tx_run.py b/tests/stub/tx_run/test_tx_run.py index e45d9f520..8f6b39ce8 100644 --- a/tests/stub/tx_run/test_tx_run.py +++ b/tests/stub/tx_run/test_tx_run.py @@ -271,8 +271,6 @@ def _test(): # initiate another stream that fails on RUN with self.assertRaises(types.DriverError) as exc: failed_res = tx.run("invalid") - if get_driver_name() in ["javascript"]: - failed_res.next() self.assertEqual(exc.exception.code, "Neo.ClientError.Statement.SyntaxError") self._assert_is_client_exception(exc) @@ -315,8 +313,6 @@ def test_should_prevent_discard_after_tx_termination_on_run(self): # initiate another stream that fails on RUN with self.assertRaises(types.DriverError) as exc: failed_res = tx.run("invalid") - if get_driver_name() in ["javascript"]: - failed_res.next() self.assertEqual(exc.exception.code, "Neo.ClientError.Statement.SyntaxError") self._assert_is_client_exception(exc) From 922c56acb220ab31cd2bc8657542759396600f98 Mon Sep 17 00:00:00 2001 From: Dmitriy Tverdiakov Date: Tue, 4 Jul 2023 10:37:10 +0100 Subject: [PATCH 09/13] Update comments --- tests/stub/tx_run/test_tx_run.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/stub/tx_run/test_tx_run.py b/tests/stub/tx_run/test_tx_run.py index 8f6b39ce8..180476a76 100644 --- a/tests/stub/tx_run/test_tx_run.py +++ b/tests/stub/tx_run/test_tx_run.py @@ -285,7 +285,7 @@ def _test(): if self.driver_supports_features(fetch_all): res.list() else: - # only explicit iteration is tested if the fetch all is + # only explicit iteration is tested if fetch all is # not supported list(res) # the streaming result surfaces the termination exception @@ -348,7 +348,7 @@ def _test(): if self.driver_supports_features(fetch_all): res.list() else: - # only explicit iteration is tested if the fetch all is + # only explicit iteration is tested if fetch all is # not supported list(res) self.assertEqual(exc.exception.code, @@ -397,8 +397,8 @@ def _test(): if self.driver_supports_features(fetch_all): res.list() else: - # if the fetch all is not supported, only explicit - # iteration can be tested + # only explicit iteration is tested if fetch all is + # not supported list(res) # the streaming result surfaces the termination exception self.assertEqual(exc.exception.code, From f14ab70d5b34c06511f2213019ad36ca522e6719 Mon Sep 17 00:00:00 2001 From: Dmitriy Tverdiakov Date: Tue, 4 Jul 2023 10:37:57 +0100 Subject: [PATCH 10/13] Formatting --- tests/stub/tx_run/test_tx_run.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/stub/tx_run/test_tx_run.py b/tests/stub/tx_run/test_tx_run.py index 180476a76..81d1b737c 100644 --- a/tests/stub/tx_run/test_tx_run.py +++ b/tests/stub/tx_run/test_tx_run.py @@ -297,6 +297,7 @@ def _test(): self._session.close() self._session = None self._server1.done() + for iterate in ["true", "false"]: with self.subTest(iterate=iterate): _test() @@ -365,6 +366,7 @@ def _test(): self._session.close() self._session = None self._server1.done() + for iterate in ["true", "false"]: with self.subTest(iterate=iterate): _test() @@ -409,6 +411,7 @@ def _test(): self._session.close() self._session = None self._server1.done() + for iterate in ["true", "false"]: with self.subTest(iterate=iterate): _test() From 8abf4949c5178517fa4900482aad425b4e04adb6 Mon Sep 17 00:00:00 2001 From: Dmitriy Tverdiakov Date: Tue, 4 Jul 2023 10:43:58 +0100 Subject: [PATCH 11/13] Update error assertions --- .../test_connection_recv_timeout_seconds.py | 11 +++++++---- tests/stub/tx_run/test_tx_run.py | 12 +++++++++--- 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/tests/stub/configuration_hints/test_connection_recv_timeout_seconds.py b/tests/stub/configuration_hints/test_connection_recv_timeout_seconds.py index 7e48ad987..8f1e42c5a 100644 --- a/tests/stub/configuration_hints/test_connection_recv_timeout_seconds.py +++ b/tests/stub/configuration_hints/test_connection_recv_timeout_seconds.py @@ -65,17 +65,20 @@ def _assert_is_timeout_exception(self, e): self.assertIn("ConnectionReadTimeoutError", e.errorType) - def _assert_is_client_exception(self, e): - if get_driver_name() in ["java"]: + def _assert_is_client_or_tx_terminated_exception(self, e): + driver = get_driver_name() + if driver in ["java"]: self.assertEqual( "org.neo4j.driver.exceptions.TransactionTerminatedException", e.errorType ) - elif get_driver_name() in ["ruby"]: + elif driver in ["ruby"]: self.assertEqual( "Neo4j::Driver::Exceptions::ClientException", e.errorType ) + else: + self.fail("no error mapping is defined for %s driver" % driver) def _on_failed_retry_assertions(self): pass @@ -150,7 +153,7 @@ def test_timeout_unmanaged_tx_should_fail_subsequent_usage_after_timeout( with self.assertRaises(StubServerUncleanExitError): self._server.done() self._assert_is_timeout_exception(first_run_error.exception) - self._assert_is_client_exception(second_run_error.exception) + self._assert_is_client_or_tx_terminated_exception(second_run_error.exception) self.assertEqual(self._server.count_responses(""), 1) self.assertEqual(self._server.count_responses(""), 1) self.assertEqual(self._server.count_requests('RUN "timeout"'), 1) diff --git a/tests/stub/tx_run/test_tx_run.py b/tests/stub/tx_run/test_tx_run.py index 81d1b737c..309446189 100644 --- a/tests/stub/tx_run/test_tx_run.py +++ b/tests/stub/tx_run/test_tx_run.py @@ -297,7 +297,7 @@ def _test(): self._session.close() self._session = None self._server1.done() - + for iterate in ["true", "false"]: with self.subTest(iterate=iterate): _test() @@ -418,15 +418,21 @@ def _test(): self._server1.reset() def _assert_is_client_exception(self, e): - if get_driver_name() in ["java"]: + driver = get_driver_name() + if driver in ["java"]: self.assertEqual( "org.neo4j.driver.exceptions.ClientException", e.exception.errorType ) + else: + self.fail("no error mapping is defined for %s driver" % driver) def _assert_is_tx_terminated_exception(self, e): - if get_driver_name() in ["java"]: + driver = get_driver_name() + if driver in ["java"]: self.assertEqual( "org.neo4j.driver.exceptions.TransactionTerminatedException", e.exception.errorType ) + else: + self.fail("no error mapping is defined for %s driver" % driver) From 910eb4e14adea0a3035b809f73f5d5d67af01055 Mon Sep 17 00:00:00 2001 From: Dmitriy Tverdiakov Date: Tue, 4 Jul 2023 10:50:20 +0100 Subject: [PATCH 12/13] Add test_should_prevent_run_after_tx_termination_on_run --- tests/stub/tx_run/test_tx_run.py | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/tests/stub/tx_run/test_tx_run.py b/tests/stub/tx_run/test_tx_run.py index 309446189..7f8be6787 100644 --- a/tests/stub/tx_run/test_tx_run.py +++ b/tests/stub/tx_run/test_tx_run.py @@ -330,6 +330,29 @@ def test_should_prevent_discard_after_tx_termination_on_run(self): self._session = None self._server1.done() + def test_should_prevent_run_after_tx_termination_on_run(self): + self._create_direct_driver() + script = "tx_error_on_run.script" + self._server1.start(path=self.script_path(script)) + self._session = self._driver.session("r") + tx = self._session.begin_transaction() + with self.assertRaises(types.DriverError) as exc: + tx.run("invalid") + self.assertEqual(exc.exception.code, + "Neo.ClientError.MadeUp.Code") + self._assert_is_client_exception(exc) + + with self.assertRaises(types.DriverError) as exc: + tx.run("invalid") + # new actions on the transaction result in a tx terminated + # exception, a subclass of the client exception + self._assert_is_tx_terminated_exception(exc) + + tx.close() + self._session.close() + self._session = None + self._server1.done() + def test_should_prevent_run_after_tx_termination_on_pull(self): def _test(): self._create_direct_driver() From 8c3984ea16dfbb8f970a6a9b75283d3d55989891 Mon Sep 17 00:00:00 2001 From: Dmitriy Tverdiakov Date: Tue, 4 Jul 2023 10:54:14 +0100 Subject: [PATCH 13/13] Refactoring --- .../test_connection_recv_timeout_seconds.py | 4 ++-- tests/stub/tx_run/test_tx_run.py | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/stub/configuration_hints/test_connection_recv_timeout_seconds.py b/tests/stub/configuration_hints/test_connection_recv_timeout_seconds.py index 8f1e42c5a..114f6b8cc 100644 --- a/tests/stub/configuration_hints/test_connection_recv_timeout_seconds.py +++ b/tests/stub/configuration_hints/test_connection_recv_timeout_seconds.py @@ -65,7 +65,7 @@ def _assert_is_timeout_exception(self, e): self.assertIn("ConnectionReadTimeoutError", e.errorType) - def _assert_is_client_or_tx_terminated_exception(self, e): + def _assert_is_tx_terminated_exception(self, e): driver = get_driver_name() if driver in ["java"]: self.assertEqual( @@ -153,7 +153,7 @@ def test_timeout_unmanaged_tx_should_fail_subsequent_usage_after_timeout( with self.assertRaises(StubServerUncleanExitError): self._server.done() self._assert_is_timeout_exception(first_run_error.exception) - self._assert_is_client_or_tx_terminated_exception(second_run_error.exception) + self._assert_is_tx_terminated_exception(second_run_error.exception) self.assertEqual(self._server.count_responses(""), 1) self.assertEqual(self._server.count_responses(""), 1) self.assertEqual(self._server.count_requests('RUN "timeout"'), 1) diff --git a/tests/stub/tx_run/test_tx_run.py b/tests/stub/tx_run/test_tx_run.py index 7f8be6787..6ef95f0c1 100644 --- a/tests/stub/tx_run/test_tx_run.py +++ b/tests/stub/tx_run/test_tx_run.py @@ -270,7 +270,7 @@ def _test(): # initiate another stream that fails on RUN with self.assertRaises(types.DriverError) as exc: - failed_res = tx.run("invalid") + tx.run("invalid") self.assertEqual(exc.exception.code, "Neo.ClientError.Statement.SyntaxError") self._assert_is_client_exception(exc) @@ -313,7 +313,7 @@ def test_should_prevent_discard_after_tx_termination_on_run(self): # initiate another stream that fails on RUN with self.assertRaises(types.DriverError) as exc: - failed_res = tx.run("invalid") + tx.run("invalid") self.assertEqual(exc.exception.code, "Neo.ClientError.Statement.SyntaxError") self._assert_is_client_exception(exc)