Received: from malur.postgresql.org ([2a02:16a8:dc51::56]) by arkaria.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA384:256) (Exim 4.89) (envelope-from ) id 1fM94R-00048l-8i for pgadmin-hackers@arkaria.postgresql.org; Fri, 25 May 2018 09:31:35 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.89) (envelope-from ) id 1fM94P-0003qp-VX for pgadmin-hackers@arkaria.postgresql.org; Fri, 25 May 2018 09:31:33 +0000 Received: from makus.postgresql.org ([2001:4800:1501:1::229]) by malur.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA384:256) (Exim 4.89) (envelope-from ) id 1fM94P-0003qf-NE for pgadmin-hackers@lists.postgresql.org; Fri, 25 May 2018 09:31:33 +0000 Received: from mail-wr0-x243.google.com ([2a00:1450:400c:c0c::243]) by makus.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.89) (envelope-from ) id 1fM94E-0001Mq-5J for pgadmin-hackers@postgresql.org; Fri, 25 May 2018 09:31:32 +0000 Received: by mail-wr0-x243.google.com with SMTP id a15-v6so8045891wrm.0 for ; Fri, 25 May 2018 02:31:21 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pgadmin-org.20150623.gappssmtp.com; s=20150623; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=vSLbc/crNGzycQ1wPurV5D2Ov2flolB3yNT8qoD80Lg=; b=wyZ+RDriqAYfTnsi/uOTBqyT9lsU1PR/q4Y8kTRL8K4rn1FIcQVGOy7jWefrH2ZQPO Ka0qujSiRPmWHdke1FZj7xeOOHxlRXv2PScugESAxLaxuXNBZ1P3Hl46ZZFI6zkaZzNd /H9Aj9TT6LtctC4Wgrm6H8ayqYTwDZsfUoNUS/MLJs4V3aOvYn2M7eZ0GAB58nF9fY3N oqAJtax/ll+yk+j++6hMNIyXWaisqRDB4FtBvkbTEndEmlASrOX8SvRd1J65MffdE7+V cvdmJMa9A35hYKuA0ZuqP4s5cy3XmfkMhYMyeUK1Cy1y1b8CyTH7RH61EpVVDK+4fn7N oEog== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=vSLbc/crNGzycQ1wPurV5D2Ov2flolB3yNT8qoD80Lg=; b=CJilcn0h+5cZlVPj+/epGqmFEIABisTIjnj55J4uLOseK+iONN/EG1vdkrHnDMryX6 7dnzY7IkvLFZRFI9S4XjfAVBGMd3lVUdO+wRRE/Yodn7rYKgt6vF3BR1VWc7BgUDAc03 XG+XobHdddXsPY8YBry25aSXbF+vkReg1y/q+uLp6z8iy7bS30BewHcUhrDQYtxeSa6b gJz5Q7y5A8YAcTpxULVjWucszUfjbteFBXUmVf3X4nwTOT9IvFHUA39UL7QE/s0o1nro N5rvR6TmilEhPirLH+BYLX88urx6acjvvfZ/1xGiFXoPN0KKmSyeupaW2spDRw97NnBZ gsew== X-Gm-Message-State: ALKqPwdBupYTUPb96BNKSnby80gfpLUwkQbLEdhiiq5rwTqr2maEoEuV jTZsm89FBWXMvIeQnmPpUbfyLRrbVArYjLEbqrrIkw== X-Google-Smtp-Source: AB8JxZo1YEai9DnjlVleCJMFahEvvHsdgfB38PXCUYMW5QhDxOhcwhzn5kqmz94pa4ezvjvsPp+WApNWQ2+U2XAtXnk= X-Received: by 2002:adf:86b2:: with SMTP id 47-v6mr1404454wrx.256.1527240680385; Fri, 25 May 2018 02:31:20 -0700 (PDT) MIME-Version: 1.0 Received: by 2002:a1c:55c2:0:0:0:0:0 with HTTP; Fri, 25 May 2018 02:31:19 -0700 (PDT) In-Reply-To: References: From: Dave Page Date: Fri, 25 May 2018 10:31:19 +0100 Message-ID: Subject: Re: [pgadmin4][patch] Use pytest test runner for unit tests To: Joao De Almeida Pereira Cc: Anthony Emengo , pgadmin-hackers Content-Type: multipart/alternative; boundary="0000000000003dc155056d046e5d" List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Precedence: bulk --0000000000003dc155056d046e5d Content-Type: text/plain; charset="UTF-8" Hi On Thu, May 24, 2018 at 8:36 PM, Joao De Almeida Pereira < jdealmeidapereira@pivotal.io> 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 --0000000000003dc155056d046e5d Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Hi

On Thu, May 24, 2018 at 8:36 PM, Joao De Almeida Pereira <jdealmeidapereira@pivotal.io> wrote:
As part of the developm= ent environment we do not see the reasoning behind not add PYTHONPATH to th= e environment variables, specially because this looks like the way pytest w= as invisoned.

Really? It's = one more step that wasn't previously required, and for which there is n= o good reason when running in a properly configured virtual environment. No= t only that, but PYTHONPATH is typically used as a search path to find modu= les on which the application/script is dependent - not to find the applicat= ion/script itself. Unconditionally overriding it is likely to lead to probl= ems in some possible configurations (at the very least I would expect to se= e the Makefile do PYTHONPATH=3D$PYTHONPATH:$(PWD)/web).
=C2=A0
=
However please try the following patch instead. We've changed= the pytest invocation to assume the relevant dir as part of the directorie= s to load, as well as the docs and Makefile

Some initial feedback:

- The JSON results a= re no longer being generated.

- The output is *ext= remely* verbose, showing a lot of seemingly unnecessary info (do we really = need to output the source code of failing test cases?).=C2=A0I 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 i= t 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 al= l passing.

- It is a *lot* faster - not sure if th= at'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
--0000000000003dc155056d046e5d--