Hello Surinder,
We are having issues running the tests, this is the error that we are
getting:
File
"/Users/pivotal/workspace/pgadmin4/web/regression/python_test_utils/test_utils.py",
line 196, in create_table_with_query
pg_cursor.execute(query)
ProgrammingError: role "postgres" does not exist
Traceback (most recent call last):
File
"/Users/pivotal/workspace/pgadmin4/web/regression/python_test_utils/test_utils.py",
line 196, in create_table_with_query
pg_cursor.execute(query)
ProgrammingError: relation "defaults_id_seq" does not exist
There is already a function that waits for an element to be displayed on
the screen. The function is: self.page.find_by_xpath
In line 179 and 180, both functions do the same thing, why do we need to
wait first and then wait again. Were you experiencing flakiness?
Does _check_xss_in_view_data method checks for Cross Site Scripting?
Was there any reason to duplicate self.page._connects_to_server and
self.page._close_query_tool?
The method _verify_insert_data looks more or less the same code as in the
end of _copy_paste_row, should this be merged?
Thanks,
Joao & Shruti
On Thu, May 25, 2017 at 4:41 PM, Dave Page <dp...@pgadmin.org> wrote:
> Hi
>
> The tests failed on both PG 9.4 and 9.6 for me :-(
>
> ======================================================================
> ERROR: runTest (pgadmin.feature_tests.view_data_dml_queries.
> CheckForViewDataTest)
> Validate Insert, Update operations in View data with given test data
> ----------------------------------------------------------------------
> Traceback (most recent call last):
> File "/Users/dpage/git/pgadmin4/web/pgadmin/feature_tests/
> view_data_dml_queries.py",
> line 120, in runTest
> self._verify_insert_data()
> File "/Users/dpage/git/pgadmin4/web/pgadmin/feature_tests/
> view_data_dml_queries.py",
> line 316, in _verify_insert_data
> self._compare_cell_value(cell_xpath, config_data[str(idx)][1])
> File "/Users/dpage/git/pgadmin4/web/pgadmin/feature_tests/
> view_data_dml_queries.py",
> line 165, in _compare_cell_value
> "Timed out waiting for element to appear"
> File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-
> packages/selenium/webdriver/support/wait.py",
> line 80, in until
> raise TimeoutException(message, screen, stacktrace)
> TimeoutException: Message: Timed out waiting for element to appear
>
> Also, Can we replace the sleep with a "wait for object" or similar?
>
> On Thu, May 25, 2017 at 6:42 AM, Surinder Kumar
> <surinder.ku...@enterprisedb.com> wrote:
> > Hi
> >
> > Please find attached patch for Feature test cases.
> >
> > The approach is to create a single table 'defaults' with columns of
> various
> > types(number, text, json and boolean) and write test cases for these cell
> > types with different input values.
> >
> > Following are the test cases covered:
> >
> > 1) Add a new row, save and compare the resulted value with expected
> values
> >
> > 2) Copy/Paste row, save and compare cell data
> >
> > a) Clear cell value and escape, the cell must set to [default]
> >
> > 3) Update cell:
> >
> > a) Insert two single quotes(''), expected value is blank string
> >
> > b) Clear a cell, expected value is [null]
> >
> > c) Insert a string \’\’, expected value is ''
> >
> > d) Insert a string \\’\\’, expected value is \’\’
> >
> > e) Insert a string \\\\’\\\\’, expected value is \\’\\’
> >
> > f) If a checkbox cell is double clicked, return value must be 'true'
> >
> >
> > Thanks
> > Surinder Kumar
> >
> >
> >
> >
> >
> >
> > On Fri, May 19, 2017 at 6:08 PM, Surinder Kumar
> > <surinder.ku...@enterprisedb.com> wrote:
> >>
> >> Hi
> >>
> >> Please find updated patch and review.
> >>
> >> On Thu, May 18, 2017 at 7:36 PM, Joao Pedro De Almeida Pereira
> >> <jdealmeidapere...@pivotal.io> wrote:
> >>>
> >>> Hello Hackers,
> >>>
> >>> We reviewed the PR, and there are a lot of new lines of code in this
> >>> patch, are we sure that we can have a good coverage of the
> functionality
> >>> just by doing feature tests?
> >>> Adding more code to a 3.5k+ lines file doesn't look like a good option,
> >>> do you think it is possible to extract some of the functionality to
> their
> >>> own files and have test around those functionalities?
> >>
> >> To improve the code readability, reduce code complexity and to make code
> >> testable, the code must be splitted component wise.
> >> Here is my suggestion:
> >>
> >> 1. The code for classes like “SQLEditorView” and “SqlEditorController”
> can
> >> be moved into two files like "editor_view.js and “editor_controller.js"
> and
> >> called from within "sqleditor.js".
> >>
> >> 2. All utilities functions can be moved into separate utils file and can
> >> write test cases.
> >>
> >> 3. Slickgrid listener functions such as:
> >> onBeforeEditCell, onKeyDown,
> >> onCellChange, onAddNewRow
> >>
> >> can be moved into
> >> a file and write test cases around.
> >>
> >> This needs discussion. Any suggestion?
> >>>
> >>>
> >>> Do we really need to have an epicRandomString function in our code?
> Would
> >>> it be better to use a library that would provide us that functionality
> out
> >>> of the box?
> >>
> >> We are using "epicRandomString function" to uniquely identify each
> record,
> >> I talked to Harshal who is eliminating the use of this function and
> instead
> >> maintaining counter for the rows added/updated/deleted.
> >>>
> >>> The functions this.applyValue in slick.pgadmin.editors.js that were
> >>> change in this patch are exactly the same, can we extract that code
> into a
> >>> single function instead of repeating the code?
> >>
> >> I have moved common logic into a new function.
> >>>
> >>>
> >>> Using feature tests is a good option to ensure that the integration of
> >>> all the components of the application is working as expected, but in
> order
> >>> to tests behaviors that are edge cases the Unit Tests are much cheaper
> to
> >>> run and create.
> >>
> >> I will write test cases for functions once they are moved into their
> >> separate files as discussed above.
> >>>
> >>>
> >>> Thanks
> >>> Joao & Shruti
> >>>
> >>>
> >>> On Thu, May 18, 2017 at 1:22 AM, Surinder Kumar
> >>> <surinder.ku...@enterprisedb.com> wrote:
> >>>>
> >>>> Hi Dave,
> >>>>
> >>>> Please review the updated patch.
> >>>>
> >>>> On Wed, May 17, 2017 at 8:46 PM, Dave Page <dp...@pgadmin.org> wrote:
> >>>>>
> >>>>> Hi
> >>>>>
> >>>>> On Wed, May 17, 2017 at 3:08 PM, Surinder Kumar
> >>>>> <surinder.ku...@enterprisedb.com> wrote:
> >>>>>>
> >>>>>> Hi Dave,
> >>>>>>
> >>>>>> Implementation:
> >>>>>>
> >>>>>> 1) Took a flag 'is_row_copied' for each copied row:
> >>>>>>
> >>>>>> - to distinguish it from add/update row.
> >>>>>> - to write code specific to copied row only as it requires
> different
> >>>>>> logic than add row.
> >>>>>>
> >>>>>> 2) After pasting an existing row:
> >>>>>>
> >>>>>> - If a user clear the cell value, it must set cell to [default]
> value
> >>>>>> if default value is explicitly given for column while creating table
> >>>>>> otherwise [null].
> >>>>>>
> >>>>>> - Again, if a user entered a value in same cell, then removes that
> >>>>>> value, set it to [null] (the same behaviour is for add row).
> >>>>>>
> >>>>>> 3) Took a 2-dimensional array "grid.copied_rows" to keep track the
> >>>>>> changes of each row's cell so that changes made to each cell are
> independent
> >>>>>> and removed once data is saved.
> >>>>>>
> >>>>>> 4) On pasting a row, the cell must be highlighted with light green
> >>>>>> colour, so triggers an addNewRow event. Now copied row will add to
> grid
> >>>>>> instead of re-rendering all rows again.
> >>>>>>
> >>>>>> 5) Moved the common logic into functions.
> >>>>>>
> >>>>>> This patch pass the feature test cases and jasmine test case.
> >>>>>> Also I will add the test cases for copy row and send an updated
> patch.
> >>>>>>
> >>>>>> Please find attached patch and review.
> >>>>>
> >>>>>
> >>>>> Looks good. I saw the following issues:
> >>>>>
> >>>>> - The "new" row is not available once I've created the first new row,
> >>>>> or pasted some.
> >>>>
> >>>> Fixed.
> >>>>>
> >>>>>
> >>>>> - Rows are pasted in reverse order.
> >>>>
> >>>> Fixed.
> >>>>>
> >>>>>
> >>>>> - If I copy/paste a new row that has yet to be saved, none of the
> >>>>> values are actually copied.
> >>>>
> >>>> Fixed.
> >>>>>
> >>>>>
> >>>>> - Attempting to save a row that contains all null/default values
> >>>>> results in an invalid query:
> >>>>>
> >>>>> INSERT INTO public.defaults (
> >>>>> ) VALUES (
> >>>>> );
> >>>>>
> >>>>> I think the only answer here is to explicitly insert NULL into any
> >>>>> columns *without* a default value.
> >>>>
> >>>> I could not reproduce, However #3 might have fixed it too.
> >>>>
> >>>> Apart from this, I noticed epicRandomString(...) function doesn't
> >>>> generate unique number always, due to this save and delete rows was
> >>>> affected. Not all rows saved or deleted. The new function always
> returns a
> >>>> unique random number.
> >>>> Fixed.
> >>>>>
> >>>>>
> >>>>> Thanks.
> >>>>>
> >>>>> --
> >>>>> Dave Page
> >>>>> Blog: http://pgsnake.blogspot.com
> >>>>> Twitter: @pgsnake
> >>>>>
> >>>>> EnterpriseDB UK: http://www.enterprisedb.com
> >>>>> The Enterprise PostgreSQL Company
> >>>>
> >>>>
> >>>>
> >>>>
> >>>> --
> >>>> Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org
> )
> >>>> To make changes to your subscription:
> >>>> http://www.postgresql.org/mailpref/pgadmin-hackers
> >>>>
> >>>
> >>
> >
>
>
>
> --
> Dave Page
> Blog: http://pgsnake.blogspot.com
> Twitter: @pgsnake
>
> EnterpriseDB UK: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>
