public inbox for [email protected]  
help / color / mirror / Atom feed
From: Dave Page <[email protected]>
To: Joao De Almeida Pereira <[email protected]>
Cc: Anthony Emengo <[email protected]>
Cc: pgadmin-hackers <[email protected]>
Subject: Re: [pgadmin4][patch] Use pytest test runner for unit tests
Date: Fri, 25 May 2018 10:31:19 +0100
Message-ID: <CA+OCxowt6nQrdNj-tKupZ_ohJeGyH20G3VfNVjKa85RgVe+A1g@mail.gmail.com> (raw)
In-Reply-To: <CAE+jjam92a4rwYM+VxekLTRjLoWdAiPw4MuiCP1dUT_aWc5-6g@mail.gmail.com>
References: <CAG8BBZM0gapFd0ZvMq7euCYamxKEBNqewC4YX4bYddyc_6AatA@mail.gmail.com>
	<CA+OCxoyhMprhxCMss5oESTMcVVnVcyBOP_AuntNyZsRyeZu5rg@mail.gmail.com>
	<CAG8BBZOkQ8FPzXzJAwe5+EgAKEhbpcx+_=XQh3UQhWQE620xqg@mail.gmail.com>
	<CA+OCxozBpu020t4+tagcvMStjHakp7aEduGVOFgM8quaRvmxCg@mail.gmail.com>
	<CAE+jjam92a4rwYM+VxekLTRjLoWdAiPw4MuiCP1dUT_aWc5-6g@mail.gmail.com>

Hi

On Thu, May 24, 2018 at 8:36 PM, Joao De Almeida Pereira <
[email protected]> wrote:

> As part of the development environment we do not see the reasoning behind
> not add PYTHONPATH to the environment variables, specially because this
> looks like the way pytest was invisoned.
>

Really? It's one more step that wasn't previously required, and for which
there is no good reason when running in a properly configured virtual
environment. Not only that, but PYTHONPATH is typically used as a search
path to find modules on which the application/script is dependent - not to
find the application/script itself. Unconditionally overriding it is likely
to lead to problems in some possible configurations (at the very least I
would expect to see the Makefile do PYTHONPATH=$PYTHONPATH:$(PWD)/web).


>
> However please try the following patch instead. We've changed the pytest
> invocation to assume the relevant dir as part of the directories to load,
> as well as the docs and Makefile
>

Some initial feedback:

- The JSON results are no longer being generated.

- The output is *extremely* verbose, showing a lot of seemingly unnecessary
info (do we really need to output the source code of failing test cases?). I
would suggest that the verbose output be directed to the log, and the
visible output should be much more terse along the lines of what it is now.

- There is no summary at the end showing what passed, failed or was
skipped. To figure out what to look at, I have to trawl through over 13K
lines of output (642KB).

- 69 tests failed for me running test:unit. They were previously all
passing.

- It is a *lot* faster - not sure if that's a result of tests failing, but
I expect not entirely.

- /README was updated, but not /web/regression/README

Thanks.

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

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


view thread (17+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected]
  Subject: Re: [pgadmin4][patch] Use pytest test runner for unit tests
  In-Reply-To: <CA+OCxowt6nQrdNj-tKupZ_ohJeGyH20G3VfNVjKa85RgVe+A1g@mail.gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox