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