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:26:20
Message-ID: CAM9w-_kHGxsu+zzeO+jGq28pXOUOEimrFAV+A2QJFwGrq_m4_A@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

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"

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Yosry Muhammad 2019-08-13 11:45:06 Re: [GSoC] Query History Upgrade
Previous Message Yosry Muhammad 2019-08-12 23:33:18 Re: [GSoC] Query History Upgrade