Refactor File module to improve test coverage.
Refactor File and BackupCommon modules to improve test coverage.
Added unit tests for low-level functions in the File and BackupCommon modules.
Split test modules into separate files to make the code more maintainable. Tests are dynamically loaded by name rather than requiring an if-else block.
diff --git a/lib/pgBackRest/BackupCommon.pm b/lib/pgBackRest/BackupCommon.pm index 9ee51e924..3e58a12be 100644 --- a/lib/pgBackRest/BackupCommon.pm +++ b/lib/pgBackRest/BackupCommon.pm @@ -22,7 +22,9 @@ use constant LINK_LATEST => OPTION_DE push @EXPORT, qw(LINK_LATEST); #################################################################################################################################### -# backupRegExpGet - Generate a regexp depending on the backups that need to be found +# backupRegExpGet +# +# Generate a regexp depending on the backups that need to be found. #################################################################################################################################### sub backupRegExpGet { @@ -44,51 +46,58 @@ sub backupRegExpGet {name => 'bAnchor', default => true} ); - if (!$bFull && !$bDifferential && !$bIncremental) + # One of the types must be selected + if (!($bFull || $bDifferential || $bIncremental)) { - confess &log(ASSERT, 'one parameter must be true'); + confess &log(ASSERT, 'at least one backup type must be selected'); } + # Standard regexp to match date and time formattting my $strDateTimeRegExp = "[0-9]{8}\\-[0-9]{6}"; - my $strRegExp = $bAnchor ? '^' : ''; - - if ($bFull || $bDifferential || $bIncremental) - { - $strRegExp .= $strDateTimeRegExp . 'F'; - } + # Start the expression with the anchor if requested, date/time regexp and full backup indicator + my $strRegExp = ($bAnchor ? '^' : '') . $strDateTimeRegExp . 'F'; + # Add the diff and/or incr expressions if requested if ($bDifferential || $bIncremental) { + # If full requested then diff/incr is optional if ($bFull) { $strRegExp .= "(\\_"; } + # Else diff/incr is required else { $strRegExp .= "\\_"; } + # Append date/time regexp for diff/incr $strRegExp .= $strDateTimeRegExp; + # Filter on both diff/incr if ($bDifferential && $bIncremental) { $strRegExp .= '(D|I)'; } + # Else just diff elsif ($bDifferential) { $strRegExp .= 'D'; } + # Else just incr else { $strRegExp .= 'I'; } + # If full requested then diff/incr is optional if ($bFull) { $strRegExp .= '){0,1}'; } } + # Append the end anchor if requested $strRegExp .= $bAnchor ? "\$" : ''; # Return from function and log return values if any @@ -103,6 +112,8 @@ push @EXPORT, qw(backupRegExpGet); #################################################################################################################################### # backupLabelFormat +# +# Format the label for a backup. #################################################################################################################################### sub backupLabelFormat { @@ -122,32 +133,41 @@ sub backupLabelFormat {name => 'lTimestampStop', trace => true} ); + # Full backup label my $strBackupLabel; if ($strType eq BACKUP_TYPE_FULL) { + # Last backup label must not be defined if (defined($strBackupLabelLast)) { - confess &log(ASSERT, "strBackupPathLast cannot be defined when type = ${strType}"); + confess &log(ASSERT, "strBackupLabelLast must not be defined when strType = '${strType}'"); } + # Format the timestamp and add the full indicator $strBackupLabel = timestampFileFormat(undef, $lTimestampStop) . 'F'; } + # Else diff or incr label else { + # Last backup label must be defined if (!defined($strBackupLabelLast)) { - confess &log(ASSERT, "strBackupLabelLast must be defined when type = ${strType}"); + confess &log(ASSERT, "strBackupLabelLast must be defined when strType = '${strType}'"); } + # Get the full backup portion of the last backup label $strBackupLabel = substr($strBackupLabelLast, 0, 16); + # Format the timestamp $strBackupLabel .= '_' . timestampFileFormat(undef, $lTimestampStop); + # Add the diff indicator if ($strType eq BACKUP_TYPE_DIFF) { $strBackupLabel .= 'D'; } + # Else incr indicator else { $strBackupLabel .= 'I'; diff --git a/test/lib/pgBackRestTest/Backup/BackupUnitTest.pm b/test/lib/pgBackRestTest/Backup/BackupUnitTest.pm new file mode 100644 index 000000000..abe252ec5 --- /dev/null +++ b/test/lib/pgBackRestTest/Backup/BackupUnitTest.pm @@ -0,0 +1,110 @@ +#################################################################################################################################### +# BackupUnitTest.pm - Tests for Backup module +#################################################################################################################################### +package pgBackRestTest::Backup::BackupUnitTest; +use parent 'pgBackRestTest::Backup::BackupCommonTest'; + +#################################################################################################################################### +# Perl includes +#################################################################################################################################### +use strict; +use warnings FATAL => qw(all); +use Carp qw(confess); + +use File::Basename qw(dirname); + +use pgBackRest::BackupCommon; +use pgBackRest::Common::Exception; +use pgBackRest::Common::Log; +use pgBackRest::Common::String; +use pgBackRest::Config::Config; + +#################################################################################################################################### +# run +#################################################################################################################################### +sub run +{ + my $self = shift; + + # Increment the run, log, and decide whether this unit test should be run + if (!$self->begin('unit')) {return} + + # Unit tests for backupLabelFormat() + #----------------------------------------------------------------------------------------------------------------------- + { + # Test full backup label + my $strBackupLabelFull = timestampFileFormat(undef, 1482000000) . 'F'; + $self->testResult(sub {backupLabelFormat(BACKUP_TYPE_FULL, undef, 1482000000)}, $strBackupLabelFull); + + # Make sure that an assertion is thrown if strBackupLabelLast is passed when formatting a full label + $self->testException( + sub {backupLabelFormat(BACKUP_TYPE_FULL, $strBackupLabelFull, 1482000000)}, + ERROR_ASSERT, "strBackupLabelLast must not be defined when strType = 'full'"); + + # Test diff backup label + my $strBackupLabelDiff = "${strBackupLabelFull}_" . timestampFileFormat(undef, 1482000400) . 'D'; + $self->testResult(sub {backupLabelFormat(BACKUP_TYPE_DIFF, $strBackupLabelFull, 1482000400)}, $strBackupLabelDiff); + + # Make sure that an assertion is thrown if strBackupLabelLast is not passed when formatting a diff label. The same + # check is used from incr labels so no need for a separate test. + $self->testException( + sub {backupLabelFormat(BACKUP_TYPE_DIFF, undef, 1482000400)}, + ERROR_ASSERT, "strBackupLabelLast must be defined when strType = 'diff'"); + + # Test incr backup label + $self->testResult( + sub {backupLabelFormat(BACKUP_TYPE_INCR, $strBackupLabelDiff, 1482000800)}, + "${strBackupLabelFull}_" . timestampFileFormat(undef, 1482000800) . 'I'); + } + + # Unit tests for backupRegExpGet() + #----------------------------------------------------------------------------------------------------------------------- + { + # Expected results matrix + my $hExpected = {}; + $hExpected->{&false}{&false}{&true}{&false} = '[0-9]{8}\-[0-9]{6}F\_[0-9]{8}\-[0-9]{6}I'; + $hExpected->{&false}{&false}{&true}{&true} = '^[0-9]{8}\-[0-9]{6}F\_[0-9]{8}\-[0-9]{6}I$'; + $hExpected->{&false}{&true}{&false}{&false} = '[0-9]{8}\-[0-9]{6}F\_[0-9]{8}\-[0-9]{6}D'; + $hExpected->{&false}{&true}{&false}{&true} = '^[0-9]{8}\-[0-9]{6}F\_[0-9]{8}\-[0-9]{6}D$'; + $hExpected->{&false}{&true}{&true}{&false} = '[0-9]{8}\-[0-9]{6}F\_[0-9]{8}\-[0-9]{6}(D|I)'; + $hExpected->{&false}{&true}{&true}{&true} = '^[0-9]{8}\-[0-9]{6}F\_[0-9]{8}\-[0-9]{6}(D|I)$'; + $hExpected->{&true}{&false}{&false}{&false} = '[0-9]{8}\-[0-9]{6}F'; + $hExpected->{&true}{&false}{&false}{&true} = '^[0-9]{8}\-[0-9]{6}F$'; + $hExpected->{&true}{&false}{&true}{&false} = '[0-9]{8}\-[0-9]{6}F(\_[0-9]{8}\-[0-9]{6}I){0,1}'; + $hExpected->{&true}{&false}{&true}{&true} = '^[0-9]{8}\-[0-9]{6}F(\_[0-9]{8}\-[0-9]{6}I){0,1}$'; + $hExpected->{&true}{&true}{&false}{&false} = '[0-9]{8}\-[0-9]{6}F(\_[0-9]{8}\-[0-9]{6}D){0,1}'; + $hExpected->{&true}{&true}{&false}{&true} = '^[0-9]{8}\-[0-9]{6}F(\_[0-9]{8}\-[0-9]{6}D){0,1}$'; + $hExpected->{&true}{&true}{&true}{&false} = '[0-9]{8}\-[0-9]{6}F(\_[0-9]{8}\-[0-9]{6}(D|I)){0,1}'; + $hExpected->{&true}{&true}{&true}{&true} = '^[0-9]{8}\-[0-9]{6}F(\_[0-9]{8}\-[0-9]{6}(D|I)){0,1}$'; + + # Iterate though all possible combinations + for (my $bFull = false; $bFull <= true; $bFull++) + { + for (my $bDiff = false; $bDiff <= true; $bDiff++) + { + for (my $bIncr = false; $bIncr <= true; $bIncr++) + { + for (my $bAnchor = false; $bAnchor <= true; $bAnchor++) + { + # Make sure that an assertion is thrown if no types are requested + if (!($bFull || $bDiff || $bIncr)) + { + $self->testException( + sub {backupRegExpGet($bFull, $bDiff, $bIncr, $bAnchor)}, + ERROR_ASSERT, 'at least one backup type must be selected'); + } + # Else make sure the returned value is correct + else + { + $self->testResult( + sub {backupRegExpGet($bFull, $bDiff, $bIncr, $bAnchor)}, + $hExpected->{$bFull}{$bDiff}{$bIncr}{$bAnchor}); + } + } + } + } + } + } +} + +1; diff --git a/test/lib/pgBackRestTest/Common/DefineTest.pm b/test/lib/pgBackRestTest/Common/DefineTest.pm index db9b97b5c..765860a5c 100644 --- a/test/lib/pgBackRestTest/Common/DefineTest.pm +++ b/test/lib/pgBackRestTest/Common/DefineTest.pm @@ -153,6 +153,12 @@ my $oTestDef = &TESTDEF_TEST => [ + { + &TESTDEF_TEST_NAME => 'unit', + &TESTDEF_TEST_TOTAL => 1, + &TESTDEF_TEST_INDIVIDUAL => false, + &TESTDEF_EXPECT => false, + }, { &TESTDEF_TEST_NAME => 'archive-push', &TESTDEF_TEST_TOTAL => 8 diff --git a/test/lib/pgBackRestTest/Common/RunTest.pm b/test/lib/pgBackRestTest/Common/RunTest.pm index 284b07f61..0518cd92f 100644 --- a/test/lib/pgBackRestTest/Common/RunTest.pm +++ b/test/lib/pgBackRestTest/Common/RunTest.pm @@ -180,7 +180,13 @@ sub begin else { my $hModule = testDefModuleGet($self->{strModule}); - $self->{bExpect} = $hModule->{&TESTDEF_EXPECT} ? true : false; + my $hModuleTest = testDefModuleTestGet($hModule, $self->{strModuleTest}); + $self->{bExpect} = + defined($hModuleTest->{&TESTDEF_EXPECT}) ? + ($hModuleTest->{&TESTDEF_EXPECT} ? true : false) : + (defined($hModule->{&TESTDEF_EXPECT}) ? + ($hModule->{&TESTDEF_EXPECT} ? true : false) : + false); } # Increment the run counter;