Commit a0dd44a7 authored by Andy Yates's avatar Andy Yates
Browse files

[ENSCORESW-478] and [ENSCORESW-467]. Brought in a Perl::Critic test being run...

[ENSCORESW-478] and [ENSCORESW-467]. Brought in a Perl::Critic test being run on the lowest level. Tests are only run when we have the ENV TEST_AUTHOR is set to a true value. Otherwise it does not impact on the current test cases. Also had to fix a number of issues associcated with these flags. Mostly were due to not using warnings or strict. Anything I felt was ok or not my call has been annotated with a no critic flag.
parent a13dba36
......@@ -157,13 +157,13 @@ sub parse_options {
$conffile = abs_path($conffile);
if (-e $conffile) {
open(CONF, $conffile) or throw(
open(my $fh, '<', $conffile) or throw(
"Unable to open configuration file $conffile for reading: $!");
my $serverroot = $self->serverroot;
my $last;
while (my $line = <CONF>) {
while (my $line = <$fh>) {
chomp $line;
# remove leading and trailing whitespace
......@@ -200,6 +200,7 @@ sub parse_options {
}
$self->param($name, $val);
}
close($fh);
$self->param('conffile', $conffile);
}
......@@ -556,14 +557,14 @@ sub list_or_file {
# we didn't get a list of values, but a file to read values from
@vals = ();
open(IN, $firstval) or throw("Cannot open $firstval for reading: $!");
open(my $fh, '<', $firstval) or throw("Cannot open $firstval for reading: $!");
while(<IN>){
while(<$fh>){
chomp;
push(@vals, $_);
}
close(IN);
close($fh);
$self->param($param, @vals);
}
......
......@@ -135,10 +135,10 @@ sub parse_common_options {
my $conffile = $h{'conffile'} || $self->serverroot . "/sanger-plugins/vega/conf/ini-files/Conversion.ini";
$conffile = abs_path($conffile);
if (-e $conffile) {
open(CONF, $conffile) or throw(
open(my $fh, '<', $conffile) or throw(
"Unable to open configuration file $conffile for reading: $!");
my $serverroot = $self->serverroot;
while (<CONF>) {
while (<$fh>) {
chomp;
# remove comments
......@@ -155,6 +155,7 @@ sub parse_common_options {
}
$self->param($name, $val);
}
close $fh;
$self->param('conffile', $conffile);
}
elsif ($conffile) {
......@@ -616,12 +617,12 @@ sub list_or_file {
if (scalar(@vals) == 1 && -e $firstval) {
# we didn't get a list of values, but a file to read values from
@vals = ();
open(IN, $firstval) or throw("Cannot open $firstval for reading: $!");
while(<IN>){
open(my $fh, '<', $firstval) or throw("Cannot open $firstval for reading: $!");
while(<$fh>){
chomp;
push(@vals, $_);
}
close(IN);
close($fh);
$self->param($param, @vals);
}
$self->comma_to_list($param);
......@@ -881,11 +882,11 @@ sub dynamic_use {
my ($self, $classname) = @_;
my ($parent_namespace, $module) = $classname =~/^(.*::)(.*)$/ ? ($1,$2) : ('::', $classname);
no strict 'refs';
no strict 'refs'; ## no critic
# return if module has already been imported
return 1 if $parent_namespace->{$module.'::'} && %{ $parent_namespace->{$module.'::'}||{} };
eval "require $classname";
eval "require $classname"; ## no critic
throw("Failed to require $classname: $@") if ($@);
$classname->import();
......@@ -1278,7 +1279,7 @@ sub log {
sub lock_log {
my ($self) = @_;
## no critic
my $fh = $self->{'_log_filehandle'};
return if -t $fh or -p $fh; # Shouldn't lock such things
flock($self->{'_log_filehandle'},LOCK_EX) || return 0;
......@@ -1294,7 +1295,7 @@ sub lock_log {
sub unlock_log {
my ($self) = @_;
## no critic
my $fh = $self->{'_log_filehandle'};
return if -t $fh or -p $fh; # We don't lock such things
# flush is implicit in flock
......
......@@ -171,7 +171,7 @@ sub _convert_single{
foreach my $field (qw(in out)){
my $slot=__PACKAGE__ ."::$field";
no strict 'refs';
no strict 'refs'; ## no critic
*$field=sub{
my $self=shift;
$self->{$slot}=shift if @_;
......
......@@ -83,7 +83,7 @@ sub _convert_single_repeatFeature {
);
my $output_module = $self->out;
require "$output_module";
eval "require $output_module"; ## no critic
return new Bio::SeqFeature::FeaturePair(
-feature1 => $feature1,
-feature2 => $feature2
......
......@@ -519,6 +519,7 @@ sub deprecate {
=cut
## no critic
sub try (&$) {
my ($try, $catch) = @_;
eval { &$try };
......@@ -529,6 +530,7 @@ sub try (&$) {
}
}
## no critic
sub catch (&) {
shift;
}
......
......@@ -58,6 +58,9 @@ most classes.
package Bio::EnsEMBL::Utils::Proxy;
use strict;
use warnings;
use Bio::EnsEMBL::Utils::Exception qw/throw/;
use vars '$AUTOLOAD';
......@@ -179,7 +182,10 @@ sub AUTOLOAD {
my $type = ref $self ? 'object' : 'class';
throw qq{Can't locate $type method "$method_name" via package "$package_name". No subroutine was generated};
}
*$AUTOLOAD = $sub;
{
no strict 'refs'; ## no critic ProhibitNoStrict
*{$AUTOLOAD} = $sub;
}
goto &$sub;
}
......
......@@ -48,10 +48,10 @@ Bio::EnsEMBL::Utils::ConversionSupport
package Bio::EnsEMBL::Utils::SchemaConversion;
use Bio::EnsEMBL::Utils::ConversionSupport;
use strict;
use warnings;
use Bio::EnsEMBL::Utils::ConversionSupport;
use Data::Dumper;
=head2 new
......@@ -184,7 +184,7 @@ sub choose_conversion_type {
$species = $self->species_alias($self->conv_support->param('source_db'));
if ($self->conv_support->param('do_vega_sc')) {
$species = "vega::".$species;
eval "require SeqStoreConverter::$species";
eval "require SeqStoreConverter::$species"; ## no critic
if($@) {
warn("Could not require conversion module SeqStoreConverter::$species\ for vega conversion\n" .
"Using SeqStoreConverter::BasicConverter instead:\n$@");
......@@ -196,7 +196,7 @@ sub choose_conversion_type {
}
}
else {
eval "require SeqStoreConverter::$species";
eval "require SeqStoreConverter::$species"; ## no critic
if($@) {
warn("Could not require conversion module SeqStoreConverter::$species for Ensembl conversion\n" .
"Using SeqStoreConverter::BasicConverter instead:\n$@");
......
......@@ -243,12 +243,12 @@ sub inject {
my $classname = shift;
my ($parent_namespace, $module) = $classname =~/^(.*::)(.*)$/ ?
($1,$2) : ('::', $classname);
no strict 'refs';
no strict 'refs'; ## no critic
# return if module has already been imported
return 1 if $parent_namespace->{$module.'::'};
eval "require $classname";
eval "require $classname"; ## no critic
die("Failed to require $classname: $@") if ($@);
$classname->import();
......
## no critic (RequireFilenameMatchesPackage)
use strict;
use warnings;
our $verbose = 0;
......@@ -75,7 +78,7 @@ package ExternalFF2;
package Test;
package main;
use Bio::EnsEMBL::Test::TestUtils;
......
......@@ -18,7 +18,7 @@ my $gene_ids = [18256, 18257, 18258];
my $genes = $gene_adaptor->fetch_all_by_dbID_list($gene_ids);
sub BEGIN {
no strict 'refs';
no strict 'refs'; ## no critic
*Bio::EnsEMBL::DBSQL::GeneAdaptor::_build_id_cache = sub {
my ($self) = @_;
return Bio::EnsEMBL::DBSQL::Support::FullIdCache->new($self);
......
## no critic (RequireFilenameMatchesPackage)
package Test::SO::Term;
use strict;
use warnings;
sub new {
my ($class) = @_;
......
use strict;
use warnings;
use Cwd;
use File::Spec;
use File::Basename qw/dirname/;
use Test::More;
if ( not $ENV{TEST_AUTHOR} ) {
my $msg = 'Author test. Set $ENV{TEST_AUTHOR} to a true value to run.';
plan( skip_all => $msg );
}
eval {
require Test::Perl::Critic;
require Perl::Critic::Utils;
};
if($@) {
plan( skip_all => 'Test::Perl::Critic required.' );
note $@;
}
#chdir into the file's target & request cwd() which should be fully resolved now.
#then go back
my $file_dir = dirname(__FILE__);
my $original_dir = cwd();
chdir($file_dir);
my $cur_dir = cwd();
chdir($original_dir);
my $root = File::Spec->catdir($cur_dir, File::Spec->updir(),File::Spec->updir());
# Configure critic
Test::Perl::Critic->import(-profile => File::Spec->catfile($root, 'perlcriticrc'), -severity => 5, -verbose => 8);
#Find all files & run
my @perl_files = Perl::Critic::Utils::all_perl_files(
File::Spec->catdir($root, 'modules')
);
foreach my $perl (@perl_files) {
critic_ok($perl);
}
done_testing();
\ No newline at end of file
## no critic (RequireFilenameMatchesPackage)
package main;
use strict;
......@@ -18,7 +20,7 @@ my $gene_ids = [18256, 18257, 18258];
my $genes = $gene_adaptor->fetch_all_by_dbID_list($gene_ids);
sub BEGIN {
no strict 'refs';
no strict 'refs'; ##no critic
*Bio::EnsEMBL::DBSQL::GeneAdaptor::_build_id_cache = sub {
my ($self) = @_;
return Bio::EnsEMBL::DBSQL::Support::LruIdCache->new($self, 3);
......
## Bioperl Test Harness Script for Modules
##
# CVS Version
# $Id$
# Before `make install' is performed this script should be runnable with
# `make test'. After `make install' it should work as `perl test.t'
......@@ -20,6 +17,8 @@
#-----------------------------------------------------------------------
use Test::More;
use strict;
use warnings;
# This test script heavily edited by ihh@fruitfly.org
......@@ -37,7 +36,7 @@ ok( 1 );
use Bio::EnsEMBL::Mapper;
$mapper = Bio::EnsEMBL::Mapper->new( "rawcontig", "virtualcontig" );
my $mapper = Bio::EnsEMBL::Mapper->new( "rawcontig", "virtualcontig" );
load_sgp_dump( $mapper, undef );
# loading done successfully
......@@ -335,10 +334,10 @@ chr1 625359 1214016 1216330 1 2315 1
@sgp_dump = reverse (@sgp_dump) if defined $reverse; # test the auto-sorting feature
my $first = 1;
for $line ( @sgp_dump ) {
for my $local_line ( @sgp_dump ) {
if( $first ) { $first = 0; next; }
my ( $chr_name, $contig_id, $chr_start,
$chr_end, $contig_start, $contig_end, $contig_ori ) = split ( /\t/, $line );
$chr_end, $contig_start, $contig_end, $contig_ori ) = split ( /\t/, $local_line );
# new argument order:
$map->add_map_coordinates( $contig_id, $contig_start, $contig_end, $contig_ori,
......
use Test::More;
use strict;
use warnings;
use Bio::EnsEMBL::Test::MultiTestDB;
use Bio::EnsEMBL::Test::TestUtils;
......
use Test::More;
use strict;
use warnings;
use Bio::EnsEMBL::SeqEdit;
use Bio::EnsEMBL::Attribute;
......
......@@ -117,7 +117,7 @@ my $scalar;
my $other_scalar;
open my $scalar_fh, '>', \$scalar;
open my $other_scalar_fh, '>', \$other_scalar;
bless($other_scalar_fh);
bless($other_scalar_fh, __PACKAGE__);
my $io_handle = IO::Handle->new(); # no need to close as it isn't opened yet just created
throws_ok { assert_file_handle(undef) } qr/undefined/, 'Passing in undefined scalar means death';
dies_ok { assert_file_handle(bless(1, 'Brian'), 'met')} 'Passing in a blessed scalar means death';
......
severity = 5
[Subroutines::ProhibitExplicitReturnUndef]
severity=4
\ No newline at end of file
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment