From 4cc89e815be51840b0286f25ba91abdd9f702fec Mon Sep 17 00:00:00 2001 From: Paul Cochrane Date: Fri, 20 Oct 2017 14:54:47 +0200 Subject: [PATCH 1/3] Replace plain terminal input with new _slurp() method This change simply adds infrastructure to allow for the `<>` input to be more easily overridden in later tests by overriding the new `_slurp` method. It seems that one would have to jump through many hoops in order to make `<>` overridable so that one could test its behaviour and that of e.g. `get_env_var`, hence introducing a new method seemed the best option. The new method is used in `get_env_var` to replace direct usage of the diamond operator. This change also adds a simple test of the new method which checks and documents its behaviour. --- Makefile.PL | 1 + lib/Module/Release.pm | 24 +++++++++++++++++++++++- t/input.t | 32 ++++++++++++++++++++++++++++++++ 3 files changed, 56 insertions(+), 1 deletion(-) create mode 100644 t/input.t diff --git a/Makefile.PL b/Makefile.PL index 8982f83..23200d6 100644 --- a/Makefile.PL +++ b/Makefile.PL @@ -74,6 +74,7 @@ my %WriteMakefile = ( 'Test::More' => '1', 'Test::Output' => '0', 'Test::Without::Module' => '0', + 'File::Temp' => '0', }, 'META_MERGE' => { diff --git a/lib/Module/Release.pm b/lib/Module/Release.pm index 45e85d9..173f9c8 100644 --- a/lib/Module/Release.pm +++ b/lib/Module/Release.pm @@ -195,6 +195,7 @@ sub _set_defaults { debug => $ENV{RELEASE_DEBUG} || 0, local_file => undef, remote_file => undef, + input_fh => *STDIN{IO}, output_fh => *STDOUT{IO}, debug_fh => *STDERR{IO}, null_fh => IO::Null->new(), @@ -559,6 +560,16 @@ sub reset_perls { } +=item input_fh + +Return the value of input_fh. + +=cut + +sub input_fh { + return $_[0]->{input_fh}; +} + =item output_fh If quiet is off, return the value of output_fh. If output_fh is not @@ -1291,7 +1302,7 @@ sub get_env_var { return $pass if defined( $pass ) && length( $pass ); $self->_print( "$field is not set. Enter it now: " ); - $pass = <>; + $pass = $self->_slurp; chomp $pass; return $pass if defined( $pass ) && length( $pass ); @@ -1314,6 +1325,17 @@ output_fh to a null filehandle, output goes nowhere. sub _print { print { $_[0]->output_fh } @_[1..$#_] } +=item _slurp + +Read a line from whatever is in input_fh and return it. + +=cut + +sub _slurp { + my $fh = $_[0]->input_fh; + return <$fh>; +} + =item _dashes() Use this for a string representing a line in the output. Since it's a diff --git a/t/input.t b/t/input.t new file mode 100644 index 0000000..05b055a --- /dev/null +++ b/t/input.t @@ -0,0 +1,32 @@ +#!/usr/bin/env perl + +use strict; +use warnings; + +use Test::More tests => 3; +use File::Temp qw(:seekable); + +use Module::Release; + +BEGIN { + use File::Spec; + my $file = File::Spec->catfile( qw(. t lib setup_common.pl) ); + require $file; + } + +my $release = Module::Release->new; +my $temp_fh = File::Temp->new; +$temp_fh->write("to be or not to be\n"); +$temp_fh->flush(); +$temp_fh->seek( 0, SEEK_SET ); + +$release->{input_fh} = $temp_fh; +my $input = $release->_slurp; + +is( + $input, + "to be or not to be\n", + '_slurp returns content from input file handle' +); + +# vim: expandtab shiftwidth=4 From 6cc69b9509625ffc719444f2ab276e1a618faa4f Mon Sep 17 00:00:00 2001 From: Paul Cochrane Date: Fri, 20 Oct 2017 15:01:10 +0200 Subject: [PATCH 2/3] Add tests of get_env_var() These tests document the current behaviour of this method. --- Makefile.PL | 2 ++ t/get_env_var.t | 58 +++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 60 insertions(+) create mode 100644 t/get_env_var.t diff --git a/Makefile.PL b/Makefile.PL index 23200d6..445beb0 100644 --- a/Makefile.PL +++ b/Makefile.PL @@ -74,7 +74,9 @@ my %WriteMakefile = ( 'Test::More' => '1', 'Test::Output' => '0', 'Test::Without::Module' => '0', + 'Capture::Tiny' => '0', 'File::Temp' => '0', + 'Sub::Override' => '0', }, 'META_MERGE' => { diff --git a/t/get_env_var.t b/t/get_env_var.t new file mode 100644 index 0000000..c77ea3a --- /dev/null +++ b/t/get_env_var.t @@ -0,0 +1,58 @@ +#!/usr/bin/env perl + +use strict; +use warnings; + +use Test::More tests => 8; +use Sub::Override; +use Capture::Tiny qw( capture ); + +use Module::Release; + +BEGIN { + use File::Spec; + my $file = File::Spec->catfile( qw(. t lib setup_common.pl) ); + require $file; + } + +my $release = Module::Release->new; +$release->{'preset_field'} = 'preset'; +is( $release->get_env_var('PRESET_FIELD'), + 'preset', 'Preset field value returned' ); + +$ENV{'MOO'} = 'baa'; +is( $release->get_env_var('MOO'), + 'baa', 'Preset environment variable value returned' ); + +{ + my $terminal_input = + Sub::Override->new( 'Module::Release::_slurp' => sub { "baa\n" } ); + $ENV{'MOO'} = ''; + my ( $stdout, $stderr, @result ) = capture { $release->get_env_var('MOO') }; + is( + $stdout, + 'MOO is not set. Enter it now: ', + 'Empty environment variable prompts for value' + ); + is( $result[0], 'baa', 'Variable read from input' ); +} + +{ + my $terminal_input = + Sub::Override->new( 'Module::Release::_slurp' => sub { "\n" } ); + $ENV{'MOO'} = undef; + $release->turn_debug_on; + my ( $stdout, $stderr, @result ) = capture { $release->get_env_var('MOO') }; + is( + $stdout, + 'MOO is not set. Enter it now: ', + 'Undef environment variable prompts for value' + ); + is( + $stderr, + "MOO not supplied. Aborting...\n", + "Error message about missing variable shown in debug mode" + ); +} + +# vim: expandtab shiftwidth=4 From aaf46d0eac7374f263f85b9370394c85ff0313bd Mon Sep 17 00:00:00 2001 From: Paul Cochrane Date: Fri, 20 Oct 2017 15:14:23 +0200 Subject: [PATCH 3/3] Avoid echoing passwords to the terminal If we want to enter the CPAN_PASS value, we don't want to see it echoed to the terminal. The implementation used here is based off that used for the `cpan-upload` script from `CPAN::Uploader`. Unfortunately, testing that the value isn't echoed is very difficult, hence the tests added here only exercise the code path using the CPAN_PASS variable and check that the value returned from `get_env_var` is still that that one would expect. --- Makefile.PL | 1 + lib/Module/Release.pm | 12 +++++++++++- t/get_env_var.t | 15 ++++++++++++++- 3 files changed, 26 insertions(+), 2 deletions(-) diff --git a/Makefile.PL b/Makefile.PL index 445beb0..acc3ffa 100644 --- a/Makefile.PL +++ b/Makefile.PL @@ -67,6 +67,7 @@ my %WriteMakefile = ( 'ConfigReader::Simple' => '0', 'IO::Null' => '0', 'Mojolicious' => '4.50', + 'Term::ReadKey' => '0', 'URI' => '0', }, diff --git a/lib/Module/Release.pm b/lib/Module/Release.pm index 173f9c8..67dad5e 100644 --- a/lib/Module/Release.pm +++ b/lib/Module/Release.pm @@ -1302,7 +1302,17 @@ sub get_env_var { return $pass if defined( $pass ) && length( $pass ); $self->_print( "$field is not set. Enter it now: " ); - $pass = $self->_slurp; + if ($field eq 'CPAN_PASS') { + # don't echo passwords to the screen + require Term::ReadKey; + local $| = 1; + Term::ReadKey::ReadMode('noecho'); + $pass = $self->_slurp; + Term::ReadKey::ReadMode('restore'); + } + else { + $pass = $self->_slurp; + } chomp $pass; return $pass if defined( $pass ) && length( $pass ); diff --git a/t/get_env_var.t b/t/get_env_var.t index c77ea3a..ded2f9b 100644 --- a/t/get_env_var.t +++ b/t/get_env_var.t @@ -3,7 +3,7 @@ use strict; use warnings; -use Test::More tests => 8; +use Test::More tests => 10; use Sub::Override; use Capture::Tiny qw( capture ); @@ -55,4 +55,17 @@ is( $release->get_env_var('MOO'), ); } +{ + my $terminal_input = + Sub::Override->new( 'Module::Release::_slurp' => sub { "s3cr3t\n" } ); + $ENV{'CPAN_PASS'} = undef; + my ( $stdout, $stderr, @result ) = capture { $release->get_env_var('CPAN_PASS') }; + is( + $stdout, + 'CPAN_PASS is not set. Enter it now: ', + 'Undef CPAN_PASS variable prompts for value' + ); + is( $result[0], 's3cr3t', 'Variable password from input' ); +} + # vim: expandtab shiftwidth=4