Re: [GSoC] Query History Upgrade

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"