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 <yosry...@gmail.com> wrote: > Great! thanks everyone. > > On Wed, Aug 14, 2019, 10:08 AM Akshay Joshi <akshay.jo...@enterprisedb.com> > wrote: > >> Hi Yosry >> >> >> On Tue, Aug 13, 2019 at 6:33 PM Yosry Muhammad <yosry...@gmail.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.toshni...@enterprisedb.com> wrote: >>> >>>> >>>> >>>> On Tue, Aug 13, 2019 at 5:35 PM Yosry Muhammad <yosry...@gmail.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 <yosry...@gmail.com> >>>>> wrote: >>>>> >>>>>> Is it always the same error when it fails? >>>>>> >>>>>> On Tue, Aug 13, 2019 at 1:50 PM Aditya Toshniwal < >>>>>> aditya.toshni...@enterprisedb.com> wrote: >>>>>> >>>>>>> The test cases fails intermittently. It passes sometimes. >>>>>>> >>>>>>> On Tue, Aug 13, 2019 at 5:15 PM Yosry Muhammad <yosry...@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.toshni...@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 <yosry...@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.toshni...@enterprisedb.com> wrote: >>>>>>>>>> >>>>>>>>>>> Hi Yosry, >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> On Mon, Aug 12, 2019 at 2:00 PM Yosry Muhammad < >>>>>>>>>>> yosry...@gmail.com> wrote: >>>>>>>>>>> >>>>>>>>>>>> Hi Aditya, >>>>>>>>>>>> >>>>>>>>>>>> On Mon, Aug 12, 2019 at 9:04 AM Aditya Toshniwal < >>>>>>>>>>>> aditya.toshni...@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. >>>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>>> 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*
