On Thu, Mar 1, 2018 at 3:21 PM, Joao De Almeida Pereira < jdealmeidapere...@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
