Merge pull request #137 from sanger-pathogens/sorting_conf_files

Sorted conf files by key

Dockerfile

@@ -16,12 +16,48 @@ ENV   LANG     en_GB.UTF-8
 ENV   LANGUAGE en_GB:en
 ENV   LC_ALL   en_GB.UTF-8
 
+
+# This RUN will install known Perl dependencies ahead of the automatic 
+# installation by with `dzil authordeps` and `dzil listdeps` (below). Adding this 
+# RUN  will cache these in a Docker layer makes the build much faster you have 
+# only made a change in the Bio-VertRes-Config repo (just a one-line change 
+# triggers the COPY command below and causes all the Perl dependencies to be 
+# reinstalled).
+# *But* it will make the image larger, and could in future install packages that
+# are no longer required.
+# => NOT recommended for production builds
+# 
+# use --build-arg "FAST_REBUILDS=true" to enable this
+# 
+ARG   FAST_REBUILDS
+RUN   if [ "$FAST_REBUILDS" = "true" ]; then \
+         echo 'Installing known dependencies in separate layer to speed up docker repeated builds (not recommended for production)'; \
+         cpanm Dist::Zilla::Plugin::PodWeaver \
+               Dist::Zilla::PluginBundle::Git \
+               Pod::Elemental::Transformer::List \
+               Array::Utils \
+               DBI \
+               File::Slurper \
+               Test::Files \
+               Pod::Elemental::Transformer::List \
+               DBD::mysql; \
+      fi
+
+# Dist::Zilla::PluginBundle::Starter tests fail unless Dist::Zilla has been
+# installed first.  Doing this allows the --force option than was previously
+# used with cpanm (below) to be removed.
+RUN   cpanm Dist::Zilla && \
+      cpanm Dist::Zilla::PluginBundle::Starter
+
 # copy and install (with perl deps)
-COPY . /opt/Bio-VertRes-Config
-WORKDIR /opt/Bio-VertRes-Config
-RUN	dzil authordeps --missing | cpanm --force && \
-    cpanm Pod::Elemental::Transformer::List && \
-	cpanm DBD::mysql && \
-	dzil listdeps --missing | cpanm --force && \
-	dzil test && \
-	dzil install
+# note tests include looking for user 'pathpipe'
+COPY     . /opt/Bio-VertRes-Config
+WORKDIR  /opt/Bio-VertRes-Config
+RUN      dzil authordeps --missing | cpanm && \
+         cpanm Pod::Elemental::Transformer::List && \
+         cpanm DBD::mysql && \
+         dzil listdeps --missing | cpanm && \
+         adduser --system pathpipe && \
+         dzil test && \
+         deluser pathpipe && \
+         dzil install

bin/sort_conf

@@ -0,0 +1,28 @@
+#!/usr/bin/env perl
+
+# ABSTRACT: Sorts conf files by key
+# PODNAME: sort_conf
+
+=head1 SYNOPSIS
+
+Sorts conf files by key.  Reads conf from STDIN, or conf file name can be
+provided on the command line.   Writes sorted conf  to STDOUT.
+
+   sort_conf unsorted.conf
+   cat unsorted.conf | sort_conf > sorted.conf
+
+=cut
+
+
+use strict;
+use warnings;
+use Data::Dumper;
+BEGIN { unshift( @INC, '../lib' ) }
+BEGIN { unshift( @INC, './lib' ) }
+BEGIN { unshift( @INC, '/software/pathogen/internal/prod/lib/' ) }
+
+use Bio::VertRes::Config::Utils qw(sort_hash_keys);
+
+$Data::Dumper::Sortkeys = sub { [ sort {sort_hash_keys($a,$b)} keys %{$_[0]} ] };
+
+print Dumper eval join('', <>);

lib/Bio/VertRes/Config/Pipelines/Common.pm

@@ -19,6 +19,7 @@ use Data::Dumper;
 use File::Basename;
 use File::Path qw(make_path);
 
+use Bio::VertRes::Config::Utils qw(sort_hash_keys);
 use Bio::VertRes::Config::DatabaseManager;
 with 'Bio::VertRes::Config::Pipelines::Roles::RootDatabaseLookup';
 with 'Bio::VertRes::Config::Pipelines::Roles::UnixGroup';
@@ -164,6 +165,8 @@ sub create_config_file {
 
     # dont print out an extra wrapper variable
     $Data::Dumper::Terse = 1;
+    # sort keys in the Dumper output
+    local $Data::Dumper::Sortkeys = sub { [ sort {sort_hash_keys($a,$b)} keys %{$_[0]} ] };
 
     write_text( $self->config, Dumper( $self->to_hash ) );
     chmod $mode, $self->config;

lib/Bio/VertRes/Config/Utils.pm

@@ -0,0 +1,27 @@
+package Bio::VertRes::Config::Utils;
+
+# ABSTRACT: Minor functions available throughout Bio::VertRes::Config
+
+=head1 SYNOPSIS
+
+Just somewhere to keep minor functions (which don't warrant instantiating an object)
+that we want to be available throughout Bio::VertRes::Config
+
+=cut
+
+require Exporter;
+@ISA = qw(Exporter);
+@EXPORT_OK = qw(sort_hash_keys);
+
+=head2 sort_hash_keys
+
+Use this with C<sort> for a standardized sorting of hash keys.
+
+=cut
+
+sub sort_hash_keys {
+   my($a,$b) = @_;
+   return($a cmp $b);
+}
+
+1;

t/bin/log_parameters.t

@@ -4,6 +4,8 @@ use warnings;
 use Data::Dumper;
 use File::Temp;
 
+# list of scripts that don't write log files; tests are skipped for these
+use constant NON_LOGGING_SCRIPTS => [qw(sort_conf)];
 
 BEGIN { unshift( @INC, './lib' ) }
 
@@ -16,15 +18,19 @@ my @available_scripts = grep { /^[^\.]/ } readdir($dh);
 closedir $dh;
 
 for my $script_name (  @available_scripts ) {
-    my $destination_directory_obj = File::Temp->newdir( CLEANUP => 1 );
-    my $destination_directory = $destination_directory_obj->dirname();
-
-    system("./bin/$script_name --config_base $destination_directory --log_base $destination_directory >/dev/null 2>&1");
-    ok( -e $destination_directory . '/command_line.log', "log file has been created for $script_name" );
-    open(my $fh, $destination_directory . '/command_line.log');
-    my $line = <$fh>;
-    ok(($line =~ /^[\w]+[\s]+[\w]+[\s]+[\d]+[\s]+[\d]+:[\d]+:[\d]+[\s]+[\d]+[\s]+[\w\d]+[\s]+\.\/bin\/$script_name --config_base $destination_directory --log_base $destination_directory$/), 'correct format of log file' );
-
+   SKIP: {
+      skip "script \"$script_name\" isn't supposed to write log files", 2
+         if grep {$_ eq $script_name} @{+NON_LOGGING_SCRIPTS};
+
+      my $destination_directory_obj = File::Temp->newdir( CLEANUP => 1 );
+      my $destination_directory = $destination_directory_obj->dirname();
+
+      system("./bin/$script_name --config_base $destination_directory --log_base $destination_directory >/dev/null 2>&1");
+      ok( -e $destination_directory . '/command_line.log', "log file has been created for $script_name" );
+      open(my $fh, $destination_directory . '/command_line.log');
+      my $line = <$fh>;
+      ok(($line =~ /^[\w]+[\s]+[\w]+[\s]+[\d]+[\s]+[\d]+:[\d]+:[\d]+[\s]+[\d]+[\s]+[\w\d]+[\s]+\.\/bin\/$script_name --config_base $destination_directory --log_base $destination_directory$/), 'correct format of log file' );
+   }
 }
 
 done_testing();