Re: [pgadmin4][patch] Use pytest test runner for unit tests

From: Joao De Almeida Pereira <jdealmeidapereira(at)pivotal(dot)io>
To: Dave Page <dpage(at)pgadmin(dot)org>
Cc: Anthony Emengo <aemengo(at)pivotal(dot)io>, pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org>
Subject: Re: [pgadmin4][patch] Use pytest test runner for unit tests
Date: 2018-05-25 11:35:53
Message-ID: CAE+jjakFfSjDWyyQEwQrAsveSSWvJ_cpcNESeC6F4TShZqfP8Q@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

Hello Dave

> 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).
>
Good point we didn't consider the possibility of someone developing
multiple python apps in the same machine.
We will revisit this and start using
python -m pytest
as that should solve the problem

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

That is a flag that we didn't add to the script bug we will review the
patch with that

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

We can see what we can do about this point

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

If we use the flag -q the output is smaller. We can add that too

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

Can you provide some log of the failing tests?

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

Out lapse, will do that

Joao

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Fahar Abbas 2018-05-25 11:42:32 Re: Nightly snapshot builds
Previous Message Joao De Almeida Pereira 2018-05-25 11:29:13 Re: Nightly snapshot builds