Test.pm & t/force_todo refactoring idea
Stevan Little
occurred to me.
- Should we remove all the todo_* functions and just use the
t/force_todo file?
This would cut the number of functions in Test.pm almost in half, which
would decrease loading/parsing time and memory-consumption.
However, since t/force_todo would be larger, and it gets read each time
Test.pm is loaded this might not save us any time at all.
Which then lead me to my next thought.
- Should we do away with the t/force_todo file and introduce a
force_todo() function?
The idea is to move from the more centralized t/force_todo file to a
more decentralized force_todo() function which would accept a list of
test numbers which would be forced-TODO on a per-file basis.
One advantage of this would be that there is no performance penalty if
the test file is not forcing any TODOs. Currently if you are not
forcing TODO in the test file, the whole t/force_todo file must be
read.
It would also allow for code like this:
force_todo(1, 5, 10 .. 15, 25);
Which (IMHO) is nicer than having to do this in the t/force_todo file:
t/my_test.t 1 5 10 11 12 13 14 15 25
I also think that having the forced-TODOs listed in the test file
itself will make it easier to remember to un-force-TODO.
However this is a very large change, and so I wanted to run it by the
group before I even started.
Please let me know your thoughts.
- Stevan
Nathan Gray
> It would also allow for code like this:
>
> force_todo(1, 5, 10 .. 15, 25);
>
> Which (IMHO) is nicer than having to do this in the t/force_todo file:
>
> t/my_test.t 1 5 10 11 12 13 14 15 25
>
> I also think that having the forced-TODOs listed in the test file
> itself will make it easier to remember to un-force-TODO.
>
> However this is a very large change, and so I wanted to run it by the
> group before I even started.
>
> Please let me know your thoughts.
I really like that idea. It puts the TODO list closer to the tests,
similar to the benefit of having documentation close to the code it
describes.
Go Stevan!
-kolibrie
Ingo Blechschmidt
Stevan Little <stevan <at> iinteractive.com> writes:
> - Should we remove all the todo_* functions and just use the
> t/force_todo file?
TODOed because of release preparation, and the todo_* functions
mark tests which are real TODO tests.
If we take this way, then we should certainly rename t/force_todo
to t/todo_tests (or sth. similar).
> - Should we do away with the t/force_todo file and introduce a
> force_todo() function?
>
> The idea is to move from the more centralized t/force_todo file to a
> more decentralized force_todo() function which would accept a list of
> test numbers which would be forced-TODO on a per-file basis.
so all tests which were only force_todoed were instantly unTODOed.
But if we remove all todo_* functions and go the todo_tests way, then
we really should use a sub (todo_tests(...), for example), as you
propose.
> force_todo(1, 5, 10 .. 15, 25);
Of course, we could make Test.pm parse sth. like the following, too.
t/my_test.t 1 5 10..15 25
--Ingo
--
Linux, the choice of a GNU | Life would be so much easier if we could
generation on a dual AMD | just look at the source code.
Athlon! | -- Dave Olson
Stevan Little
On Apr 21, 2005, at 10:03 AM, Ingo Blechschmidt wrote:
>> - Should we do away with the t/force_todo file and introduce a
>> force_todo() function?
>>
>> The idea is to move from the more centralized t/force_todo file to a
>> more decentralized force_todo() function which would accept a list of
>> test numbers which would be forced-TODO on a per-file basis.
>
> The idea was that t/force_todo can be deleted/cleared after a release,
> so all tests which were only force_todoed were instantly unTODOed.
Unfortunately that is not how it is being used though. It sticks
around, and in some cases maybe even being used as a informal TODO
list.
>> force_todo(1, 5, 10 .. 15, 25);
>
> That'd be cool :)
>
> Of course, we could make Test.pm parse sth. like the following, too.
> t/my_test.t 1 5 10..15 25
We could but again, we would be imposing more of a penalty on test
files which dont need it.
Although after some discussion with nothingmuch on IRC, I think I have
come to good compromise.
We remove all the todo_ functions, and we add a named param "todo" to
all our test functions. This would allow test level TODO-ing.
Then we add the force_todo() function, to allow for file level TODO-ing.
And we introduce some kind of global variable/env variable to do test
suite wide TODO-ing.
This should give us enough granularity of control as well as make it
simple to just "flip a switch" for release.
Any thoughts?
- Stevan
Ingo Blechschmidt
Stevan Little wrote:
>> The idea was that t/force_todo can be deleted/cleared after a
>> release, so all tests which were only force_todoed were instantly
>> unTODOed.
>
> Unfortunately that is not how it is being used though. It sticks
> around, and in some cases maybe even being used as a informal TODO
> list.
true.
> Although after some discussion with nothingmuch on IRC, I think I have
> come to good compromise.
>
> We remove all the todo_ functions, and we add a named param "todo" to
> all our test functions. This would allow test level TODO-ing.
>
> Then we add the force_todo() function, to allow for file level
> TODO-ing.
>
> And we introduce some kind of global variable/env variable to do test
> suite wide TODO-ing.
>
> This should give us enough granularity of control as well as make it
> simple to just "flip a switch" for release.
That sounds great! :)
--Ingo
--
Linux, the choice of a GNU | The next statement is not true.
generation on a dual AMD | The previous statement is true.
Athlon! |
Nathan Gray
> Although after some discussion with nothingmuch on IRC, I think I have
> come to good compromise.
>
> We remove all the todo_ functions, and we add a named param "todo" to
> all our test functions. This would allow test level TODO-ing.
>
> Then we add the force_todo() function, to allow for file level TODO-ing.
>
> And we introduce some kind of global variable/env variable to do test
> suite wide TODO-ing.
I don't quite understand the global/env variable idea. Can you explain
that in more detail?
> This should give us enough granularity of control as well as make it
> simple to just "flip a switch" for release.
>
> Any thoughts?
I really like the solution you've presented: clean and functional.
-kolibrie
Stevan Little
On Apr 21, 2005, at 12:00 PM, Nathan Gray wrote:
> On Thu, Apr 21, 2005 at 11:17:11AM -0400, Stevan Little wrote:
>> Although after some discussion with nothingmuch on IRC, I think I have
>> come to good compromise.
>>
>> We remove all the todo_ functions, and we add a named param "todo" to
>> all our test functions. This would allow test level TODO-ing.
>>
>> Then we add the force_todo() function, to allow for file level
>> TODO-ing.
>>
>> And we introduce some kind of global variable/env variable to do test
>> suite wide TODO-ing.
>
> I don't quite understand the global/env variable idea. Can you explain
> that in more detail?
It could be something as simple as this:
%*ENV<TODO_ALL_FOR_RELEASE> = 1;
It is really just a hack to allow for quick TODO-ing of all failing
tests in the suite. I am not sure how well it will work out to be
honest.
>> This should give us enough granularity of control as well as make it
>> simple to just "flip a switch" for release.
>>
>> Any thoughts?
>
> I really like the solution you've presented: clean and functional.
Thanks,
I have to give credit to nothingmuch, the "todo" named param was his
idea :)
>
> -kolibrie
>
Nathan Gray
> On Apr 21, 2005, at 12:00 PM, Nathan Gray wrote:
> >I don't quite understand the global/env variable idea. Can you explain
> >that in more detail?
>
> It could be something as simple as this:
>
> %*ENV<TODO_ALL_FOR_RELEASE> = 1;
>
> It is really just a hack to allow for quick TODO-ing of all failing
> tests in the suite. I am not sure how well it will work out to be
> honest.
So if TODO_ALL_FOR_RELEASE is true, we just have 'proclaim' treat all
failures as TODO. Sounds effective enough.
-kolibrie
Andy Dougherty
I'm probably misunderstanding, but this sounds *too* effective.
Wouldn't it also mask unexpected failures? That
is, it's reasonable to have a set of "known and expected failures", but
you do want to be sure to learn about unexpected failures.
I don't see any way to avoid specifying exactly which tests are expected
to fail.
--
Andy Dougherty doug...@lafayette.edu
Stevan Little
On Apr 22, 2005, at 11:20 AM, Andy Dougherty wrote:
> On Fri, 22 Apr 2005, Nathan Gray wrote:
>> On Thu, Apr 21, 2005 at 02:24:29PM -0400, Stevan Little wrote:
>>> On Apr 21, 2005, at 12:00 PM, Nathan Gray wrote:
>>>> I don't quite understand the global/env variable idea. Can you
>>>> explain
>>>> that in more detail?
>>>
>>> It could be something as simple as this:
>>>
>>> %*ENV<TODO_ALL_FOR_RELEASE> = 1;
>>>
>>> It is really just a hack to allow for quick TODO-ing of all failing
>>> tests in the suite. I am not sure how well it will work out to be
>>> honest.
>>
>> So if TODO_ALL_FOR_RELEASE is true, we just have 'proclaim' treat all
>> failures as TODO. Sounds effective enough.
>
> I'm probably misunderstanding, but this sounds *too* effective.
> Wouldn't it also mask unexpected failures?
Yes it would, but it would be a tool only used for release (if at all).
And Pugs common practice it to mask all failures in the release.
Stevan Little
Essentially yes.
Stevan
Andy Dougherty
>
> On Apr 22, 2005, at 11:20 AM, Andy Dougherty wrote:
> > > So if TODO_ALL_FOR_RELEASE is true, we just have 'proclaim' treat all
> > > failures as TODO. Sounds effective enough.
> >
> > I'm probably misunderstanding, but this sounds *too* effective.
> > Wouldn't it also mask unexpected failures?
>
> Yes it would, but it would be a tool only used for release (if at all). And
> Pugs common practice it to mask all failures in the release.
Not exactly (again, assuming I understand this all correctly). It's been
common practice to mask all *expected* failures (though the extent to
which one ought to appeal to "historical" practice in Pugs is a bit
unclear to me ...).
My question is about *unexpected* failures. Do you really want to mask
them too? If so, why not simply disable the 'make test' target in the
release?
Here's a scenario I have in mind: If I ever successfully bootstrap
ghc-6.4, then I'll try to run Pugs. In my experience, trying things on
new platforms often yields surprises. Things like processor wordsize and
endianness, alignment constraints, different underlying C libraries, etc.,
are the sorts of things worth testing for. Masking any such failures would
seem counterproductive to the whole point of trying to get Pugs running on
a new architecture in the first place.
--
Andy Dougherty doug...@lafayette.edu
Stevan Little
I think you are misunderstanding me.
On Apr 22, 2005, at 2:57 PM, Andy Dougherty wrote:
>> Yes it would, but it would be a tool only used for release (if at
>> all). And
>> Pugs common practice it to mask all failures in the release.
>
> Not exactly (again, assuming I understand this all correctly). It's
> been
> common practice to mask all *expected* failures (though the extent to
> which one ought to appeal to "historical" practice in Pugs is a bit
> unclear to me ...).
For each CPAN release, we have masked *all* failures. This is to avoid
needless bug reports for errors we know exist. Originally this was done
by hand (yuk), then by script (not as bad, but still tedious and ugly),
then we started using the t/force_todo file, etc etc etc.
Then once the tarball was made and put onto CPAN, we would usually then
un-TODO-ed all the tests again, so that we had our expected failures.
I agree that globally masking failures is a bad thing, and to be honest
I think that the per-file force_todo() will be the best way to go.
Stevan