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-06-24 10:47:21
Message-ID: CA+OCxoyiNMAem5bYAZ1XM16vK4wJETCCPncRgKYbVdmH2m4Ocg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

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? We should have just one set of credentials for
everything.

>>>
>>> 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

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Murtuza Zabuawala 2016-06-24 11:24:30 Re: Re: [pgAdmin 4 - Bug #1292] ERROR: template database "!@#$%^&*()_+{}|:"<>?=-\\][';/.," does not exist message throws if template database contain special charterers
Previous Message Murtuza Zabuawala 2016-06-24 10:20:27 Re: PACTH: To fix the issue while selecting primary key in table (pgAdmin4)