A fundamental principle of medicine is "do no harm." It has a long history and you can find it in the Hippocratic oath with a slightly different wording.
This is also an important principle of software development. If you add a new feature or fix a bug, make sure the resulting code isn't worse off than before. Do no harm is the basic motivation behind regression testing.
I have been thinking about Brion's suggestion of fixing the bug in WebRequest::extractTitle(). It is a reasonable point. Don't just whine about a problem. Fix it. He even provided the best strategy for accomplishing this. Make "sure $wgScriptPath gets properly escaped when initialized." I am sure doing this would not require a significant amount of coding. But, how would changing the way $wgScriptPath is formatted affect the rest of the code base?
I decided to do a multi-file search for $wgScriptPath in phase3 and extensions [r53650]. There are 439 references to it in phase3 and extensions combined. In phase3 alone, there are 47 references. Roughly 1/3 of these are in global declarations, so phase3 has about 30 "active" references and in phase3 and extensions combined there are roughly 300. [By "active" I mean references in which the value of $wgScriptPath affects the code's logic.]
So, if I were to change the formatting of $wgScriptPath, there potentially are 30 places in the main code and 300 places in extensions where problems might occur. To ensure the change does no harm, I would have to observe the effect of the change on at least 30 and up to 330 places in the distribution. This is pretty onerous requirement. My guess is very few developers would take the time to do it.
On the other hand, if there were regression tests for the main code and for the most important extensions, I could make the change, run the regression tests and see if any break. If some do, I could focus my attention on those problems. I would not have to find every place the global is referenced and see if the change adversely affects the logic.
On Thu, Jul 23, 2009 at 11:07 AM, dan nessettdnessett@yahoo.com wrote: [snip]
On the other hand, if there were regression tests for the main code and for the most important extensions, I could make the change, run the regression tests and see if any break. If some do, I could focus my attention on those problems. I would not have to find every place the global is referenced and see if the change adversely affects the logic.
This only holds if the regression test would fail as a result of the change. This is far from a given for many changes and many common tests.
Not to mention the practical complications— many extensions have complicated configuration and/or external dependencies. "make test_all_extensions" is not especially realistic.
Automated tests are good, necessary even, but they don't relieve you of the burden of directly evaluating the impact of a broad change.
True. Regressions tests do not guarantee bug are not introduced by changes. However, they are a fundamental piece of the QA puzzle.
--- On Thu, 7/23/09, Gregory Maxwell gmaxwell@gmail.com wrote:
From: Gregory Maxwell gmaxwell@gmail.com Subject: Re: [Wikitech-l] Do no harm To: "Wikimedia developers" wikitech-l@lists.wikimedia.org Date: Thursday, July 23, 2009, 9:50 AM On Thu, Jul 23, 2009 at 11:07 AM, dan nessettdnessett@yahoo.com wrote: [snip]
On the other hand, if there were regression tests for
the main code and for the most important extensions, I could make the change, run the regression tests and see if any break. If some do, I could focus my attention on those problems. I would not have to find every place the global is referenced and see if the change adversely affects the logic.
This only holds if the regression test would fail as a result of the change. This is far from a given for many changes and many common tests.
Not to mention the practical complications— many extensions have complicated configuration and/or external dependencies. "make test_all_extensions" is not especially realistic.
Automated tests are good, necessary even, but they don't relieve you of the burden of directly evaluating the impact of a broad change.
Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
On Thu, Jul 23, 2009 at 11:07 AM, dan nessettdnessett@yahoo.com wrote:
On the other hand, if there were regression tests for the main code and for the most important extensions, I could make the change, run the regression tests and see if any break. If some do, I could focus my attention on those problems. I would not have to find every place the global is referenced and see if the change adversely affects the logic.
We are all aware of the benefits of regression tests.
The reason I started this conversation is I want to write an extension. I also want to be a good citizen and do this in a way that doesn't break things (this would also have the desirable effect of making it more likely that some MW installation would use the extension).
So, since, as you point out, everyone agrees that regression tests are beneficial and since, except for parserTests, there doesn't seem to be any substantive regression tests available, what are some practical steps that would improve the situation?
--- On Thu, 7/23/09, Aryeh Gregor Simetrical+wikilist@gmail.com wrote:
From: Aryeh Gregor Simetrical+wikilist@gmail.com Subject: Re: [Wikitech-l] Do no harm To: "Wikimedia developers" wikitech-l@lists.wikimedia.org Date: Thursday, July 23, 2009, 9:51 AM On Thu, Jul 23, 2009 at 11:07 AM, dan nessettdnessett@yahoo.com wrote:
On the other hand, if there were regression tests for
the main code and for the most important extensions, I could make the change, run the regression tests and see if any break. If some do, I could focus my attention on those problems. I would not have to find every place the global is referenced and see if the change adversely affects the logic.
We are all aware of the benefits of regression tests.
Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Here's what I do in similar circumstances. Create another variable, $wgScriptPathEscaped. Then, gradually make the changes. Wait for tests. Change some more. Eventually, most of the old ones will be gone.
By inspection, many of the uses will be terminal, not passed to other routines, with no side effects. Those should be done first.
Sure, it might take a month or three. But wishing for some universal regression suite is going to be about the same as waiting for the single pass parser.
Sounds like a plan. Be my guest.
--- On Thu, 7/23/09, William Allen Simpson william.allen.simpson@gmail.com wrote:
From: William Allen Simpson william.allen.simpson@gmail.com Subject: Re: [Wikitech-l] Do no harm To: "Wikimedia developers" wikitech-l@lists.wikimedia.org Date: Thursday, July 23, 2009, 10:21 AM Here's what I do in similar circumstances. Create another variable, $wgScriptPathEscaped. Then, gradually make the changes. Wait for tests. Change some more. Eventually, most of the old ones will be gone.
By inspection, many of the uses will be terminal, not passed to other routines, with no side effects. Those should be done first.
Sure, it might take a month or three. But wishing for some universal regression suite is going to be about the same as waiting for the single pass parser.
Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
On 07/23/2009 10:21 AM, William Allen Simpson wrote:
Here's what I do in similar circumstances. Create another variable, $wgScriptPathEscaped. Then, gradually make the changes. Wait for tests. Change some more. Eventually, most of the old ones will be gone.
$wgScriptPath is a URL fragment, and by definition MUST be properly URL-escaped.
There is no need for a second variable; it just needs to be initialized correctly -- something that never got done because it's extremely rare to stick a web app in a folder that has characters that would need escaping.
-- brion
Brion Vibber brion@wikimedia.org wrote:
Here's what I do in similar circumstances. Create another variable, $wgScriptPathEscaped. Then, gradually make the changes. Wait for tests. Change some more. Eventually, most of the old ones will be gone.
$wgScriptPath is a URL fragment, and by definition MUST be properly URL-escaped.
There is no need for a second variable; it just needs to be initialized correctly -- something that never got done because it's extremely rare to stick a web app in a folder that has characters that would need escaping.
It just needs to be *documented* correctly :-).
Tim
wikitech-l@lists.wikimedia.org