public inbox for [email protected]  
help / color / mirror / Atom feed
From: Joao De Almeida Pereira <[email protected]>
To: Khushboo Vashi <[email protected]>
Cc: pgadmin-hackers <[email protected]>
Subject: Re: pgAdmin 4 commit: Support EXPLAIN on Greenplum. Fixes #3097
Date: Thu, 08 Mar 2018 14:22:45 +0000
Message-ID: <CAE+jjankjBpBR1AOwG2wW+0_tb=pUO1ZkTS0WgQ3RwAi7-4v5g@mail.gmail.com> (raw)
In-Reply-To: <CAFOhELdYoYcSL6m7uhNoKDKbge7sf8n_Ta2g3aGDy6hD_5VJfA@mail.gmail.com>
References: <[email protected]>
	<CAFOhELdYoYcSL6m7uhNoKDKbge7sf8n_Ta2g3aGDy6hD_5VJfA@mail.gmail.com>

Hello Khushboo,
Completely forgot about this python "feature".......
Attached is the fix.

Just as a side question, does anyone else feel the pain of wanting to run a
single test using a IDE or the command line and not being able to?
We an HandRolled the loader, and that as some implications. Did anyone try
to use a different launcher like pytest or nose instead of the current
runner?
I understand that testscenarios is one of the problems we have if we want
to move away from this way of running tests.
Any suggestion?

Thanks
Joao

On Wed, Mar 7, 2018 at 11:41 PM Khushboo Vashi <
[email protected]> wrote:

> Hi Joao,
>
> In the test_start_running_query.py, 2 static methods (is_begin_required_for_sql_query
> and is_rollback_statement_required)
> of StartRunningQuery class were used directly without @patch. Due to
> this, in all the cases, the original value of them doesn't restore.
>
> To fix this, I have sent the patch in another thread, to restore its
> original state, but I wonder if we can use these methods with @patch.
>
> Thanks,
> Khushboo
>
>
> On Fri, Feb 9, 2018 at 5:24 PM, Dave Page <[email protected]> wrote:
>
>> Support EXPLAIN on Greenplum. Fixes #3097
>>
>>  - Extract SQLEditor.execute and SQLEditor._poll into their own files and
>> add test around them
>>  - Extract SQLEditor backend functions that start executing query to
>> their own files and add tests around it
>>  - Move the Explain SQL from the front-end and now pass the Explain plan
>> parameters as a JSON object in the start query call.
>>  - Extract the compile_template_name into a function that can be used by
>> the different places that try to select the version of the template and the
>> server type
>>
>> Branch
>> ------
>> master
>>
>> Details
>> -------
>>
>> https://git.postgresql.org/gitweb?p=pgadmin4.git;a=commitdiff;h=e16a95275336529a734bf0066889e39cc8ef...
>> Author: Joao Pedro De Almeida Pereira <[email protected]>
>>
>> Modified Files
>> --------------
>> .../databases/schemas/tables/tests/test_utils.py   |    0
>> web/pgadmin/static/js/sqleditor/execute_query.js   |  287 ++++
>> .../js/sqleditor/is_new_transaction_required.js    |   14 +
>> .../static/js/sqleditor/query_tool_actions.js      |   33 +-
>> web/pgadmin/tools/sqleditor/__init__.py            |  396 +----
>> web/pgadmin/tools/sqleditor/static/js/sqleditor.js |  227 +--
>> .../sqleditor/sql/10_plus/explain_plan.sql         |   23 +
>> .../sqleditor/sql/9.2_plus/explain_plan.sql        |   20 +
>> .../sqleditor/sql/default/explain_plan.sql         |   17 +
>> .../sqleditor/sql/gpdb_5.0_plus/explain_plan.sql   |    5 +
>> web/pgadmin/tools/sqleditor/tests/__init__.py      |    8 +
>> .../sqleditor/tests/test_explain_plan_templates.py |  152 ++
>> .../test_extract_sql_from_network_parameters.py    |   59 +
>> .../tools/sqleditor/tests/test_start_query_tool.py |   38 +
>> web/pgadmin/tools/sqleditor/utils/__init__.py      |   14 +
>> .../sqleditor/utils/apply_explain_plan_wrapper.py  |   24 +
>> .../tools/sqleditor/utils/constant_definition.py   |   32 +
>> .../tools/sqleditor/utils/is_begin_required.py     |  169 ++
>> .../tools/sqleditor/utils/start_running_query.py   |  172 ++
>> .../tools/sqleditor/utils/tests/__init__.py        |    8 +
>> .../utils/tests/test_apply_explain_plan_wrapper.py |  121 ++
>> .../utils/tests/test_start_running_query.py        |  445 +++++
>> .../utils/update_session_grid_transaction.py       |   18 +
>> web/pgadmin/utils/compile_template_name.py         |   17 +
>> .../utils/tests/test_compile_template_name.py      |   34 +
>> web/pgadmin/utils/versioned_template_loader.py     |    2 +-
>> web/regression/javascript/fake_endpoints.js        |    6 +-
>> .../javascript/sqleditor/execute_query_spec.js     | 1702
>> ++++++++++++++++++++
>> .../sqleditor/is_new_transaction_required_spec.js  |   65 +
>> .../sqleditor/query_tool_actions_spec.js           |  141 +-
>> 30 files changed, 3670 insertions(+), 579 deletions(-)
>>
>>
>


Attachments:

  [application/octet-stream] use_patch_instead_of_replace_method.diff (4.4K, 3-use_patch_instead_of_replace_method.diff)
  download | inline diff:
diff --git a/web/pgadmin/tools/sqleditor/utils/tests/test_start_running_query.py b/web/pgadmin/tools/sqleditor/utils/tests/test_start_running_query.py
index 23a5c7fc..b7745463 100644
--- a/web/pgadmin/tools/sqleditor/utils/tests/test_start_running_query.py
+++ b/web/pgadmin/tools/sqleditor/utils/tests/test_start_running_query.py
@@ -8,8 +8,8 @@
 ##########################################################################
 import sys
 
-from flask import Response
 import simplejson as json
+from flask import Response
 
 from pgadmin.tools.sqleditor.utils.start_running_query import StartRunningQuery
 from pgadmin.utils.exception import ConnectionLost
@@ -385,8 +385,14 @@ class StartRunningQueryTest(BaseTestGenerator):
            '.internal_server_error')
     @patch('pgadmin.tools.sqleditor.utils.start_running_query'
            '.update_session_grid_transaction')
-    def runTest(self, update_session_grid_transaction_mock,
-                internal_server_error_mock, get_driver_mock, pickle_mock,
+    @patch('pgadmin.tools.sqleditor.utils.start_running_query'
+           '.StartRunningQuery.is_begin_required_for_sql_query')
+    @patch('pgadmin.tools.sqleditor.utils.start_running_query'
+           '.StartRunningQuery.is_rollback_statement_required')
+    def runTest(self, is_rollback_statement_required_stub,
+                is_begin_required_for_sql_query_stub,
+                update_session_grid_transaction_stub,
+                internal_server_error_mock, get_driver_stub, pickle_stub,
                 make_json_response_mock,
                 apply_explain_plan_wrapper_if_needed_mock):
         """Check correct function is called to handle to run query."""
@@ -398,41 +404,27 @@ class StartRunningQueryTest(BaseTestGenerator):
         make_json_response_mock.return_value = expected_response
         if self.expect_internal_server_error_called_with is not None:
             internal_server_error_mock.return_value = expected_response
-        pickle_mock.loads.return_value = self.pickle_load_return
+        pickle_stub.loads.return_value = self.pickle_load_return
         blueprint_mock = MagicMock(
             info_notifier_timeout=MagicMock(get=lambda: 5))
 
-        # Save value for the later use
-        self.is_begin_required_for_sql_query = \
-            StartRunningQuery.is_begin_required_for_sql_query
-        self.is_rollback_statement_required = \
-            StartRunningQuery.is_rollback_statement_required
-
         if self.is_begin_required:
-            StartRunningQuery.is_begin_required_for_sql_query = MagicMock(
-                return_value=True
-            )
+            is_begin_required_for_sql_query_stub.return_value = True
         else:
-            StartRunningQuery.is_begin_required_for_sql_query = MagicMock(
-                return_value=False
-            )
+            is_begin_required_for_sql_query_stub.return_value = False
         if self.is_rollback_required:
-            StartRunningQuery.is_rollback_statement_required = MagicMock(
-                return_value=True
-            )
+            is_rollback_statement_required_stub.return_value = True
         else:
-            StartRunningQuery.is_rollback_statement_required = MagicMock(
-                return_value=False
-            )
+            is_rollback_statement_required_stub.return_value = False
 
         apply_explain_plan_wrapper_if_needed_mock.return_value = \
             self.apply_explain_plan_wrapper_if_needed_return_value
 
         manager = self.__create_manager()
         if self.get_driver_exception:
-            get_driver_mock.side_effect = get_driver_exception
+            get_driver_stub.side_effect = get_driver_exception
         else:
-            get_driver_mock.return_value = MagicMock(
+            get_driver_stub.return_value = MagicMock(
                 connection_manager=lambda session_id: manager)
 
         try:
@@ -525,10 +517,3 @@ class StartRunningQueryTest(BaseTestGenerator):
             self.connection.execute_void.assert_called_with('ROLLBACK;')
         elif not self.is_begin_required:
             self.connection.execute_void.assert_not_called()
-
-    def tearDown(self):
-        #  Reset methods to the original state
-        StartRunningQuery.is_rollback_statement_required = \
-            staticmethod(self.is_rollback_statement_required)
-        StartRunningQuery.is_rollback_statement_required = \
-            staticmethod(self.is_rollback_statement_required)


view thread (9+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected]
  Subject: Re: pgAdmin 4 commit: Support EXPLAIN on Greenplum. Fixes #3097
  In-Reply-To: <CAE+jjankjBpBR1AOwG2wW+0_tb=pUO1ZkTS0WgQ3RwAi7-4v5g@mail.gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox