Re: [GSoC] Query History Upgrade - Mailing list pgadmin-hackers

From Yosry Muhammad
Subject Re: [GSoC] Query History Upgrade
Date
Msg-id CAFSMqn_GVgEf88F0orf6Bc0FPuT-eOU1u7L7uySP9ZKM3mnAOA@mail.gmail.com
Whole thread Raw
In response to Re: [GSoC] Query History Upgrade  (Aditya Toshniwal <aditya.toshniwal@enterprisedb.com>)
Responses Re: [GSoC] Query History Upgrade  (Yosry Muhammad <yosrym93@gmail.com>)
List pgadmin-hackers
Is it always the same error when it fails?

On Tue, Aug 13, 2019 at 1:50 PM Aditya Toshniwal <aditya.toshniwal@enterprisedb.com> wrote:
The test cases fails intermittently. It passes sometimes.

On Tue, Aug 13, 2019 at 5:15 PM Yosry Muhammad <yosrym93@gmail.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.toshniwal@enterprisedb.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@gmail.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.toshniwal@enterprisedb.com> wrote:
Hi Yosry,


On Mon, Aug 12, 2019 at 2:00 PM Yosry Muhammad <yosrym93@gmail.com> wrote:
Hi Aditya,

On Mon, Aug 12, 2019 at 9:04 AM Aditya Toshniwal <aditya.toshniwal@enterprisedb.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. 
 
4I 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.
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.


--
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.


--
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.


--
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.
Attachment

pgadmin-hackers by date:

Previous
From: Aditya Toshniwal
Date:
Subject: Re: [GSoC] Query History Upgrade
Next
From: Yosry Muhammad
Date:
Subject: Re: [GSoC] Query History Upgrade