Hi, > I've reviewed the new version and now the coverage with this version is > 92%, the version 2 was 95%, probably this is because the test doesn't > cover the "UNION" case. > > I still wonder why you don't want to cover the 100% if it's just a small > changes in the test, but for now, this does third version it does > increase the coverage by 42%.
There is a line of code: ``` j->cross ? " CROSS " : " UNION " ``` The difference in coverage you are referring to is that in one case we cover both if-else branches while in another only one of them. In my opinion there is little practical value in covering both of them. Executing one line of the source code at least once is enough, regardless of what number `lcov` reports. This is always a compromise between the coverage, the ease of maintaining it and the speed of our tests on the buildfarm. If anyone else believes that we should do 92% -> 95% here I will submit the corrected patch though. -- Best regards, Aleksander Alekseev
