MIME-Version: 1.0
From: Atira Odhner <aodhner@pivotal.io>
Date: Tue, 17 Jan 2017 11:09:48 -0500
Message-ID: 
 <CA+Vc24rPt8BGgtZM5Pv4CARpw7t42A5aqQX5q0_QyWWOfgsGTA@mail.gmail.com>
Subject: Re: Acceptance Tests against a browser (WIP)
To: pgadmin-hackers@postgresql.org
Cc: George Gelashvili <ggelashvili@pivotal.io>
Content-Type: multipart/alternative; boundary=001a114ac04883821c05464c87cc
Precedence: bulk
Sender: pgadmin-hackers-owner@postgresql.org

--001a114ac04883821c05464c87cc
Content-Type: text/plain; charset=UTF-8

Thanks for your feedback, Dave!

We can put the tests under the regression directory. I think that makes
sense.
I'm not picturing these tests being module specific, but we may want to
enable running it as a separate suite of tests.

Thanks for the callout about the port and title. We'll make sure those are
pulled from config or that the pgAdmin server is spun up by the test with
specific values.

I have a couple ideas about why the test might not have been running for
you. I think the patch we attached didn't spin up its own pgAdmin yet and
it definitely doesn't fill in username/password if your app is running that
way. That's part of the WIP-ness :-P

-Tira

Hi

On Thu, Jan 12, 2017 at 10:41 PM, George Gelashvili
<ggelashvili(at)pivotal(dot)io> wrote:
> here's the patch we forgot to attach. Also, you can see work on our branch
> at:
> https://github.com/pivotalsoftware/pgadmin4/tree/pivotal/acceptance-tests
>
> On Thu, Jan 12, 2017 at 5:26 PM, George Gelashvili <ggelashvili(at)pivotal(dot)io>
> wrote:
>>
>> Hi there,
>>
>> We are working on browser-automation-based acceptance tests that exercise
>> pgAdmin4 the way a user might.

Nice!

>> The first "connect to database" test works, but at the moment depends on
>> Chrome and chromedriver. We would appreciate feedback on any possible
>> license or code style issues at this point, as well as any thoughts on
>> adding this sort of test to the codebase.

A few thoughts:

- If these tests are to run as part of the regression suite, the
framework for them should live under that directory.

- Are any of the tests likely to be module-specific? If so, they
should really be part of the relevant module as the regression tests
are. If they're more general/less tightly coupled, then I don't see a
problem with them residing where they are.

- Please take care not to include changes to .gitgnore files that
aren't relevant to the rest of us.

- The port number is hard-coded in the test.

- You've hard-coded the string "pgAdmin 4". We've tried to keep that
title as a config option in config.py, so you should pull the string
from there rather than hard-coding it.

- The connect test fails for me (Mac, Python 2.7). I have a suspicion
that this may be because when the test starts chromedriver, OS X
prompts the user about whether a listening port should be opened, but
the tests don't wait (though, I tested with 3 servers configured and
it failed with the same error on the second and third as well, long
after I clicked OK on the prompt):

Traceback (most recent call last):
  File "/Users/dpage/git/pgadmin4/web/acceptance/test_connects_to_database.py",
line 32, in runTest
    self.assertEqual("pgAdmin 4", self.driver.title)
AssertionError: 'pgAdmin 4' != u'localhost'

- Please keep tests in the pgadmin. namespace (pgadmin.acceptance.??).

- It looks like running a single test won't work yet (because of
TestsGeneratorRegistry.load_generators('pgadmin.%s.tests' %
arguments['pkg']))

Thanks!

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

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

--001a114ac04883821c05464c87cc
Content-Type: text/html; charset=UTF-8
Content-Transfer-Encoding: quoted-printable

<div dir=3D"ltr"><div>Thanks for your feedback, Dave!</div><div><br></div><=
div>We can put the tests under the regression directory. I think that makes=
 sense.=C2=A0</div><div>I&#39;m not picturing these tests being module spec=
ific, but we may want to enable running it as a separate suite of tests.=C2=
=A0</div><div><br></div><div>Thanks for the callout about the port and titl=
e. We&#39;ll make sure those are pulled from config or that the pgAdmin ser=
ver is spun up by the test with specific values.=C2=A0</div><div><br></div>=
<div>I have a couple ideas about why the test might not have been running f=
or you. I think the patch we attached didn&#39;t spin up its own pgAdmin ye=
t and it definitely doesn&#39;t fill in username/password if your app is ru=
nning that way. That&#39;s part of the WIP-ness :-P</div><div><br></div><di=
v>-Tira</div><div><br></div><div><pre style=3D"font-size:1.2em;word-wrap:br=
eak-word;white-space:pre-wrap;padding:1em 0px 1em 2em;border-width:2px 0px;=
border-color:lightgray;border-style:solid;color:rgb(0,0,0)">Hi

On Thu, Jan 12, 2017 at 10:41 PM, George Gelashvili
&lt;ggelashvili(at)pivotal(dot)io&gt; wrote:
&gt; here&#39;s the patch we forgot to attach. Also, you can see work on ou=
r branch
&gt; at:
&gt; <a href=3D"https://github.com/pivotalsoftware/pgadmin4/tree/pivotal/ac=
ceptance-tests" rel=3D"nofollow" style=3D"color:rgb(0,83,110)">https://gith=
ub.com/pivotalsoftware/pgadmin4/tree/pivotal/acceptance-tests</a>
&gt;
&gt; On Thu, Jan 12, 2017 at 5:26 PM, George Gelashvili &lt;ggelashvili(at)=
pivotal(dot)io&gt;
&gt; wrote:
&gt;&gt;
&gt;&gt; Hi there,
&gt;&gt;
&gt;&gt; We are working on browser-automation-based acceptance tests that e=
xercise
&gt;&gt; pgAdmin4 the way a user might.

Nice!

&gt;&gt; The first &quot;connect to database&quot; test works, but at the m=
oment depends on
&gt;&gt; Chrome and chromedriver. We would appreciate feedback on any possi=
ble
&gt;&gt; license or code style issues at this point, as well as any thought=
s on
&gt;&gt; adding this sort of test to the codebase.

A few thoughts:

- If these tests are to run as part of the regression suite, the
framework for them should live under that directory.

- Are any of the tests likely to be module-specific? If so, they
should really be part of the relevant module as the regression tests
are. If they&#39;re more general/less tightly coupled, then I don&#39;t see=
 a
problem with them residing where they are.

- Please take care not to include changes to .gitgnore files that
aren&#39;t relevant to the rest of us.

- The port number is hard-coded in the test.

- You&#39;ve hard-coded the string &quot;pgAdmin 4&quot;. We&#39;ve tried t=
o keep that
title as a config option in config.py, so you should pull the string
from there rather than hard-coding it.

- The connect test fails for me (Mac, Python 2.7). I have a suspicion
that this may be because when the test starts chromedriver, OS X
prompts the user about whether a listening port should be opened, but
the tests don&#39;t wait (though, I tested with 3 servers configured and
it failed with the same error on the second and third as well, long
after I clicked OK on the prompt):

Traceback (most recent call last):
  File &quot;/Users/dpage/git/pgadmin4/web/acceptance/test_connects_to_data=
base.py&quot;,
line 32, in runTest
    self.assertEqual(&quot;pgAdmin 4&quot;, self.driver.title)
AssertionError: &#39;pgAdmin 4&#39; !=3D u&#39;localhost&#39;

- Please keep tests in the pgadmin. namespace (pgadmin.acceptance.??).

- It looks like running a single test won&#39;t work yet (because of
TestsGeneratorRegistry.load_generators(&#39;pgadmin.%s.tests&#39; %
arguments[&#39;pkg&#39;]))

Thanks!

--=20
Dave Page
Blog: <a href=3D"http://pgsnake.blogspot.com/" rel=3D"nofollow" style=3D"co=
lor:rgb(0,83,110)">http://pgsnake.blogspot.com</a>
Twitter: @pgsnake

EnterpriseDB UK: <a href=3D"http://www.enterprisedb.com/" rel=3D"nofollow" =
style=3D"color:rgb(0,83,110)">http://www.enterprisedb.com</a>
The Enterprise PostgreSQL Company
</pre></div><div><br></div></div>

--001a114ac04883821c05464c87cc--