[GitHub] maven-surefire pull request #147: Reporters uses buffers to reduce number of...

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
14 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] maven-surefire pull request #147: Reporters uses buffers to reduce number of...

grkvlt
GitHub user jerrinot opened a pull request:

    https://github.com/apache/maven-surefire/pull/147

    Reporters uses buffers to reduce number of I/O operations

    Fixes SUREFIRE-1362 and SUREFIRE-1361
    Also adds buffering to FileReporter.

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/jerrinot/maven-surefire improvements/buffered-output/master

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/maven-surefire/pull/147.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #147
   
----
commit 921ac2da2b8668caeca7b7a6d6c2ab1d658df99d
Author: Jaromir Hamala <[hidden email]>
Date:   2017-04-14T20:11:16Z

    Reporters uses buffers to reduce number of I/O operations
   
    Fixes SUREFIRE-1362 and SUREFIRE-1361
    Also adds buffering to FileReporter.

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] maven-surefire issue #147: Reporters uses buffers to reduce number of I/O op...

grkvlt
Github user jerrinot commented on the issue:

    https://github.com/apache/maven-surefire/pull/147
 
    I've got a failure in the [Surefire1295AttributeJvmCrashesToTestsIT](https://github.com/jerrinot/maven-surefire/blob/93d3f8a9e41a334a5e1dacdb511e5d6596f04162/surefire-integration-tests/src/test/java/org/apache/maven/surefire/its/jiras/Surefire1295AttributeJvmCrashesToTestsIT.java#L43-L43)
   
    That's certainly suspicious given the nature of the changeset. However I cannot reproduce it anymore.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] maven-surefire pull request #147: Reporters use buffers to reduce number of ...

grkvlt
In reply to this post by grkvlt
Github user Tibor17 commented on a diff in the pull request:

    https://github.com/apache/maven-surefire/pull/147#discussion_r111643120
 
    --- Diff: maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/report/ConsoleOutputFileReporter.java ---
    @@ -91,7 +93,7 @@ public void writeTestOutput( byte[] buf, int off, int len, boolean stdout )
                         reportsDirectory.mkdirs();
                     }
                     File file = getReportFile( reportsDirectory, reportEntryName, reportNameSuffix, "-output.txt" );
    -                fileOutputStream = new FileOutputStream( file );
    +                fileOutputStream = new BufferedOutputStream( new FileOutputStream( file ), 16 * 1024 );
    --- End diff --
   
    Can you please override (git amend) this commit and add Writer.close() in finally block?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] maven-surefire pull request #147: Reporters use buffers to reduce number of ...

grkvlt
In reply to this post by grkvlt
Github user Tibor17 commented on a diff in the pull request:

    https://github.com/apache/maven-surefire/pull/147#discussion_r111643377
 
    --- Diff: maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/report/ConsoleOutputFileReporter.java ---
    @@ -91,7 +93,7 @@ public void writeTestOutput( byte[] buf, int off, int len, boolean stdout )
                         reportsDirectory.mkdirs();
                     }
                     File file = getReportFile( reportsDirectory, reportEntryName, reportNameSuffix, "-output.txt" );
    -                fileOutputStream = new FileOutputStream( file );
    +                fileOutputStream = new BufferedOutputStream( new FileOutputStream( file ), 16 * 1024 );
    --- End diff --
   
    Sorry, I mean this in `FileReporter.java`.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] maven-surefire pull request #147: Reporters use buffers to reduce number of ...

grkvlt
In reply to this post by grkvlt
Github user Tibor17 commented on a diff in the pull request:

    https://github.com/apache/maven-surefire/pull/147#discussion_r111644325
 
    --- Diff: maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/report/StatelessXmlReporter.java ---
    @@ -292,15 +293,15 @@ private FileOutputStream getOutputStream( WrappedReportEntry testSetReportEntry
     
             try
             {
    -            return new FileOutputStream( reportFile );
    +            return new BufferedOutputStream( new FileOutputStream( reportFile ), 16 * 1024 );
    --- End diff --
   
    I think we have problem.
    `FileOutputStream outputStream = getOutputStream( testSetReportEntry );`
    `OutputStreamWriter fw = getWriter( outputStream );`
    Both variables are in use which is bad and the only one should be used.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] maven-surefire pull request #147: Reporters use buffers to reduce number of ...

grkvlt
In reply to this post by grkvlt
Github user jerrinot commented on a diff in the pull request:

    https://github.com/apache/maven-surefire/pull/147#discussion_r111679608
 
    --- Diff: maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/report/ConsoleOutputFileReporter.java ---
    @@ -91,7 +93,7 @@ public void writeTestOutput( byte[] buf, int off, int len, boolean stdout )
                         reportsDirectory.mkdirs();
                     }
                     File file = getReportFile( reportsDirectory, reportEntryName, reportNameSuffix, "-output.txt" );
    -                fileOutputStream = new FileOutputStream( file );
    +                fileOutputStream = new BufferedOutputStream( new FileOutputStream( file ), 16 * 1024 );
    --- End diff --
   
    fixed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] maven-surefire pull request #147: Reporters use buffers to reduce number of ...

grkvlt
In reply to this post by grkvlt
Github user jerrinot commented on a diff in the pull request:

    https://github.com/apache/maven-surefire/pull/147#discussion_r111679651
 
    --- Diff: maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/report/StatelessXmlReporter.java ---
    @@ -292,15 +293,15 @@ private FileOutputStream getOutputStream( WrappedReportEntry testSetReportEntry
     
             try
             {
    -            return new FileOutputStream( reportFile );
    +            return new BufferedOutputStream( new FileOutputStream( reportFile ), 16 * 1024 );
    --- End diff --
   
    Is this relevant for this changeset? I agree the reporter should use either directly the stream or the writer. However I see this as a separate problem.
   
    I can have a look at it as well, but it appears rather tricky and I would like to have it isolated from this very change. See this part: https://github.com/jerrinot/maven-surefire/blob/b66b524a42d6cbc2b23225d72b67671d25d3fc05/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/report/StatelessXmlReporter.java#L442-L442


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] maven-surefire issue #147: Reporters use buffers to reduce number of I/O ope...

grkvlt
In reply to this post by grkvlt
Github user jerrinot commented on the issue:

    https://github.com/apache/maven-surefire/pull/147
 
    @Tibor17: thanks a lot for a quick review, I updated the PR.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] maven-surefire issue #147: Reporters use buffers to reduce number of I/O ope...

grkvlt
In reply to this post by grkvlt
Github user Tibor17 commented on the issue:

    https://github.com/apache/maven-surefire/pull/147
 
    I am just discussing with ASF colleagues rewriting `StatelessXmlReporter` to JAXB.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] maven-surefire issue #147: Reporters use buffers to reduce number of I/O ope...

grkvlt
In reply to this post by grkvlt
Github user Tibor17 commented on the issue:

    https://github.com/apache/maven-surefire/pull/147
 
    @jerrinot
    Pls close this PR. I created a branch https://git-wip-us.apache.org/repos/asf?p=maven-surefire.git;a=commit;h=9c0025ed53d70c06279fef6c0fc30a54aeeab27b


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] maven-surefire pull request #147: Reporters use buffers to reduce number of ...

grkvlt
In reply to this post by grkvlt
Github user jerrinot closed the pull request at:

    https://github.com/apache/maven-surefire/pull/147


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] maven-surefire issue #147: Reporters use buffers to reduce number of I/O ope...

grkvlt
In reply to this post by grkvlt
Github user jerrinot commented on the issue:

    https://github.com/apache/maven-surefire/pull/147
 
    @Tibor17: excellent, many thanks for a quick fix! Do you have an expected release date for 2.20.1 ?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] maven-surefire issue #147: Reporters use buffers to reduce number of I/O ope...

grkvlt
In reply to this post by grkvlt
Github user Tibor17 commented on the issue:

    https://github.com/apache/maven-surefire/pull/147
 
    @jerrinot
    Thx for contributing.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] maven-surefire issue #147: Reporters use buffers to reduce number of I/O ope...

grkvlt
In reply to this post by grkvlt
Github user Tibor17 commented on the issue:

    https://github.com/apache/maven-surefire/pull/147
 
    I hope we will release 2.20.1 soon, EO April - BO May.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Loading...