Re: testing bug fixes - not!
Andreas, I wasn't trying to pick on you or anyone for that matter. It is something I've been meaning to bring up for some months actually. What I would hope to see is follows: (1) if there is a bug where a given input FO file causes an unhandled exception, and the bug fix is to fix the underlying problem causing the exception, then that input FO file, or a subset thereof, should be added as a test case, verified to fail prior to the fix, and verified not to fail after the fix; a regression after the fix would cause the exception to occur once again; (2) similarly, if the bug is reporting a failure to produce expected results for an existing feature; in this case, one or more test cases should be added to verify the expected results; (3) similarly, if a bug is requesting a new feature, and the bug fix is to add the feature, then there should be some reasonable attempt to add tests for that new feature; i'm not asking for 100% coverage of the feature, but something more than nothing is a good thing; (4) in the case of a performance fix, i would not say a test case (demonstrating the improvement) is required; of course, if someone was energetic enough to include a test case showing the performance delta, then that would be welcome; Of the above, I'm mostly concerned with the first and second cases. Regards, Glenn On Mon, Jan 24, 2011 at 3:42 PM, Andreas Delmelle < andreas.delme...@telenet.be> wrote: > > On 24 Jan 2011, at 23:17, Glenn Adams wrote: > > Hi Glenn > > > Is there a reason why *all* bug fixes are not accompanied by one or more > new test cases that demonstrate the presence and absence of the bug before > and after the fix? Adding such test cases should be mandatory for all bug > fix commits. I wouldn't quite go that far for performance and clean up > fixes, however, but for legitimate bugs, there should be a reliable way to > ensure that (1) the bug and its fix are testable and (2) that future > regressions do not occur. I realize it takes more effort, but it is worth it > in the long term, both in actual improvements in quality and the ability to > demonstrate consistently good practice to maintain that quality. > > FWIW, in case you are referring to some of my recent commits... > Any specific ones I should be taking a look at? Just following old habits, > I'm afraid, and try to add tests wherever appropriate, even if the fix > affects only a single line of code. > > Performance fixes may be rather difficult to test for, but even then one > could conjure up a way to test whether adding nodes remains a linear > operation, for example. We have no real framework set up for that type of > thing, but I guess I could invest some time in that. > > > > > Many Apache projects require this process be followed; I would urge the > FOP project to adopt a similar criterion for bug fix commits. > > > > > Basically agreed with your viewpoint, so point out where exactly you feel > something was missing and I'll see if I can accommodate you. > > > Regards, > > Andreas > --- > >
Re: testing bug fixes - not!
On 24 Jan 2011, at 23:17, Glenn Adams wrote: Hi Glenn > Is there a reason why *all* bug fixes are not accompanied by one or more new > test cases that demonstrate the presence and absence of the bug before and > after the fix? Adding such test cases should be mandatory for all bug fix > commits. I wouldn't quite go that far for performance and clean up fixes, > however, but for legitimate bugs, there should be a reliable way to ensure > that (1) the bug and its fix are testable and (2) that future regressions do > not occur. I realize it takes more effort, but it is worth it in the long > term, both in actual improvements in quality and the ability to demonstrate > consistently good practice to maintain that quality. FWIW, in case you are referring to some of my recent commits... Any specific ones I should be taking a look at? Just following old habits, I'm afraid, and try to add tests wherever appropriate, even if the fix affects only a single line of code. Performance fixes may be rather difficult to test for, but even then one could conjure up a way to test whether adding nodes remains a linear operation, for example. We have no real framework set up for that type of thing, but I guess I could invest some time in that. > > Many Apache projects require this process be followed; I would urge the FOP > project to adopt a similar criterion for bug fix commits. > Basically agreed with your viewpoint, so point out where exactly you feel something was missing and I'll see if I can accommodate you. Regards, Andreas ---
DO NOT REPLY [Bug 50635] Incorrect pagesequence passed to renderer
https://issues.apache.org/bugzilla/show_bug.cgi?id=50635 Andreas L. Delmelle changed: What|Removed |Added Status|NEW |RESOLVED Resolution||FIXED --- Comment #4 from Andreas L. Delmelle 2011-01-24 17:20:30 EST --- (In reply to comment #3) > To simplify my work, I wrote a new renderer, which creates a ZIP file > containing each page sequence as its own PDF file [each startPageSequence call > creates a new IFRender for PDF]. As the file name is derived from the page > sequence object, I noticed, that startPageSequence is called with incorrect > page sequences [eg. some are started twice]. OK, I see. Cool job, btw. Very interesting case. So, I took your word for it, and committed the change in r1063022 (http://svn.apache.org/viewvc?rev=1063022&view=rev) Thanks for reporting, and the patch! -- Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug.
testing bug fixes - not!
Is there a reason why *all* bug fixes are not accompanied by one or more new test cases that demonstrate the presence and absence of the bug before and after the fix? Adding such test cases should be mandatory for all bug fix commits. I wouldn't quite go that far for performance and clean up fixes, however, but for legitimate bugs, there should be a reliable way to ensure that (1) the bug and its fix are testable and (2) that future regressions do not occur. I realize it takes more effort, but it is worth it in the long term, both in actual improvements in quality and the ability to demonstrate consistently good practice to maintain that quality. Many Apache projects require this process be followed; I would urge the FOP project to adopt a similar criterion for bug fix commits. G.
DO NOT REPLY [Bug 50635] Incorrect pagesequence passed to renderer
https://issues.apache.org/bugzilla/show_bug.cgi?id=50635 e9925248 changed: What|Removed |Added Status|NEEDINFO|NEW --- Comment #3 from e9925248 2011-01-24 17:06:44 EST --- I'm not sure, if it affects any in-tree renderer. It can only affect renders not supporting out of order processing [if (!renderer.supportsOutOfOrder() && ...)] To simplify my work, I wrote a new renderer, which creates a ZIP file containing each page sequence as its own PDF file [each startPageSequence call creates a new IFRender for PDF]. As the file name is derived from the page sequence object, I noticed, that startPageSequence is called with incorrect page sequences [eg. some are started twice]. -- Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug.
DO NOT REPLY [Bug 50636] O(n^2) code for adding new pages
https://issues.apache.org/bugzilla/show_bug.cgi?id=50636 Andreas L. Delmelle changed: What|Removed |Added Status|NEW |RESOLVED Resolution||FIXED --- Comment #3 from Andreas L. Delmelle 2011-01-24 13:39:33 EST --- Patch applied in r1062913, with one minor change: currentPageSequenceIndex became obsolete, so removed it. See: http://svn.apache.org/viewvc?rev=1062913&view=rev Thanks for reporting, and again, for the patch! -- Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug.
DO NOT REPLY [Bug 50593] [PATCH] Add type safety to collections in fop.area package (e.a.)
https://issues.apache.org/bugzilla/show_bug.cgi?id=50593 Andreas L. Delmelle changed: What|Removed |Added Status|NEW |RESOLVED Resolution||FIXED --- Comment #1 from Andreas L. Delmelle 2011-01-24 13:14:19 EST --- No objections, I assumed, so I went ahead and committed in r1062901 ( http://svn.apache.org/viewvc?rev=1062901&view=rev ). -- Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug.
Re: svn commit: r1062901 [1/2] - in /xmlgraphics/fop/trunk/src/java/org/apache/fop/area: ./ inline/
On 24 Jan 2011, at 19:06, adelme...@apache.org wrote: > Author: adelmelle > Date: Mon Jan 24 18:06:25 2011 > New Revision: 1062901 > > URL: http://svn.apache.org/viewvc?rev=1062901&view=rev > Log: > Bugzilla 50593: Mostly add type safety to various collections in the fop.area > package. Additionally, added @Override annotations and used static import for > Constants. Hope nobody minds about the latter. I still have some history to catch up on, but saw @Overrides popping up here and there, so assumed this was already agreed upon. Not sure if I got them all, though. Concerning the static imports, it's more a matter of preference, I guess... I just like the look of immediately seeing in the import header which literals are actually used throughout a class. If anyone minds, just shout and I will revert. Regards, Andreas ---
DO NOT REPLY [Bug 50636] O(n^2) code for adding new pages
https://issues.apache.org/bugzilla/show_bug.cgi?id=50636 --- Comment #2 from Andreas L. Delmelle 2011-01-24 12:05:14 EST --- Thanks for the patch! I will take this on and commit it soon, unless someone beats me to it. (Got some changes to the same package/class that I want to get through first...) -- Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug.
DO NOT REPLY [Bug 50635] Incorrect pagesequence passed to renderer
https://issues.apache.org/bugzilla/show_bug.cgi?id=50635 Andreas L. Delmelle changed: What|Removed |Added Status|NEW |NEEDINFO --- Comment #2 from Andreas L. Delmelle 2011-01-24 12:03:31 EST --- I am wondering... Would it be possible to construct a small testcase to show that this leads to wrong output? Or is this not the issue, and is it more of an optimization? Just checking to see if this is a regression that can be tested for, so we can make sure it does not reoccur. Thanks! -- Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug.
