From 37e5014d1c5fbc06292e47924f8cc609720d0d97 Mon Sep 17 00:00:00 2001 From: Przemek Hugh Kaznowski Date: Fri, 19 Jan 2018 15:55:34 +0000 Subject: [PATCH] Addressed Andrew, Praveena and Konstantin comments --- .../impl/OnlineBackupContextBuilder.java | 4 +- .../backup/impl/ParametrisedOutsideWorld.java | 3 ++ .../commandline/admin/BackupUsageTest.java | 54 ++++++++++--------- 3 files changed, 34 insertions(+), 27 deletions(-) diff --git a/enterprise/backup/src/main/java/org/neo4j/backup/impl/OnlineBackupContextBuilder.java b/enterprise/backup/src/main/java/org/neo4j/backup/impl/OnlineBackupContextBuilder.java index 9270faae392d6..e13e5e78333b8 100644 --- a/enterprise/backup/src/main/java/org/neo4j/backup/impl/OnlineBackupContextBuilder.java +++ b/enterprise/backup/src/main/java/org/neo4j/backup/impl/OnlineBackupContextBuilder.java @@ -151,7 +151,7 @@ public OnlineBackupContext createContext( String... args ) throws IncorrectUsage OptionalHostnamePort address = toOptionalHostnamePortFromRawAddress( arguments.get( ARG_NAME_BACKUP_SOURCE ) ); - Path folder = getFolder( arguments ); + Path folder = getBackupDirectory( arguments ); String name = arguments.get( ARG_NAME_BACKUP_NAME ); boolean fallbackToFull = arguments.getBoolean( ARG_NAME_FALLBACK_FULL ); boolean doConsistencyCheck = arguments.getBoolean( ARG_NAME_CHECK_CONSISTENCY ); @@ -197,7 +197,7 @@ public OnlineBackupContext createContext( String... args ) throws IncorrectUsage } } - private Path getFolder( Arguments arguments ) throws CommandFailed + private Path getBackupDirectory( Arguments arguments ) throws CommandFailed { Path path = arguments.getMandatoryPath( ARG_NAME_BACKUP_DIRECTORY ); try diff --git a/enterprise/backup/src/main/java/org/neo4j/backup/impl/ParametrisedOutsideWorld.java b/enterprise/backup/src/main/java/org/neo4j/backup/impl/ParametrisedOutsideWorld.java index 4d5f315b2e48f..6cfcd484bd014 100644 --- a/enterprise/backup/src/main/java/org/neo4j/backup/impl/ParametrisedOutsideWorld.java +++ b/enterprise/backup/src/main/java/org/neo4j/backup/impl/ParametrisedOutsideWorld.java @@ -120,5 +120,8 @@ public InputStream inStream() public void close() throws IOException { fileSystemAbstraction.close(); + inStream().close(); + outStream().close(); + errorStream().close(); } } diff --git a/enterprise/backup/src/test/java/org/neo4j/commandline/admin/BackupUsageTest.java b/enterprise/backup/src/test/java/org/neo4j/commandline/admin/BackupUsageTest.java index fb676955162e9..c95ec8bcd99f6 100644 --- a/enterprise/backup/src/test/java/org/neo4j/commandline/admin/BackupUsageTest.java +++ b/enterprise/backup/src/test/java/org/neo4j/commandline/admin/BackupUsageTest.java @@ -19,15 +19,13 @@ */ package org.neo4j.commandline.admin; +import org.junit.Rule; import org.junit.Test; -import java.io.ByteArrayOutputStream; -import java.io.Console; -import java.io.File; import java.io.IOException; -import java.io.PrintStream; import java.io.UnsupportedEncodingException; import java.nio.file.Path; +import java.nio.file.Paths; import java.util.ArrayList; import java.util.Arrays; import java.util.List; @@ -36,12 +34,19 @@ import org.neo4j.backup.impl.OnlineBackupCommandProvider; import org.neo4j.backup.impl.ParametrisedOutsideWorld; import org.neo4j.io.fs.DefaultFileSystemAbstraction; -import org.neo4j.io.fs.FileSystemAbstraction; +import org.neo4j.test.rule.SuppressOutput; -import static org.junit.Assert.assertEquals; +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.not; +import static org.junit.Assert.assertThat; public class BackupUsageTest { + private static final Path HERE = Paths.get( "." ); + + @Rule + public SuppressOutput suppressOutput = SuppressOutput.suppressAll(); + OnlineBackupCommandProvider onlineBackupCommandProvider = new OnlineBackupCommandProvider(); CommandLocator commandLocator = AugmentedCommandLocator.fromFixedArray( onlineBackupCommandProvider ); @@ -52,8 +57,11 @@ public void outputMatchesExpectedForMissingBackupDir() throws UnsupportedEncodin String output = runBackup(); // then - String reason = "Missing argument 'backup-dir'\n\n"; - assertEquals( reason + BackupHelpOutput.BACKUP_OUTPUT, output ); + String reason = "Missing argument 'backup-dir'"; + assertThat( output, containsString( reason ) ); + + // and + assertThat( output, containsString( BackupHelpOutput.BACKUP_OUTPUT ) ); } @Test @@ -63,20 +71,24 @@ public void missingBackupName() throws UnsupportedEncodingException String output = runBackup( "--backup-dir=target" ); // then - String reason = "Missing argument 'name'\n\n"; - assertEquals( reason + BackupHelpOutput.BACKUP_OUTPUT, output ); + String reason = "Missing argument 'name'"; + assertThat( output, containsString( reason ) ); + + // and + assertThat( output, containsString( BackupHelpOutput.BACKUP_OUTPUT ) ); } @Test public void incorrectBackupDirectory() throws IOException { // when - Path backupDirectoryResolved = new File( "." ).toPath().toRealPath().resolve( "non_existing_dir" ); + Path backupDirectoryResolved = HERE.toRealPath().resolve( "non_existing_dir" ); String output = runBackup( "--backup-dir=non_existing_dir", "--name=mybackup" ); // then String reason = String.format( "command failed: Directory '%s' does not exist.\n", backupDirectoryResolved.toFile().toString() ); - assertEquals( reason, output ); + assertThat( output, containsString( reason ) ); + assertThat( output, not( containsString( BackupHelpOutput.BACKUP_OUTPUT ) ) ); } private String runBackup( String... args ) throws UnsupportedEncodingException @@ -86,24 +98,16 @@ private String runBackup( String... args ) throws UnsupportedEncodingException private String runBackup( boolean debug, String... args ) throws UnsupportedEncodingException { - ByteArrayOutputStream baos = new ByteArrayOutputStream(); - PrintStream printStream = new PrintStream( baos, true, "utf-8" ); - ParametrisedOutsideWorld outsideWorld = capturableOutputOutsideWorld( printStream ); + ParametrisedOutsideWorld outsideWorld = // ParametrisedOutsideWorld used for suppressing #close() doing System.exit() + new ParametrisedOutsideWorld( System.console(), System.out, System.err, System.in, new DefaultFileSystemAbstraction() ); AdminTool subject = new AdminTool( commandLocator, AugmentedBlockerLocator.fromList(), outsideWorld, debug ); - Path homeDir = new File( "." ).toPath(); - Path configDir = new File( "." ).toPath(); + Path homeDir = HERE; + Path configDir = HERE; List params = new ArrayList(); params.add( "backup" ); params.addAll( Arrays.asList( args ) ); String[] argArray = params.toArray( new String[params.size()] ); subject.execute( homeDir, configDir, argArray ); - return baos.toString(); - } - - private ParametrisedOutsideWorld capturableOutputOutsideWorld( PrintStream printStream ) - { - Console console = System.console(); - FileSystemAbstraction fileSystemAbstraction = new DefaultFileSystemAbstraction(); - return new ParametrisedOutsideWorld( console, printStream, printStream, System.in, fileSystemAbstraction ); + return suppressOutput.getErrorVoice().toString() + suppressOutput.getOutputVoice().toString(); } }