Thread: [pgadmin-hackers][patch] Test dependencies and screenshots

Hi

On Wed, Mar 1, 2017 at 9:34 PM, Joao Pedro De Almeida Pereira
<jpereira@pivotal.io> wrote:
> Hi Hackers
>
> We noticed that the feature test dependencies were removed in a recent
> patch, so we added them back.

They were intentionally moved - see:

https://www.postgresql.org/message-id/CA%2BOCxoxz8ZPsWBvEhSnMfq%3DW3QX-TaeXN9O9Gwbb%2Bf58X-9GQg%40mail.gmail.com

and

https://git.postgresql.org/gitweb/?p=pgadmin4.git;a=commit;h=1e5de7e66e7e578a7803087389d18102e0f5d945

> We also now create a screenshot with a
> timestamp when tests fail. This should help with debugging issues in CI.

Nice! There are a couple of minor issues though:

- The screenshots are saving with names like:
TemplateSelectionFeatureTest-2017-03-02 10/06/24.253843.png. Can you
please:

  * Change the / to a . or similar? Having forward slashes in
filenames is likely to be painful (the space should probably be
changed to an _ as well).
  * Store the screenshots in a sub-directory named for the server
that's currently being tested? That'll make it much easier to
associate a screenshot with the right server in multi-server runs
(each of my runs currently tests against 10 different servers for
example).

- I modified the ConnectToServer test to induce a failure. When I ran
the tests (on macOS Sierra, with Python 2.7) the testsuite took
screenshots for every test, not just the ones that failed. We should
only store them for tests that actually did fail.

Thanks, Dave.

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

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

Hi

On Thu, Mar 2, 2017 at 10:05 PM, Joao Pedro De Almeida Pereira
<jpereira@pivotal.io> wrote:
> Hi,
> We adjusted the patches to correct the issues that you mentioned
>
> Improves screenshots and reduces test flakiness

Hmm, was that the 0.5 second sleep? I hate doing that, but sometimes
it's the only way that doesn't require far more effort than it's worth
:-(.

>     - rename screenshot files to add python version
>     - put screenshots into separate pg version folders

I tested on Python 2.7 and Python 3.5 on macOS 10.12;

- On both versions of Python I'm left with a running chromedriver
process after the tests have exited. Chrome itself exits though.

- Screenshots only appear to have been saved once (the first time I
ran on Python 2.7). No screenshots were saved for any of my subsequent
runs.

I induced test failure by modifying the connect to server test to wait
for a menu option of "Server...." instead of "Server...".

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

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

On Fri, Mar 3, 2017 at 1:57 PM, Dave Page <dpage@pgadmin.org> wrote:
> Hi
>
> On Thu, Mar 2, 2017 at 10:05 PM, Joao Pedro De Almeida Pereira
> <jpereira@pivotal.io> wrote:
>> Hi,
>> We adjusted the patches to correct the issues that you mentioned
>>
>> Improves screenshots and reduces test flakiness
>
> Hmm, was that the 0.5 second sleep? I hate doing that, but sometimes
> it's the only way that doesn't require far more effort than it's worth
> :-(.
>
>>     - rename screenshot files to add python version
>>     - put screenshots into separate pg version folders
>
> I tested on Python 2.7 and Python 3.5 on macOS 10.12;
>
> - On both versions of Python I'm left with a running chromedriver
> process after the tests have exited. Chrome itself exits though.
>
> - Screenshots only appear to have been saved once (the first time I
> ran on Python 2.7). No screenshots were saved for any of my subsequent
> runs.

Well this is weird. The files are there if I look in the terminal, but
Finder is refusing to show them. If I search for them in spotlight,
they magically appear. I'd think I was going crazy but someone here
just confirmed I'm not.

So... assuming that's a Mac bug, we're just left with all the
chromedriver processes not dying - which is a show-stopper as we
obviously can't have an endlessly growing pile of processes on the
test server! :-)

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

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

Hi

On Fri, Mar 3, 2017 at 6:41 PM, Atira Odhner <aodhner@pivotal.io> wrote:
> Hey Dave,
>
> We're not able to reproduce that issue over here. Any chance you could debug
> it?

Per the discussion at
https://github.com/seleniumhq/selenium-google-code-issue-archive/issues/3378,
it looks like we're not closing the driver in the correct way (one
would think that close() means exit and cleanup whilst quit() means
bail out without regard for everyone else, but apparently not...).

Anyway, the attached patch seems to fix the issue for me on my laptop.
I'll go commit it - please yelp if you object.

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

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