Skip to content

Conversation

@drumonii
Copy link
Contributor

@drumonii drumonii commented Mar 7, 2018

The only occurrence of it used was IOUtils.readLines in DataSourceInitializer. And since Spring Batch 4 requires Java 8, we can make use of Files.lines instead of common io's IOUtils.readLines.

Also noticed in the spring-batch-test module that commons-collections:commons-collections is not used? And hamcrest-library should be a testCompile?

@fmbenhassine
Copy link
Contributor

Thank you for this PR. Removing a dependency is always welcome!

I opened a JIRA ticket for this improvement: BATCH-2698.

Also noticed in the spring-batch-test module that commons-collections:commons-collections is not used? And hamcrest-library should be a testCompile?

Please open a separate ticket/PR for this.

@drumonii
Copy link
Contributor Author

drumonii commented Mar 8, 2018

Done! See BATCH-2701 and BATCH-2702. I'll open a PR for those changes too once this is merged in first.

@mminella
Copy link
Member

mminella commented Apr 5, 2018

Running a full build on this fails with a number of java.nio.file.FileSystemNotFoundException. Some quick Googling makes it sound like a FileSystem needs to be explicitly created (StackOverflow). Please update accordingly.

@drumonii drumonii force-pushed the remove-commons-io-from-test-module branch from bc3c9d4 to 197d2cb Compare April 6, 2018 03:15
@drumonii
Copy link
Contributor Author

drumonii commented Apr 6, 2018

Thanks @mminella for the update and info. Very helpful. I've updated my changes to include FileSystems.newFileSystem

@drumonii drumonii changed the title Remove commons io from test module BATCH-2701 - Remove commons io from test module Apr 10, 2018
@drumonii drumonii force-pushed the remove-commons-io-from-test-module branch from 197d2cb to 1c1a032 Compare April 19, 2018 02:50
@fmbenhassine
Copy link
Contributor

Hi @drumonii ,

Thank you for updating the PR. I would use Files.readAllLines which returns directly a List instead of creating a stream and then collecting its items into a list with a collector. What do you think?

Kr,
Mahmoud

@fmbenhassine fmbenhassine changed the title BATCH-2701 - Remove commons io from test module BATCH-2698 - Remove commons io from test module May 9, 2018
@fmbenhassine
Copy link
Contributor

fmbenhassine commented May 9, 2018

While testing this PR, I noticed that while the build is passing on the command line, some tests are failing when I run them from my IDE (I use IntelliJ Idea on MacOS with Oracle JDK 1.8.0_161 from both the IDE and command line). The tests in the following classes are failing: JobRepositoryTestUtilsTests, SampleFlowJobTests, SampleSimpleJobTests, SampleStepTests. The exception is the same:


java.lang.IllegalStateException: Failed to load ApplicationContext

	at org.springframework.test.context.cache.DefaultCacheAwareContextLoaderDelegate.loadContext(DefaultCacheAwareContextLoaderDelegate.java:125)
	at org.springframework.test.context.support.DefaultTestContext.getApplicationContext(DefaultTestContext.java:107)
	at org.springframework.test.context.support.DependencyInjectionTestExecutionListener.injectDependencies(DependencyInjectionTestExecutionListener.java:117)
	at org.springframework.test.context.support.DependencyInjectionTestExecutionListener.prepareTestInstance(DependencyInjectionTestExecutionListener.java:83)
	at org.springframework.test.context.TestContextManager.prepareTestInstance(TestContextManager.java:242)
	at org.springframework.test.context.junit4.SpringJUnit4ClassRunner.createTest(SpringJUnit4ClassRunner.java:227)
	at org.springframework.test.context.junit4.SpringJUnit4ClassRunner$1.runReflectiveCall(SpringJUnit4ClassRunner.java:289)
	at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
	at org.springframework.test.context.junit4.SpringJUnit4ClassRunner.methodBlock(SpringJUnit4ClassRunner.java:291)
	at org.springframework.test.context.junit4.SpringJUnit4ClassRunner.runChild(SpringJUnit4ClassRunner.java:246)
	at org.springframework.test.context.junit4.SpringJUnit4ClassRunner.runChild(SpringJUnit4ClassRunner.java:97)
	at org.junit.runners.ParentRunner$3.run(ParentRunner.java:290)
	at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:71)
	at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:288)
	at org.junit.runners.ParentRunner.access$000(ParentRunner.java:58)
	at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:268)
	at org.springframework.test.context.junit4.statements.RunBeforeTestClassCallbacks.evaluate(RunBeforeTestClassCallbacks.java:61)
	at org.springframework.test.context.junit4.statements.RunAfterTestClassCallbacks.evaluate(RunAfterTestClassCallbacks.java:70)
	at org.junit.runners.ParentRunner.run(ParentRunner.java:363)
	at org.springframework.test.context.junit4.SpringJUnit4ClassRunner.run(SpringJUnit4ClassRunner.java:190)
	at org.junit.runners.Suite.runChild(Suite.java:128)
	at org.junit.runners.Suite.runChild(Suite.java:27)
	at org.junit.runners.ParentRunner$3.run(ParentRunner.java:290)
	at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:71)
	at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:288)
	at org.junit.runners.ParentRunner.access$000(ParentRunner.java:58)
	at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:268)
	at org.junit.runners.ParentRunner.run(ParentRunner.java:363)
	at org.junit.runner.JUnitCore.run(JUnitCore.java:137)
	at com.intellij.junit4.JUnit4IdeaTestRunner.startRunnerWithArgs(JUnit4IdeaTestRunner.java:68)
	at com.intellij.rt.execution.junit.IdeaTestRunner$Repeater.startRunnerWithArgs(IdeaTestRunner.java:47)
	at com.intellij.rt.execution.junit.JUnitStarter.prepareStreamsAndStart(JUnitStarter.java:242)
	at com.intellij.rt.execution.junit.JUnitStarter.main(JUnitStarter.java:70)
Caused by: org.springframework.beans.factory.BeanCreationException: Error creating bean with name 'dataSourceInitializer' defined in class path resource [data-source-context.xml]: Invocation of init method failed; nested exception is java.lang.IllegalArgumentException: Path component should be '/'
	at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.initializeBean(AbstractAutowireCapableBeanFactory.java:1704)
	at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.doCreateBean(AbstractAutowireCapableBeanFactory.java:583)
	at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.createBean(AbstractAutowireCapableBeanFactory.java:502)
	at org.springframework.beans.factory.support.AbstractBeanFactory.lambda$doGetBean$0(AbstractBeanFactory.java:312)
	at org.springframework.beans.factory.support.DefaultSingletonBeanRegistry.getSingleton(DefaultSingletonBeanRegistry.java:228)
	at org.springframework.beans.factory.support.AbstractBeanFactory.doGetBean(AbstractBeanFactory.java:310)
	at org.springframework.beans.factory.support.AbstractBeanFactory.getBean(AbstractBeanFactory.java:200)
	at org.springframework.beans.factory.support.DefaultListableBeanFactory.preInstantiateSingletons(DefaultListableBeanFactory.java:756)
	at org.springframework.context.support.AbstractApplicationContext.finishBeanFactoryInitialization(AbstractApplicationContext.java:868)
	at org.springframework.context.support.AbstractApplicationContext.refresh(AbstractApplicationContext.java:549)
	at org.springframework.test.context.support.AbstractGenericContextLoader.loadContext(AbstractGenericContextLoader.java:128)
	at org.springframework.test.context.support.AbstractGenericContextLoader.loadContext(AbstractGenericContextLoader.java:60)
	at org.springframework.test.context.support.AbstractDelegatingSmartContextLoader.delegateLoading(AbstractDelegatingSmartContextLoader.java:109)
	at org.springframework.test.context.support.AbstractDelegatingSmartContextLoader.loadContext(AbstractDelegatingSmartContextLoader.java:246)
	at org.springframework.test.context.cache.DefaultCacheAwareContextLoaderDelegate.loadContextInternal(DefaultCacheAwareContextLoaderDelegate.java:99)
	at org.springframework.test.context.cache.DefaultCacheAwareContextLoaderDelegate.loadContext(DefaultCacheAwareContextLoaderDelegate.java:117)
	... 32 more
Caused by: java.lang.IllegalArgumentException: Path component should be '/'
	at sun.nio.fs.UnixFileSystemProvider.checkUri(UnixFileSystemProvider.java:77)
	at sun.nio.fs.UnixFileSystemProvider.getFileSystem(UnixFileSystemProvider.java:92)
	at java.nio.file.FileSystems.getFileSystem(FileSystems.java:221)
	at org.springframework.batch.test.DataSourceInitializer.initFileSystem(DataSourceInitializer.java:174)
	at org.springframework.batch.test.DataSourceInitializer.getScriptLines(DataSourceInitializer.java:168)
	at org.springframework.batch.test.DataSourceInitializer.access$100(DataSourceInitializer.java:63)
	at org.springframework.batch.test.DataSourceInitializer$1.doInTransaction(DataSourceInitializer.java:136)
	at org.springframework.batch.test.DataSourceInitializer$1.doInTransaction(DataSourceInitializer.java:130)
	at org.springframework.transaction.support.TransactionTemplate.execute(TransactionTemplate.java:140)
	at org.springframework.batch.test.DataSourceInitializer.doExecuteScript(DataSourceInitializer.java:130)
	at org.springframework.batch.test.DataSourceInitializer.initialize(DataSourceInitializer.java:119)
	at org.springframework.batch.test.DataSourceInitializer.afterPropertiesSet(DataSourceInitializer.java:110)
	at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.invokeInitMethods(AbstractAutowireCapableBeanFactory.java:1763)
	at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.initializeBean(AbstractAutowireCapableBeanFactory.java:1700)
	... 47 more

When I debug one of the cases, I see the the URI passed to the initFileSystem method is incorrect, on my machine it is: file:/Users/mbenhassine/projects/spring-batch/spring-batch-core/out/production/resources/org/springframework/batch/core/schema-drop-hsqldb.sql which should be file:///Users/mbenhassine/projects/spring-batch/spring-batch-core/out/production/resources/org/springframework/batch/core/schema-drop-hsqldb.sql. Notice the missing // after file:.

It looks like it is the same case as in the last answer of the SO thread given by Michael. When I add a fallback to the default file system like this:

private void initFileSystem(URI uri) throws IOException {
	try {
		FileSystems.getFileSystem(uri);
	}
	catch (FileSystemNotFoundException e) {
		FileSystems.newFileSystem(uri, Collections.emptyMap());
+	} catch(IllegalArgumentException e) {
+		FileSystems.getDefault();
	}
}

It works fine..

@drumonii May I ask you on which platform are you building/testing the project? Can you confirm those tests are passing from both your IDE and the command line?

Thank you upfront!

Prefer Files.lines over common io's readLines
@drumonii drumonii force-pushed the remove-commons-io-from-test-module branch from 1c1a032 to 24132b9 Compare May 10, 2018 02:31
@drumonii
Copy link
Contributor Author

Thanks for the in depth analysis @benas! I'm on Win 10, JDK "1.8.0_161", also using IntelliJ. I've used gradlew test to run all the tests on the command line. From there, the test classes you mentioned never failed for me. That's why I opted to leave out the FileSystems.getDefault. But interestingly, after running them in the IDE (which I've not done previously), they do fail with the same java.lang.IllegalArgumentException: Path component should be '/'. And after adjusting the initFileSystem to your suggestion, they pass for me as well. I'll make that update.

In regards to your other comment about the Files.readAllLines, I actually came across that myself yesterday, and was going to adjust this MR to it. So I'm in a total agreement!

@fmbenhassine
Copy link
Contributor

@drumonii Thank you for updating the PR. Now the tests pass from the IDE and the command line.

Rebased and merged as 305b4b8

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants