Chris Morgan wrote:
Yes, having quotes around limit values breaks sql queries. I'll incorporate this into the injection change patch.
I'm curious as to why the rest of the patch is the same though. It will conflict when the other sql patch is applied.
What other sql patch? How will it conflict? I have broken your large patch up in order to test it, since you refused to do it yourself. This is the portion of the patch that I tested. I had to modify it a bit like I said but the rest is yours and you get the credit.
What do you plan on doing with this patch? Are you planning to wait until I have tested all various parts of your big patch and then apply it all at once?
--
Tony Lambregts
On Monday 26 June 2006 11:38 pm, Tony Lambregts wrote:
Chris Morgan wrote:
Yes, having quotes around limit values breaks sql queries. I'll incorporate this into the injection change patch.
I'm curious as to why the rest of the patch is the same though. It will conflict when the other sql patch is applied.
What other sql patch? How will it conflict? I have broken your large patch up in order to test it, since you refused to do it yourself. This is the portion of the patch that I tested. I had to modify it a bit like I said but the rest is yours and you get the credit.
What do you plan on doing with this patch? Are you planning to wait until I have tested all various parts of your big patch and then apply it all at once?
--
Tony Lambregts
As we've discussed before I'd rather we did a single full pass of manual testing than several full passes. It saves us time in that we don't have to test the same things repeatedly like we would have to do when making changes to things like classes that are used all over the code.
In any case I'm implementing unit tests for nearly every bug I find. I haven't thought of a good way to unit test page actions yet though.
Chris
Chris Morgan wrote:
On Monday 26 June 2006 11:38 pm, Tony Lambregts wrote:
Chris Morgan wrote:
Yes, having quotes around limit values breaks sql queries. I'll incorporate this into the injection change patch.
I'm curious as to why the rest of the patch is the same though. It will conflict when the other sql patch is applied.
What other sql patch? How will it conflict? I have broken your large patch up in order to test it, since you refused to do it yourself. This is the portion of the patch that I tested. I had to modify it a bit like I said but the rest is yours and you get the credit.
What do you plan on doing with this patch? Are you planning to wait until I have tested all various parts of your big patch and then apply it all at once?
--
Tony Lambregts
As we've discussed before I'd rather we did a single full pass of manual testing than several full passes. It saves us time in that we don't have to test the same things repeatedly like we would have to do when making changes to things like classes that are used all over the code.
Your logic is flawed and only applies if the patch has no bugs. By breaking up the patch into smaller pieces you save time in testing when there are problems.
In any case I'm implementing unit tests for nearly every bug I find. I haven't thought of a good way to unit test page actions yet though.
I have no answer for that.
--
Tony Lambregts
As we've discussed before I'd rather we did a single full pass of manual testing than several full passes. It saves us time in that we don't have to test the same things repeatedly like we would have to do when making changes to things like classes that are used all over the code.
Your logic is flawed and only applies if the patch has no bugs. By breaking up the patch into smaller pieces you save time in testing when there are problems.
When were you going to report the issue with query_parameters() that I found the other day and sent in a unit test and patch for? Why do projects like Wine have automated tests instead of manual ones? I think we want to mirror successful projects like this by automating our testing. Manual testing may catch some bugs but it is always going to be more time consuming and less reproducable.
As I've said before, manual testing is ok but isn't likely to be as good as automated testing. That function is used in all sql calls. Modifying it should mean that we have to check EVERY sql call in the appdb.
In any case I'm implementing unit tests for nearly every bug I find. I haven't thought of a good way to unit test page actions yet though.
I have no answer for that.
The trick is getting the parameters into $_REQUEST variables. I think we'll have to build up a <input> form in the test and then submit it as if the user clicked on 'submit'. That might require duplication of code how we currently have some of those pages designed but we may be able to refactor them into classes so we can reduce the duplication.
Chris
Chris Morgan wrote:
As we've discussed before I'd rather we did a single full pass of manual testing than several full passes. It saves us time in that we don't have to test the same things repeatedly like we would have to do when making changes to things like classes that are used all over the code.
Your logic is flawed and only applies if the patch has no bugs. By breaking up the patch into smaller pieces you save time in testing when there are problems.
When were you going to report the issue with query_parameters() that I found the other day and sent in a unit test and patch for?
Huh? Perhaps I would not have found it. It is Good you caught a bug in your own code.
Why do projects like Wine have automated tests instead of manual ones? I think we want to mirror successful projects like this by automating our testing. Manual testing may catch some bugs but it is always going to be more time consuming and less reproducable.
You still seem to be under the impression I am against automated testing. It is not a case of Manual vs Automated testing. Yes Wine has automated testing but it does not prevent regressions occurring. Wine has literaly hundreds of manual testers that use bugzilla and the AppDB to report how well wine is doing. With wine if a regression occurs at least the user has the option of using a previous version. Users of the AppDB do not have that option.
There are lots of times that people have asked someone to break up a patch. In fact, it is in our documentation that it is considerd best practice to keep patches small.
http://www.winehq.org/site?page=sending_patches. http://www.winehq.org/site/docs/winedev-guide/style-notes
As I've said before, manual testing is ok but isn't likely to be as good as automated testing. That function is used in all sql calls. Modifying it should mean that we have to check EVERY sql call in the appdb.
To me if you are making a change to a function that is used "everywhere" then that goes into a patch by itself because we should test everywhere it is called.
Automated testing is only as good as the things it tests. If the test is flawed then it won't catch a bug and can give a false sense of security. Manual testing is only as good a the tests that the person does. Yes manual testing can be consuming but it is flexable. Neither are ideal.
I see that you have committed that patch while we were still discusing it. That tells me that my opinion just does not matter to you and you do not respect me.
I really hope that you tested it through and through.
--
Tony Lambregts
-
Chris Morgan -
Tony Lambregts