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
>>>
>>>
>>
>
Feature_test_cases_view_data_v1.patch
Description: Binary data
-- Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgadmin-hackers
