Re: [GSoC] Query History Upgrade

From: Aditya Toshniwal <aditya(dot)toshniwal(at)enterprisedb(dot)com>
To: Yosry Muhammad <yosrym93(at)gmail(dot)com>
Cc: pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org>, Dave Page <dpage(at)pgadmin(dot)org>
Subject: Re: [GSoC] Query History Upgrade
Date: 2019-08-13 11:49:49
Message-ID: CAM9w-_=pknwtJD4ML1EE5nPr+PKnEuMOTXENHe3kF+DiA3hwVA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

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"

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Yosry Muhammad 2019-08-13 11:51:20 Re: [GSoC] Query History Upgrade
Previous Message Yosry Muhammad 2019-08-13 11:45:06 Re: [GSoC] Query History Upgrade