Re: pgAdmin IV API test cases patch

From: Dave Page <dpage(at)pgadmin(dot)org>
To: Priyanka Shendge <priyanka(dot)shendge(at)enterprisedb(dot)com>
Cc: pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org>, Kanchan Mohitey <kanchan(dot)mohitey(at)enterprisedb(dot)com>
Subject: Re: pgAdmin IV API test cases patch
Date: 2016-07-04 12:00:37
Message-ID: 1C830532-F2F0-4FBC-BD17-4428CE9AA53D@pgadmin.org
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

Hi

The test data was the default, and I ran against PG 9.4. All other logs were attached to my previous email.

--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK:http://www.enterprisedb.com
The Enterprise PostgreSQL Company

> On 4 Jul 2016, at 12:16, Priyanka Shendge <priyanka(dot)shendge(at)enterprisedb(dot)com> wrote:
>
> Hi Dave,
>
> I am unable to reproduce issue on my side; tried on Python 2.7 and Python 3.4.
> Could you please provide me DEBUG logs and test data using for database node?
>
> Thank you.
>
>> On 30 June 2016 at 00:51, Dave Page <dpage(at)pgadmin(dot)org> wrote:
>> Hi,
>>
>> That's better. I tweaked a few things and fixed a bug related to
>> recent changes to the schema version config. Patch attached.
>>
>> However, there are still issues:
>>
>> 1) The testsuite doesn't run to completion. See the attached
>> stdout.txt and logger.txt files.
>> 2) stdout should only display the test summary - what tests are
>> currently running (and pass/fail), and a summary at the end - even if
>> there's a crash like I saw.
>> 3) The output log file should contain the full output, including
>> what's sent to stdout.
>> 4) The output advises the user to check ".../pgadmin4/web/regression".
>> This should be in the summary at the end, and should be corrected to
>> show the correct (full) path.
>>
>> Thanks.
>>
>>
>> On Wed, Jun 29, 2016 at 2:52 PM, Priyanka Shendge
>> <priyanka(dot)shendge(at)enterprisedb(dot)com> wrote:
>> > Hi Dave,
>> >
>> > As per discussion over mail i have created separate config files for
>> > credentials and test data.
>> >
>> > PFA patch for same. Kindly, review and let me know for modifications.
>> >
>> > On 27 June 2016 at 15:10, Priyanka Shendge
>> > <priyanka(dot)shendge(at)enterprisedb(dot)com> wrote:
>> >>
>> >>
>> >>
>> >> On 27 June 2016 at 13:24, Dave Page <dpage(at)pgadmin(dot)org> wrote:
>> >>>
>> >>> On Sun, Jun 26, 2016 at 12:05 PM, Priyanka Shendge
>> >>> <priyanka(dot)shendge(at)enterprisedb(dot)com> wrote:
>> >>> >
>> >>> >
>> >>> > On 24 June 2016 at 16:17, Dave Page <dpage(at)pgadmin(dot)org> wrote:
>> >>> >>
>> >>> >> Hi
>> >>> >>
>> >>> >> On Thu, Jun 23, 2016 at 2:41 PM, Priyanka Shendge
>> >>> >> <priyanka(dot)shendge(at)enterprisedb(dot)com> wrote:
>> >>> >> >
>> >>> >> >
>> >>> >> > On 15 June 2016 at 15:05, Priyanka Shendge
>> >>> >> > <priyanka(dot)shendge(at)enterprisedb(dot)com> wrote:
>> >>> >> >>
>> >>> >> >> Thanks a lot Dave.
>> >>> >> >>
>> >>> >> >> On 15 June 2016 at 14:09, Dave Page <dpage(at)pgadmin(dot)org> wrote:
>> >>> >> >>>
>> >>> >> >>> Hi
>> >>> >> >>>
>> >>> >> >>> On Thu, Jun 9, 2016 at 1:37 PM, Priyanka Shendge
>> >>> >> >>> <priyanka(dot)shendge(at)enterprisedb(dot)com> wrote:
>> >>> >> >>> > Hi Dave,
>> >>> >> >>> >
>> >>> >> >>> > PFA updated patch. I have made changes suggested by you.
>> >>> >> >>> >
>> >>> >> >>> > Kindly, review and let me know for more changes.
>> >>> >> >>>
>> >>> >> >>> OK, I got a bit further this time, but not there yet.
>> >>> >> >>>
>> >>> >> >>> 1) The patch overwrote my test_config.json file. That should never
>> >>> >> >>> happen (that file shouldn't be in the source tree).
>> >>> >> >>> test_config.json.in should be the file that's included in the
>> >>> >> >>> patch.
>> >>> >> >>
>> >>> >> >>
>> >>> >> >> OK.
>> >>> >> >>>
>> >>> >> >>>
>> >>> >> >>> 2) The updated test_config.json file is huge.
>> >>> >> >
>> >>> >> >
>> >>> >> > Current configuration file web/regression/test_config.json contains
>> >>> >> > test
>> >>> >> > data(credentials) for each tree node;
>> >>> >> > which is used while adding and updating the respective node.
>> >>> >>
>> >>> >> Why would we need that?
>> >>> >
>> >>> >
>> >>> > Each node file (e.g. test_db_add.py and test_db_put.py) uses respective
>> >>> > credentials test data from
>> >>> > test_config.json while execution.
>> >>>
>> >>> That doesn't answer my question - why do we need separate credentials
>> >>> for each node?
>> >>
>> >>
>> >> Sorry for typo, its test data not credentials.
>> >>
>> >>>
>> >>>
>> >>> >> We should have just one set of credentials for
>> >>> >> everything.
>> >>> >
>> >>> >
>> >>> > Let me know if my understanding is clear:
>> >>> >
>> >>> > Should i keep basic credentials of each node (database, schema) into
>> >>> > test_config.json
>> >>> > instead taking care of each field?
>> >>>
>> >>> You should have one set of credentials that's used for the entire test
>> >>> run.
>> >>
>> >>
>> >> Sure. I'll separate the credentials and test data into 2 different files.
>> >> So, a normal user can run the tests into one go after some minor
>> >> credentials changes.
>> >> And an advanced user can have an option to change the test data if he
>> >> wants.
>> >>>
>> >>>
>> >>> >> >>> I should only need to
>> >>> >> >>> define one or more connections, then be able to run the tests. If
>> >>> >> >>> you
>> >>> >> >>> need to keep configuration info for "advanced users", let's put it
>> >>> >> >>> in
>> >>> >> >>> a different file to avoid confusing/scaring everyone else. Maybe
>> >>> >> >>> split
>> >>> >> >>> it into config.json for the stuff the user needs to edit
>> >>> >> >>> (config.json.in would go in git), and test_config.json for the
>> >>> >> >>> test
>> >>> >> >>> configuration.
>> >>> >> >
>> >>> >> >
>> >>> >> > Should i keep login and server credentials into
>> >>> >> > web/regression/test_config.json file and
>> >>> >> > put respective node details into config.json file of respective
>> >>> >> > node's
>> >>> >> > tests
>> >>> >> > directory?
>> >>> >>
>> >>> >> Not if you expect users to need to edit them - and if not, why are the
>> >>> >> values not just hard-coded?
>> >>> >>
>> >>> >> > e.g. for database node:
>> >>> >> > I'll create config.json file into .../databases/tests/ directory
>> >>> >> > put database add and update credentials into config.json
>> >>> >>
>> >>> >> The key here is to make it simple for users.
>> >>> >>
>> >>> >> - To run the default tests, they should be able to copy/edit a simple
>> >>> >> file, and just add database server details for the server to run
>> >>> >> against.
>> >>> >>
>> >>> >> - If we have configurable tests (because making them configurable adds
>> >>> >> genuine value), then we can use an "advanced" config file to allow the
>> >>> >> user to adjust settings as they want.
>> >>> >>
>> >>> >> In the simple case, the user should be able to run the tests
>> >>> >> successfully within a minute or two from starting.
>> >>> >>
>> >>> >> In designing the layout for files etc, remember the following:
>> >>> >>
>> >>> >> - Users should never edit a file that is in our source control. That's
>> >>> >> why we have .in files that we expect them to copy.
>> >>> >>
>> >>> >> - Unless they're an advanced user, they shouldn't need to copy the
>> >>> >> config file for advanced options. That means that the tests should
>> >>> >> have defaults that match what is in the template advanced config file
>> >>> >> (or, the tests could read advanced.json.in if advanced.json doesn't
>> >>> >> exist, though that does seem a little icky). Of course, those are
>> >>> >> example filenames, not necessarily what you may choose.
>> >>> >>
>> >>> >> --
>> >>> >> Dave Page
>> >>> >> Blog: http://pgsnake.blogspot.com
>> >>> >> Twitter: @pgsnake
>> >>> >>
>> >>> >> EnterpriseDB UK: http://www.enterprisedb.com
>> >>> >> The Enterprise PostgreSQL Company
>> >>> >
>> >>> >
>> >>> >
>> >>> >
>> >>> > --
>> >>> > Best,
>> >>> > Priyanka
>> >>> >
>> >>> > EnterpriseDB Corporation
>> >>> > The Enterprise PostgreSQL Company
>> >>>
>> >>>
>> >>>
>> >>> --
>> >>> 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(at)postgresql(dot)org)
>> >>> To make changes to your subscription:
>> >>> http://www.postgresql.org/mailpref/pgadmin-hackers
>> >>
>> >>
>> >>
>> >>
>> >> --
>> >> Best,
>> >> Priyanka
>> >>
>> >> EnterpriseDB Corporation
>> >> The Enterprise PostgreSQL Company
>> >
>> >
>> >
>> >
>> > --
>> > Best,
>> > Priyanka
>> >
>> > EnterpriseDB Corporation
>> > The Enterprise PostgreSQL Company
>>
>>
>>
>> --
>> Dave Page
>> Blog: http://pgsnake.blogspot.com
>> Twitter: @pgsnake
>>
>> EnterpriseDB UK: http://www.enterprisedb.com
>> The Enterprise PostgreSQL Company
>
>
>
> --
> Best,
> Priyanka
>
> EnterpriseDB Corporation
> The Enterprise PostgreSQL Company

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Harshal Dhumal 2016-07-04 14:00:35 Re: Fix for issue RM1336 [pgadmin4]
Previous Message Priyanka Shendge 2016-07-04 11:16:34 Re: pgAdmin IV API test cases patch