Re: [GSoC] Query History Upgrade

From: Akshay Joshi <akshay(dot)joshi(at)enterprisedb(dot)com>
To: Yosry Muhammad <yosrym93(at)gmail(dot)com>
Cc: Aditya Toshniwal <aditya(dot)toshniwal(at)enterprisedb(dot)com>, pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org>, Dave Page <dpage(at)pgadmin(dot)org>
Subject: Re: [GSoC] Query History Upgrade
Date: 2019-08-14 10:06:13
Message-ID: CANxoLDes=wH6uMhQuFgu=-t0V=+4Z9hmFgxx4rPP6V0Z13TpKA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

Attaching Browser Error Screenshot

On Wed, Aug 14, 2019 at 3:33 PM Akshay Joshi <akshay(dot)joshi(at)enterprisedb(dot)com>
wrote:

> Hi Yosry
>
> I have found following issues:
>
> - Jasmine test cases are failing.
> - Browser error when applying the patch and open the query tool.
> Previously saved queries are there:
> - [image: Screenshot 2019-08-14 at 3.21.21 PM.png]
> - Toggle Switch should have text "Yes/No" as we already have label
> "Show queries generated internally by pgAdmin?"
> - Show/Hide query works on click of label as well.
> - For View/Edit Data it seems to take all the queries as internal, is
> this expected ?
>
> Feature #4612 <https://redmine.postgresql.org/issues/4612> has been
> created to track it.
>
> On Wed, Aug 14, 2019 at 1:39 PM Yosry Muhammad <yosrym93(at)gmail(dot)com> wrote:
>
>> Great! thanks everyone.
>>
>> On Wed, Aug 14, 2019, 10:08 AM Akshay Joshi <
>> akshay(dot)joshi(at)enterprisedb(dot)com> wrote:
>>
>>> Hi Yosry
>>>
>>>
>>> On Tue, Aug 13, 2019 at 6:33 PM Yosry Muhammad <yosrym93(at)gmail(dot)com>
>>> wrote:
>>>
>>>> Any more suggestions or comments on the patch?
>>>>
>>>
>>> I am reviewing it.
>>>
>>>>
>>>> On Tue, Aug 13, 2019 at 2:55 PM Aditya Toshniwal <
>>>> aditya(dot)toshniwal(at)enterprisedb(dot)com> wrote:
>>>>
>>>>>
>>>>>
>>>>> On Tue, Aug 13, 2019 at 5:35 PM Yosry Muhammad <yosrym93(at)gmail(dot)com>
>>>>> wrote:
>>>>>
>>>>>> Please find attached an updated patch with a small modification to
>>>>>> the feature test. This makes sure the first history element is selected and
>>>>>> focused before trying to move through entries using the down arrow key.
>>>>>> When the test fails, is it always the same error you sent before ?
>>>>>>
>>>>> Seems to be working now. And yes, I got the same error every time it
>>>>> had failed.
>>>>>
>>>>>>
>>>>>> On Tue, Aug 13, 2019 at 1:51 PM Yosry Muhammad <yosrym93(at)gmail(dot)com>
>>>>>> wrote:
>>>>>>
>>>>>>> Is it always the same error when it fails?
>>>>>>>
>>>>>>> On Tue, Aug 13, 2019 at 1:50 PM Aditya Toshniwal <
>>>>>>> aditya(dot)toshniwal(at)enterprisedb(dot)com> wrote:
>>>>>>>
>>>>>>>> The test cases fails intermittently. It passes sometimes.
>>>>>>>>
>>>>>>>> On Tue, Aug 13, 2019 at 5:15 PM Yosry Muhammad <yosrym93(at)gmail(dot)com>
>>>>>>>> wrote:
>>>>>>>>
>>>>>>>>> Hi Aditya,
>>>>>>>>>
>>>>>>>>> The test passes on my computer, is there anything I could try to
>>>>>>>>> reproduced the failure?
>>>>>>>>>
>>>>>>>>> By looking at the error, I suspect clicking the down arrow key on
>>>>>>>>> your machine did not move to the next history entry during the test. Does
>>>>>>>>> clicking the down arrow normally go to the next history entry on your
>>>>>>>>> machine?
>>>>>>>>>
>>>>>>>>> On Tue, Aug 13, 2019 at 1:26 PM Aditya Toshniwal <
>>>>>>>>> aditya(dot)toshniwal(at)enterprisedb(dot)com> wrote:
>>>>>>>>>
>>>>>>>>>> Hi Yosry,
>>>>>>>>>>
>>>>>>>>>> Everything looks fine to me. Except intermediate feature test
>>>>>>>>>> failure. May be @committers can try on their machine.
>>>>>>>>>>
>>>>>>>>>> ======================================================================
>>>>>>>>>> FAIL: runTest
>>>>>>>>>> (pgadmin.feature_tests.query_tool_journey_test.QueryToolJourneyTest)
>>>>>>>>>> Tests the path through the query tool
>>>>>>>>>>
>>>>>>>>>> ----------------------------------------------------------------------
>>>>>>>>>> Traceback (most recent call last):
>>>>>>>>>> File
>>>>>>>>>> "/Users/adityatoshniwal/projects/pgadmin4/web/pgadmin/feature_tests/query_tool_journey_test.py",
>>>>>>>>>> line 78, in runTest
>>>>>>>>>> self._test_query_sources_and_generated_queries()
>>>>>>>>>> File
>>>>>>>>>> "/Users/adityatoshniwal/projects/pgadmin4/web/pgadmin/feature_tests/query_tool_journey_test.py",
>>>>>>>>>> line 186, in _test_query_sources_and_generated_queries
>>>>>>>>>> self._test_history_query_sources()
>>>>>>>>>> File
>>>>>>>>>> "/Users/adityatoshniwal/projects/pgadmin4/web/pgadmin/feature_tests/query_tool_journey_test.py",
>>>>>>>>>> line 215, in _test_history_query_sources
>>>>>>>>>> history_entries_icons)
>>>>>>>>>> File
>>>>>>>>>> "/Users/adityatoshniwal/projects/pgadmin4/web/pgadmin/feature_tests/query_tool_journey_test.py",
>>>>>>>>>> line 287, in _check_history_queries_and_icons
>>>>>>>>>> self.assertIn(query, query_history_selected_item.text)
>>>>>>>>>> AssertionError: "UPDATE public.test_editable_table2293 SET
>>>>>>>>>> normal_column = '10'::numeric WHERE pk_column = '1';" not found in
>>>>>>>>>> 'COMMIT;\n15:00:40'
>>>>>>>>>>
>>>>>>>>>> On Tue, Aug 13, 2019 at 5:03 AM Yosry Muhammad <
>>>>>>>>>> yosrym93(at)gmail(dot)com> wrote:
>>>>>>>>>>
>>>>>>>>>>> The test failed after merging with master. A previously written
>>>>>>>>>>> test needed to be updated after a previous commit.
>>>>>>>>>>>
>>>>>>>>>>> Please find an updated patch attached with the fix.
>>>>>>>>>>>
>>>>>>>>>>> On Mon, Aug 12, 2019 at 1:34 PM Aditya Toshniwal <
>>>>>>>>>>> aditya(dot)toshniwal(at)enterprisedb(dot)com> wrote:
>>>>>>>>>>>
>>>>>>>>>>>> Hi Yosry,
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> On Mon, Aug 12, 2019 at 2:00 PM Yosry Muhammad <
>>>>>>>>>>>> yosrym93(at)gmail(dot)com> wrote:
>>>>>>>>>>>>
>>>>>>>>>>>>> Hi Aditya,
>>>>>>>>>>>>>
>>>>>>>>>>>>> On Mon, Aug 12, 2019 at 9:04 AM Aditya Toshniwal <
>>>>>>>>>>>>> aditya(dot)toshniwal(at)enterprisedb(dot)com> wrote:
>>>>>>>>>>>>>
>>>>>>>>>>>>>> Hi Yosry,
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Nice work there. It seems to be working fine except a few
>>>>>>>>>>>>>> suggestions:
>>>>>>>>>>>>>> 1) Fix pep8 issues
>>>>>>>>>>>>>> 2) DOM Statements like below can be avoided and html can be
>>>>>>>>>>>>>> added directly to main template of $el instead of adding extra operations
>>>>>>>>>>>>>> of find, prepend and append. Plus, it makes it difficult to understand what
>>>>>>>>>>>>>> will the DOM look like.
>>>>>>>>>>>>>> this.$el.find('.query').prepend('<i id="query_source_icon"
>>>>>>>>>>>>>> class="query-history-icon sql-icon-lg"></i>');
>>>>>>>>>>>>>> $container.append($toggleEl).append(self.$el);
>>>>>>>>>>>>>> 3) Change the below to use class d-none with
>>>>>>>>>>>>>> toggleClass('d-none') for consistency across.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> this.$el.find('.pgadmin-query-history-entry').each(function() {
>>>>>>>>>>>>>> $(this).toggle();
>>>>>>>>>>>>>> });
>>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> Please find an updated patch attached with the above issues
>>>>>>>>>>>>> fixed. The pep8 issue was in the test, I didn't re-check pep8 after writing
>>>>>>>>>>>>> the test - my bad.
>>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>> 4) I may be wrong, but I'm seeing the flash icon for
>>>>>>>>>>>>>> view/edit data queries and view table icon for query tool queries. Looks
>>>>>>>>>>>>>> like they are swapped.
>>>>>>>>>>>>>> [image: Screenshot 2019-08-12 at 12.15.18.png]
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>> They seem to be in the right place for me, would you mind
>>>>>>>>>>>>> rechecking?
>>>>>>>>>>>>>
>>>>>>>>>>>> Now there are showing fine.
>>>>>>>>>>>> However, the feature test case is failing for me. I tried 2
>>>>>>>>>>>> times:
>>>>>>>>>>>> =============Running the test cases for 'PostgreSQL
>>>>>>>>>>>> 11'=============
>>>>>>>>>>>> runTest
>>>>>>>>>>>> (pgadmin.feature_tests.query_tool_journey_test.QueryToolJourneyTest)
>>>>>>>>>>>> Tests the path through the query tool ... Copy rows... OK.
>>>>>>>>>>>> Copy columns... OK.
>>>>>>>>>>>> History tab... OK.
>>>>>>>>>>>> Updatable resultsets...FAIL
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> ======================================================================
>>>>>>>>>>>> FAIL: runTest
>>>>>>>>>>>> (pgadmin.feature_tests.query_tool_journey_test.QueryToolJourneyTest)
>>>>>>>>>>>> Tests the path through the query tool
>>>>>>>>>>>>
>>>>>>>>>>>> ----------------------------------------------------------------------
>>>>>>>>>>>> Traceback (most recent call last):
>>>>>>>>>>>> File
>>>>>>>>>>>> "/Users/adityatoshniwal/projects/pgadmin4/web/pgadmin/feature_tests/query_tool_journey_test.py",
>>>>>>>>>>>> line 79, in runTest
>>>>>>>>>>>> self._test_updatable_resultset()
>>>>>>>>>>>> File
>>>>>>>>>>>> "/Users/adityatoshniwal/projects/pgadmin4/web/pgadmin/feature_tests/query_tool_journey_test.py",
>>>>>>>>>>>> line 240, in _test_updatable_resultset
>>>>>>>>>>>> self._check_query_results_editable(query, False)
>>>>>>>>>>>> File
>>>>>>>>>>>> "/Users/adityatoshniwal/projects/pgadmin4/web/pgadmin/feature_tests/query_tool_journey_test.py",
>>>>>>>>>>>> line 378, in _check_query_results_editable
>>>>>>>>>>>> is_editable = self._check_cell_editable(1)
>>>>>>>>>>>> File
>>>>>>>>>>>> "/Users/adityatoshniwal/projects/pgadmin4/web/pgadmin/feature_tests/query_tool_journey_test.py",
>>>>>>>>>>>> line 395, in _check_cell_editable
>>>>>>>>>>>> self.assertFalse('editable' in cell_classes)
>>>>>>>>>>>> AssertionError: True is not false
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> ----------------------------------------------------------------------
>>>>>>>>>>>> Ran 1 test in 38.118s
>>>>>>>>>>>>
>>>>>>>>>>>> FAILED (failures=1)
>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> Thanks !
>>>>>>>>>>>>>
>>>>>>>>>>>>> --
>>>>>>>>>>>>> *Yosry Muhammad Yosry*
>>>>>>>>>>>>>
>>>>>>>>>>>>> Computer Engineering student,
>>>>>>>>>>>>> The Faculty of Engineering,
>>>>>>>>>>>>> Cairo University (2021).
>>>>>>>>>>>>> Class representative of CMP 2021.
>>>>>>>>>>>>> https://www.linkedin.com/in/yosrym93/
>>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> --
>>>>>>>>>>>> Thanks and Regards,
>>>>>>>>>>>> Aditya Toshniwal
>>>>>>>>>>>> Software Engineer | EnterpriseDB India | Pune
>>>>>>>>>>>> "Don't Complain about Heat, Plant a TREE"
>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> --
>>>>>>>>>>> *Yosry Muhammad Yosry*
>>>>>>>>>>>
>>>>>>>>>>> Computer Engineering student,
>>>>>>>>>>> The Faculty of Engineering,
>>>>>>>>>>> Cairo University (2021).
>>>>>>>>>>> Class representative of CMP 2021.
>>>>>>>>>>> https://www.linkedin.com/in/yosrym93/
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> --
>>>>>>>>>> Thanks and Regards,
>>>>>>>>>> Aditya Toshniwal
>>>>>>>>>> Software Engineer | EnterpriseDB India | Pune
>>>>>>>>>> "Don't Complain about Heat, Plant a TREE"
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> --
>>>>>>>>> *Yosry Muhammad Yosry*
>>>>>>>>>
>>>>>>>>> Computer Engineering student,
>>>>>>>>> The Faculty of Engineering,
>>>>>>>>> Cairo University (2021).
>>>>>>>>> Class representative of CMP 2021.
>>>>>>>>> https://www.linkedin.com/in/yosrym93/
>>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> --
>>>>>>>> Thanks and Regards,
>>>>>>>> Aditya Toshniwal
>>>>>>>> Software Engineer | EnterpriseDB India | Pune
>>>>>>>> "Don't Complain about Heat, Plant a TREE"
>>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> --
>>>>>>> *Yosry Muhammad Yosry*
>>>>>>>
>>>>>>> Computer Engineering student,
>>>>>>> The Faculty of Engineering,
>>>>>>> Cairo University (2021).
>>>>>>> Class representative of CMP 2021.
>>>>>>> https://www.linkedin.com/in/yosrym93/
>>>>>>>
>>>>>>
>>>>>>
>>>>>> --
>>>>>> *Yosry Muhammad Yosry*
>>>>>>
>>>>>> Computer Engineering student,
>>>>>> The Faculty of Engineering,
>>>>>> Cairo University (2021).
>>>>>> Class representative of CMP 2021.
>>>>>> https://www.linkedin.com/in/yosrym93/
>>>>>>
>>>>>
>>>>>
>>>>> --
>>>>> Thanks and Regards,
>>>>> Aditya Toshniwal
>>>>> Software Engineer | EnterpriseDB India | Pune
>>>>> "Don't Complain about Heat, Plant a TREE"
>>>>>
>>>>
>>>>
>>>> --
>>>> *Yosry Muhammad Yosry*
>>>>
>>>> Computer Engineering student,
>>>> The Faculty of Engineering,
>>>> Cairo University (2021).
>>>> Class representative of CMP 2021.
>>>> https://www.linkedin.com/in/yosrym93/
>>>>
>>>
>>>
>>> --
>>> *Thanks & Regards*
>>> *Akshay Joshi*
>>>
>>> *Sr. Software Architect*
>>> *EnterpriseDB Software India Private Limited*
>>> *Mobile: +91 976-788-8246*
>>>
>>
>
> --
> *Thanks & Regards*
> *Akshay Joshi*
>
> *Sr. Software Architect*
> *EnterpriseDB Software India Private Limited*
> *Mobile: +91 976-788-8246*
>

--
*Thanks & Regards*
*Akshay Joshi*

*Sr. Software Architect*
*EnterpriseDB Software India Private Limited*
*Mobile: +91 976-788-8246*

Attachment Content-Type Size
Browser_Error.png image/png 150.2 KB

In response to

Browse pgadmin-hackers by date

  From Date Subject
Next Message Yosry Muhammad 2019-08-14 10:22:49 Re: [GSoC] Query History Upgrade
Previous Message Akshay Joshi 2019-08-14 10:03:34 Re: [GSoC] Query History Upgrade