Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA384:256) (Exim 4.89) (envelope-from ) id 1erkn2-0000b8-UJ for pgadmin-hackers@arkaria.postgresql.org; Fri, 02 Mar 2018 13:32:01 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.89) (envelope-from ) id 1erkn1-0000Ya-NN for pgadmin-hackers@arkaria.postgresql.org; Fri, 02 Mar 2018 13:31:59 +0000 Received: from magus.postgresql.org ([2a02:c0:301:0:ffff::29]) by malur.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA384:256) (Exim 4.89) (envelope-from ) id 1erkn1-0000YQ-I2 for pgadmin-hackers@lists.postgresql.org; Fri, 02 Mar 2018 13:31:59 +0000 Received: from mail-wr0-x242.google.com ([2a00:1450:400c:c0c::242]) by magus.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.89) (envelope-from ) id 1erkmx-0001mC-SV for pgadmin-hackers@postgresql.org; Fri, 02 Mar 2018 13:31:59 +0000 Received: by mail-wr0-x242.google.com with SMTP id p104so10092157wrc.12 for ; Fri, 02 Mar 2018 05:31:55 -0800 (PST) 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=teQIG6s46EBzj18PexbE72U2CSm90S7/k9A+cZ/UETc=; b=0qGVMtcO9ZrKOdB7U4dNYG99nwiIH31zYO+DH/v7/QgPsDy+AkcqP66+WMSvyTQk9o 68R745Y/qQwkkRpIolY7ghj/8aBa7rL58frfrbZdT9hSAMuXuHoJDqVPoSuNCD3RxP/u /PPoPknZsA1mDvI+H8SMIe6bKUy/H/X2V7zm1TrD9m+pEMUwUCWtT7zfjuj+p3tSfe9O yzKsjW2pAxCp46kHizmD5zJLK06qZKL4C3Ra6Bt70WHhb/cv7xwh/AtL61Nxfv8sqntM 4u7jpG3IpUHTCbLBMs0evIz4eCmL9+AJeWmgYDjooiptrG3kPIIv4CPg5rC7tO9V5j5F IRsw== 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=teQIG6s46EBzj18PexbE72U2CSm90S7/k9A+cZ/UETc=; b=DpmsrUOYMjL9IZOatA9yqYy3sDrDJoa/nD4h187GlwjmZw8DlvMknvKW7nOA1d9WGO 69Kxvj2jLXrY9yLWExNgRx3DdemMTAn7XODXSyGbbx2Cqoje6SIXPB7+IsmmmSLfW13J RgmDGS0eGgM0svlCwbTGBrHaVlT/RAJdmMo445sh9hjoKkmVL1nvoqpB3+hQxLgZqQUs n2eDF/FEOCpr5do2BIJdEffS0HO2QrrzTpQyUuMTR8DW+jMH9qLrLCTmK4bPDzw/e6uk phjdR62PSqyfgdoV/R9E9VEcUDa3kPOtrCvCaPYP7cL0P5KV489uuYWx6pnPEbXtVH4U jCaA== X-Gm-Message-State: APf1xPBV4RYVXccRZmG3en6O6TjIxv7KP8sCQTfZJH4yP7wrRjWs0WMU 3UdH6FmiibZI1WaelyNmsbSIBExRRjLosuEVrMSvaw== X-Google-Smtp-Source: AG47ELs5apmWScEU6OmWXMrKgatR2PjJnnDOk63+fqBGMY3ovxise8mm88C7ShznXvb05F73YNQwy+YXwiqhS6Es+jo= X-Received: by 10.223.178.206 with SMTP id g72mr4713361wrd.135.1519997515075; Fri, 02 Mar 2018 05:31:55 -0800 (PST) MIME-Version: 1.0 Received: by 10.28.109.7 with HTTP; Fri, 2 Mar 2018 05:31:54 -0800 (PST) In-Reply-To: References: From: Dave Page Date: Fri, 2 Mar 2018 13:31:54 +0000 Message-ID: Subject: Re: pgAdmin 4 commit: Ensure we pick up the messages from the current query To: Joao De Almeida Pereira Cc: Khushboo Vashi , Murtuza Zabuawala , pgadmin-hackers Content-Type: multipart/alternative; boundary="f403045cf6b8f225e905666dff7c" List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Precedence: bulk --f403045cf6b8f225e905666dff7c Content-Type: text/plain; charset="UTF-8" On Thu, Mar 1, 2018 at 3:21 PM, Joao De Almeida Pereira < jdealmeidapereira@pivotal.io> wrote: > Hello Khushboo, > The patch runs successfully in our CI with all tests passing. > > I see the test that you created, and I do not understand why we need to > create tests that do HTTP requests in order to check something that is > executed against the database. What I was talking about in my previous > email was having tests that tested the function by itself. > > > (Copied from: https://jfiaffe.files.wordpress.com/2014/09/tests- > pyramid.png) > > This is the Testing Pyramid, there are a bunch of different drawings of it > and ways to explain it, but in broad stokes what is means is that we should > have the majority of tests around a Unit (that are some disagreements in > the community of what a Unit is) and a very small amount of Manual testing. > What Unit usually means is piece of code, it can be a function, it can be a > class or it can even be a module, but is something self contained. In > pgAdmin's case the majority of our tests go around the Integration Layer > because we are using HTTP requests in order to executing queries in the > database, so basically we are doing tests end to end in the backend, and > the cost time. > > I do not want to held this patch back because of this, and I say this > because I have minimal confidence with the tests that you created, that > they would catch the majority of the problems, and hope that the majority > of the code is exercised by it. > > Nevertheless I would like to challenge all the Hackers to think about > testing in a different way. The tests in our code are used to give us > confidence that the work we did is working as expected, this also makes it > much easier to refactor out bad patterns or very complicated ones into > something simple. A signal that our code is more complicated then it should > is when we need to test some behavior and we end up with a Stubbing Hell or > we need to test it End 2 End because it is to hard to isolate the part we > want to test. In the other hand we should not test all functions and every > class, because we might be coupling our tests to much to the implementation > and that will have the contrary effect, and we will not be able to refactor > and simply our code. > Like everything in life there need to be a balance. > I think that is very good advice, and I would also like to encourage all the developers to think this way. However, I would also caution against underestimating the importance of our feature tests. It is obviously important to have confidence that our code is robust and functions correctly at the unit level, but that doesn't mean that it all works together as expected to provide the user functionality we desire. I think the feature tests are critical in this regard; they protect us against class of bug that might otherwise be missed without manual testing that we strive to eliminate entirely. For the EDBers on the team, think of the feature tests in terms of what we normally call integration testing; ensuring that not only do the individual pieces work as they should, but that they work together as they should. -- Dave Page Blog: http://pgsnake.blogspot.com Twitter: @pgsnake EnterpriseDB UK: http://www.enterprisedb.com The Enterprise PostgreSQL Company --f403045cf6b8f225e905666dff7c Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable


On Thu, Mar 1, 2018 at 3:21 PM, Joao De Almeida Pereira <jd= ealmeidapereira@pivotal.io> wrote:
Hello Khushboo,
The patch runs successfully in= our CI with all tests passing.

I see the test tha= t you created, and I do not understand why we need to create tests that do = HTTP requests in order to check something that is executed against the data= base. What I was talking about in my previous email was having tests that t= ested the function by itself.

=C2=A0

This is the= Testing Pyramid, there are a bunch of different drawings of it and ways to= explain it, but in broad stokes what is means is that we should have the m= ajority of tests around a Unit (that are some disagreements in the communit= y of what a Unit is) and a very small amount of Manual testing. What Unit u= sually means is piece of code, it can be a function, it can be a class or i= t can even be a module, but is something self contained. In pgAdmin's c= ase the majority of our tests go around the Integration Layer because we ar= e using HTTP requests in order to executing queries in the database, so bas= ically we are doing tests end to end in the backend, and the cost time.

I do not want to held this patch back because of this= , and I say this because I have minimal confidence with the tests that you = created, that they would catch the majority of the problems, and hope that = the majority of the code is exercised by it.

Never= theless I would like to challenge all the Hackers to think about testing in= a different way. The tests in our code are used to give us confidence that= the work we did is working as expected, this also makes it much easier to = refactor out bad patterns or very complicated ones into something simple. A= signal that our code is more complicated then it should is when we need to= test some behavior and we end up with a Stubbing Hell or we need to test i= t End 2 End because it is to hard to isolate the part we want to test. In t= he other hand we should not test all functions and every class, because we = might be coupling our tests to much to the implementation and that will hav= e the contrary effect, and we will not be able to refactor and simply our c= ode.
Like everything in life there need to be a balance.

I think that is very good advice, and I= would also like to encourage all the developers to think this way. However= , I would also caution against underestimating the importance of our featur= e tests. It is obviously important to have confidence that our code is robu= st and functions correctly at the unit level, but that doesn't mean tha= t it all works together as expected to provide the user functionality we de= sire. I think the feature tests are critical in this regard; they protect u= s against class of bug that might otherwise be missed without manual testin= g that we strive to eliminate entirely. For the EDBers on the team, think o= f the feature tests in terms of what we normally call integration testing; = ensuring that not only do the individual pieces work as they should, but th= at they work together as they should.=C2=A0

--
Dave Pa= ge
Blog: http:= //pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterpris= edb.com
The Enterprise PostgreSQL Company
--f403045cf6b8f225e905666dff7c--