From 090d10a2886d570e8706258d697516354a27ad74 Mon Sep 17 00:00:00 2001 From: Ben Butler-Cole Date: Fri, 28 Oct 2016 11:59:16 +0100 Subject: [PATCH] Refactor CC results so that we don't have to expose internals for testing --- .../consistency/CheckConsistencyCommand.java | 2 +- .../consistency/ConsistencyCheckService.java | 57 ++++++++++++++----- .../CheckConsistencyCommandTest.java | 10 ++-- ...onsistencyCheckServiceIntegrationTest.java | 36 ++++++------ 4 files changed, 67 insertions(+), 38 deletions(-) diff --git a/community/consistency-check/src/main/java/org/neo4j/consistency/CheckConsistencyCommand.java b/community/consistency-check/src/main/java/org/neo4j/consistency/CheckConsistencyCommand.java index 62be1cd85d67..cdd66e11dd5a 100644 --- a/community/consistency-check/src/main/java/org/neo4j/consistency/CheckConsistencyCommand.java +++ b/community/consistency-check/src/main/java/org/neo4j/consistency/CheckConsistencyCommand.java @@ -159,7 +159,7 @@ public void execute( String[] args ) throws IncorrectUsage, CommandFailed if ( !consistencyCheckResult.isSuccessful() ) { throw new CommandFailed( format( "Inconsistencies found. See '%s' for details.", - consistencyCheckService.chooseReportPath( reportDir ).toString() ) ); + consistencyCheckResult.reportFile() ) ); } } catch ( ConsistencyCheckIncompleteException | IOException e ) diff --git a/community/consistency-check/src/main/java/org/neo4j/consistency/ConsistencyCheckService.java b/community/consistency-check/src/main/java/org/neo4j/consistency/ConsistencyCheckService.java index db7de72326fe..bf09b0670711 100644 --- a/community/consistency-check/src/main/java/org/neo4j/consistency/ConsistencyCheckService.java +++ b/community/consistency-check/src/main/java/org/neo4j/consistency/ConsistencyCheckService.java @@ -62,6 +62,8 @@ import org.neo4j.logging.Log; import org.neo4j.logging.LogProvider; +import static java.lang.String.format; + import static org.neo4j.helpers.collection.MapUtil.stringMap; import static org.neo4j.io.file.Files.createOrOpenAsOuputStream; @@ -207,12 +209,12 @@ public Result runFullConsistencyCheck( final File storeDir, Config tuningConfigu if ( !summary.isConsistent() ) { log.warn( "See '%s' for a detailed consistency report.", reportFile.getPath() ); - return Result.FAILURE; + return Result.failure( reportFile ); } - return Result.SUCCESS; + return Result.success( reportFile ); } - public File chooseReportPath( File reportDir ) + private File chooseReportPath( File reportDir ) { return new File( reportDir, defaultLogFileName( timestamp ) ); } @@ -228,27 +230,52 @@ private File defaultReportDir( Config tuningConfiguration, File storeDir ) return tuningConfiguration.get( GraphDatabaseSettings.logs_directory ); } - public static String defaultLogFileName( Date date ) + private static String defaultLogFileName( Date date ) { - final String formattedDate = new SimpleDateFormat( "yyyy-MM-dd.HH.mm.ss" ).format( date ); - return String.format( "inconsistencies-%s.report", formattedDate ); + return format( "inconsistencies-%s.report", new SimpleDateFormat( "yyyy-MM-dd.HH.mm.ss" ).format( date ) ); } - public enum Result + public interface Result { - FAILURE( false ), SUCCESS( true ); - - private final boolean successful; - - Result( boolean successful ) + static Result failure( File reportFile ) { - this.successful = successful; + return new Result() + { + @Override + public boolean isSuccessful() + { + return false; + } + + @Override + public File reportFile() + { + return reportFile; + } + }; } - public boolean isSuccessful() + static Result success( File reportFile ) { - return this.successful; + return new Result() + { + @Override + public boolean isSuccessful() + { + return true; + } + + @Override + public File reportFile() + { + return reportFile; + } + }; } + + boolean isSuccessful(); + + File reportFile(); } public static int defaultConsistencyCheckThreadsNumber() diff --git a/community/consistency-check/src/test/java/org/neo4j/consistency/CheckConsistencyCommandTest.java b/community/consistency-check/src/test/java/org/neo4j/consistency/CheckConsistencyCommandTest.java index a93cb131f0ce..8d392c0c3d52 100644 --- a/community/consistency-check/src/test/java/org/neo4j/consistency/CheckConsistencyCommandTest.java +++ b/community/consistency-check/src/test/java/org/neo4j/consistency/CheckConsistencyCommandTest.java @@ -89,7 +89,7 @@ public void runsConsistencyChecker() throws Exception when( consistencyCheckService .runFullConsistencyCheck( eq( databasePath ), any( Config.class ), any( ProgressMonitorFactory.class ), any( LogProvider.class ), any( FileSystemAbstraction.class ), eq( false ), anyObject() ) ) - .thenReturn( ConsistencyCheckService.Result.SUCCESS ); + .thenReturn( ConsistencyCheckService.Result.success( null ) ); checkConsistencyCommand.execute( new String[]{"--database=mydb"} ); @@ -114,7 +114,7 @@ public void enablesVerbosity() throws Exception when( consistencyCheckService .runFullConsistencyCheck( eq( databasePath ), any( Config.class ), any( ProgressMonitorFactory.class ), any( LogProvider.class ), any( FileSystemAbstraction.class ), eq( true ), anyObject() ) ) - .thenReturn( ConsistencyCheckService.Result.SUCCESS ); + .thenReturn( ConsistencyCheckService.Result.success( null ) ); checkConsistencyCommand.execute( new String[]{"--database=mydb", "--verbose"} ); @@ -138,9 +138,7 @@ public void failsWhenInconsistenciesAreFound() throws Exception when( consistencyCheckService .runFullConsistencyCheck( eq( databasePath ), any( Config.class ), any( ProgressMonitorFactory.class ), any( LogProvider.class ), any( FileSystemAbstraction.class ), eq( true ), anyObject() ) ) - .thenReturn( ConsistencyCheckService.Result.FAILURE ); - when( consistencyCheckService.chooseReportPath( new File(".").getCanonicalFile() ) ) - .thenReturn( new File("/the/report/path") ); + .thenReturn( ConsistencyCheckService.Result.failure( new File( "/the/report/path" ) ) ); try { @@ -166,7 +164,7 @@ public void shouldWriteReportFileToCurrentDirectoryByDefault() consistencyCheckService ); stub( consistencyCheckService.runFullConsistencyCheck( anyObject(), anyObject(), anyObject(), anyObject(), - anyObject(), anyBoolean(), anyObject() ) ).toReturn( ConsistencyCheckService.Result.SUCCESS ); + anyObject(), anyBoolean(), anyObject() ) ).toReturn( ConsistencyCheckService.Result.success( null ) ); checkConsistencyCommand.execute( new String[]{"--database=mydb"} ); diff --git a/community/consistency-check/src/test/java/org/neo4j/consistency/ConsistencyCheckServiceIntegrationTest.java b/community/consistency-check/src/test/java/org/neo4j/consistency/ConsistencyCheckServiceIntegrationTest.java index edaf005f12e3..be81f683176c 100644 --- a/community/consistency-check/src/test/java/org/neo4j/consistency/ConsistencyCheckServiceIntegrationTest.java +++ b/community/consistency-check/src/test/java/org/neo4j/consistency/ConsistencyCheckServiceIntegrationTest.java @@ -19,17 +19,18 @@ */ package org.neo4j.consistency; -import org.apache.commons.lang3.StringUtils; -import org.junit.Rule; -import org.junit.Test; -import org.junit.rules.RuleChain; - import java.io.File; import java.io.IOException; +import java.text.SimpleDateFormat; import java.util.Date; import java.util.HashMap; import java.util.Map; +import org.apache.commons.lang3.StringUtils; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.RuleChain; + import org.neo4j.consistency.ConsistencyCheckService.Result; import org.neo4j.consistency.checking.GraphStoreFixture; import org.neo4j.consistency.checking.full.ConsistencyCheckIncompleteException; @@ -50,10 +51,12 @@ import org.neo4j.test.TestGraphDatabaseFactory; import org.neo4j.test.rule.TestDirectory; +import static java.lang.String.format; + import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; -import static org.neo4j.consistency.ConsistencyCheckService.defaultLogFileName; + import static org.neo4j.helpers.collection.MapUtil.stringMap; import static org.neo4j.test.Property.property; import static org.neo4j.test.Property.set; @@ -92,8 +95,8 @@ public void shouldSucceedIfStoreIsConsistent() throws Exception ConsistencyCheckService.Result result = runFullConsistencyCheck( service, configuration ); // then - assertEquals( ConsistencyCheckService.Result.SUCCESS, result ); - File reportFile = new File( fixture.directory(), defaultLogFileName( timestamp ) ); + assertTrue( result.isSuccessful() ); + File reportFile = result.reportFile(); assertFalse( "Unexpected generation of consistency check report file: " + reportFile, reportFile.exists() ); } @@ -104,19 +107,20 @@ public void shouldFailIfTheStoreInNotConsistent() throws Exception breakNodeStore(); Date timestamp = new Date(); ConsistencyCheckService service = new ConsistencyCheckService( timestamp ); + String logsDir = testDirectory.directory().getPath(); Config configuration = new Config( - settings( GraphDatabaseSettings.logs_directory.name(), testDirectory.directory().getPath() ), + settings( GraphDatabaseSettings.logs_directory.name(), logsDir ), GraphDatabaseSettings.class, ConsistencyCheckSettings.class ); // when ConsistencyCheckService.Result result = runFullConsistencyCheck( service, configuration ); // then - assertEquals( ConsistencyCheckService.Result.FAILURE, result ); - File reportFile = new File( - configuration.get( GraphDatabaseSettings.logs_directory ), - defaultLogFileName( timestamp ) ); - assertTrue( "Inconsistency report file " + reportFile + " not generated", reportFile.exists() ); + assertFalse( result.isSuccessful() ); + String reportFile = format( "inconsistencies-%s.report", + new SimpleDateFormat( "yyyy-MM-dd.HH.mm.ss" ).format( timestamp ) ); + assertEquals( new File( logsDir, reportFile ), result.reportFile() ); + assertTrue( "Inconsistency report file not generated", result.reportFile().exists() ); } @Test @@ -148,7 +152,7 @@ public void shouldNotReportDuplicateForHugeLongValues() throws Exception Result result = runFullConsistencyCheck( service, configuration ); // then - assertEquals( ConsistencyCheckService.Result.SUCCESS, result ); + assertTrue( result.isSuccessful() ); } @Test @@ -173,7 +177,7 @@ public void shouldAllowGraphCheckDisabled() throws IOException, ConsistencyCheck Result result = runFullConsistencyCheck( service, configuration ); // then - assertEquals( ConsistencyCheckService.Result.SUCCESS, result ); + assertTrue( result.isSuccessful() ); } private GraphDatabaseService getGraphDatabaseService()