Code broken by regex fixes in 5.10.0, or, why it's good to help test release candidates
My baby, ack, broke under Perl 5.10.0, because of a fix in regex behavior that I had been using unknowingly. See, I had always used my regex objects like this:
my $re = qr/^blah blah/; if ( $string =~ /$re/sm )...
when I should have been using it like this:
my $re = qr/^blah blah/sm; if ( $string =~ /$re/ )...
The bug in 5.8.x is that the /$re/sm would incorrectly apply the /sm modifiers to $re. This made the code happen to work, but for the wrong reason. What was especially tricky about finding my bug was that in 5.10.0, the call to /$re/sm ignores the /sm, but doesn't tell you that.
After some back and forth on p5p, a patch was submitted that gave the warning about the ignored /sm flags, but alas, Perl 5.10 was already out. It wouldn't have been so bad if it hadn't been the day AFTER it was released.
So, lesson learned: Test your code against new release candidates of Perl, both for your code's sake, AND for Perl's sake.
And y'know, now that I think of it, this is probably a great policy for Perl::Critic just waiting to happen. I wonder how many other people are doing their regexes the wrong way, too.
If perl warns about this construction, what's the advantage of making a Perl::Critic policy for it as well? So that it can be discovered even by people not running 5.10.0?
Er, 5.10.1 and later, I should have said. Sorry.
Perl::Critic already has such a policy. It's called RegularExpressions::RequireLineBoundaryMatching and has been present in P::C since 0.13. However, an oversight caused it to apply only to m// and s/// and not qr// until this past September when I fixed it during the course of my TPF grant work.
There are two problems.
* not applying the /m to the qr//
* applying the /m to the m//.
Your policy only handles the first case, which I'm glad it does, but is only half the battle. I just had someone else tell me that he had been doing regexes my way as well, so I'm not alone in this goof.
You're certainly not alone. I _did_ test Astro::SpaceTrack under 5.10.0, and _still_ had to patch.
In this case the test suite was inadequate. Still is, since I'm reluctant to look too hard at downloaded data (see Net::Dict or Astro::SIMBAD for why not), and I can't just download and bundle a file to munch on without getting permission from NORAD.
I'm not sure Test::Coverage would have helped, since I don't know how it knows the results of execution were actually looked at.
Bottom line: adequate testing is as difficult as it is essential.