Re: Acceptance Tests against a browser (WIP)

From: George Gelashvili <ggelashvili(at)pivotal(dot)io>
To: Dave Page <dpage(at)pgadmin(dot)org>
Cc: Atira Odhner <aodhner(at)pivotal(dot)io>, pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org>
Subject: Re: Acceptance Tests against a browser (WIP)
Date: 2017-01-26 22:40:26
Message-ID: CAHowoHa4k7E6LBDaiuUFPBKOcsa_ML7M3KEhDZUsbR-KKujemw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

instead of that patch, please use this no-zombies version that kills the
started process group instead of pid-only.

On Wed, Jan 25, 2017 at 6:31 PM, George Gelashvili <ggelashvili(at)pivotal(dot)io>
wrote:

> ah
> That diff was generated before the python 3 patch was applied. This should
> work against master
>
> Cheers,
> George
>
> On Tue, Jan 24, 2017 at 4:43 AM, Dave Page <dpage(at)pgadmin(dot)org> wrote:
>
>> On Fri, Jan 20, 2017 at 5:33 PM, George Gelashvili
>> <ggelashvili(at)pivotal(dot)io> wrote:
>> > Thanks for bringing that to our attention! Here's the latest patch
>>
>> piranha:pgadmin4 dpage$ git apply
>> ~/Downloads/acceptance-tests-with-server-start-and-polling.diff
>> error: patch failed: web/regression/test_utils.py:69
>> error: web/regression/test_utils.py: patch does not apply
>>
>> :-(
>>
>> > On Fri, Jan 20, 2017 at 10:38 AM, Dave Page <dpage(at)pgadmin(dot)org> wrote:
>> >>
>> >> On Thu, Jan 19, 2017 at 10:07 PM, George Gelashvili
>> >> <ggelashvili(at)pivotal(dot)io> wrote:
>> >> >
>> >> > Here is an updated patch which starts the server up when the test
>> starts
>> >> > and
>> >> > uses the values from config.py for server name etc. It still requires
>> >> > installing chromedriver before running. Should we add something to
>> the
>> >> > readme about that?
>> >>
>> >> Yes, we definitely should (including download site URL)
>> >>
>> >> > On Tue, Jan 17, 2017 at 11:09 AM, Atira Odhner <aodhner(at)pivotal(dot)io>
>> >> > wrote:
>> >> >>
>> >> >> 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
>> >> >>
>> >> >>
>> >> >
>> >> >
>> >> >
>> >> > --
>> >> > Sent via pgadmin-hackers mailing list (pgadmin-hackers(at)postgresql(dot)or
>> g)
>> >> > To make changes to your subscription:
>> >> > http://www.postgresql.org/mailpref/pgadmin-hackers
>> >> >
>> >>
>> >>
>> >>
>> >> --
>> >> Dave Page
>> >> Blog: http://pgsnake.blogspot.com
>> >> Twitter: @pgsnake
>> >>
>> >> EnterpriseDB UK: http://www.enterprisedb.com
>> >> The Enterprise PostgreSQL Company
>> >
>> >
>>
>>
>>
>> --
>> Dave Page
>> Blog: http://pgsnake.blogspot.com
>> Twitter: @pgsnake
>>
>> EnterpriseDB UK: http://www.enterprisedb.com
>> The Enterprise PostgreSQL Company
>>
>
>

Attachment Content-Type Size
acceptance-test-no-zombies.diff text/plain 13.0 KB

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Dave Page 2017-01-27 09:20:08 pgAdmin 4 commit: Request Six>=1.9.0 as the current version is 1.10.0 a
Previous Message George Gelashvili 2017-01-26 20:48:01 Re: [Patch] Refactor sql template version picking