Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

try with resources #5

Merged
merged 2 commits into from Jul 17, 2020
Merged

try with resources #5

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a3a9da9
try with resources
elharo Jul 16, 2020
e2242b7
merge master
elharo Jul 17, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
12 changes: 1 addition & 11 deletions src/test/java/org/apache/maven/plugins/resources/ResourcesMojoTest.java
View file
Open in desktop
Expand Up @@ -37,7 +37,6 @@
import org.apache.maven.plugins.resources.stub.MavenProjectResourcesStub;
import org.codehaus.plexus.PlexusContainer;
import org.codehaus.plexus.util.FileUtils;
import org.codehaus.plexus.util.IOUtil;
import org.eclipse.aether.RepositorySystemSession;

public class ResourcesMojoTest
Expand Down Expand Up @@ -691,18 +690,9 @@ private void assertContent( String fileName, String data )
throws IOException
{
assertTrue( FileUtils.fileExists( fileName ) );

BufferedReader reader = null;
try
try( BufferedReader reader = new BufferedReader( new FileReader( fileName ) ) )
{
reader = new BufferedReader( new FileReader( fileName ) );
assertEquals( data, reader.readLine() );
reader.close();
reader = null;
}
finally
{
IOUtil.close( reader );
}
}

Expand Down
15 changes: 3 additions & 12 deletions src/test/java/org/apache/maven/plugins/resources/stub/MavenProjectBuildStub.java
View file
Open in desktop
Expand Up @@ -240,7 +240,7 @@ private void createDirectories( String parent, String testparent )
}
}

private void createFiles( String parent, String testparent )
private void createFiles( String parent, String testparent ) throws IOException
{
File currentFile;

Expand Down Expand Up @@ -278,15 +278,8 @@ private void createFiles( String parent, String testparent )

if ( !currentFile.exists() )
{
try
{
currentFile.createNewFile();
populateFile( currentFile );
}
catch ( IOException io )
{
//TODO: handle exception
}
currentFile.createNewFile();
populateFile( currentFile );
}
}
}
Expand All @@ -300,8 +293,6 @@ private void populateFile( File file ) throws IOException
try ( FileOutputStream outputStream = new FileOutputStream( file ) )
{
outputStream.write( data.getBytes() );
outputStream.flush();
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't think the .flush() is guaranteed by try-with-resources... but perhaps by not wrapping FOS in a BufferedOutputStream then that's not a problem?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

close also flushes

Copy link
Contributor

@pzygielo pzygielo Jul 17, 2020 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think @roxspring is right about FOS not wrapped in BOS not being problem for flushing or not. But I'm not sure that close also flushes. There is flush called in FOS.finalize but close inherited from OutputStream is no-op (immaterial), as FOS.close doesn't seem to call flush neither.
On the other hand close of BufferedOutputStream flushes indeed.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Calling flush just before close is generally useless cause close ensures all data have been written.
That said we are speaking of a FileOutputStream where flush() is a noop so it is even safer to drop.

outputStream.close();
}
}
}
Expand Down