[GitHub] maven-surefire pull request #127: [SUREFIRE-1293] Simplify org.apache.maven....

2016-10-12 Thread britter 
Github user britter closed the pull request at:

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


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@maven.apache.org
For additional commands, e-mail: dev-h...@maven.apache.org

[GitHub] maven-surefire pull request #127: [SUREFIRE-1293] Simplify org.apache.maven....

2016-10-10 Thread Tibor17 
Github user Tibor17 commented on a diff in the pull request:

https://github.com/apache/maven-surefire/pull/127#discussion_r82700832
  
--- Diff: 
maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/report/TestSetRunListener.java
 ---
@@ -76,7 +76,7 @@ public TestSetRunListener( ConsoleReporter 
consoleReporter, FileReporter fileRep
StatisticsReporter statisticsReporter, 
boolean trimStackTrace,
boolean isPlainFormat, boolean 
briefOrPlainFormat )
 {
-this.consoleReporter = consoleReporter;
+this.consoleReporter = consoleReporter != null ? consoleReporter : 
new NullConsoleReporter();
--- End diff --

@britter 
>>does this belong into DefaultReporterFactory.createReporter()
If you mean this code:
`ConsoleReporter consoleReporter = shouldReportToConsole() ? new 
ConsoleReporter( consoleLogger ) : null;`

Then I would keep the responsibility of creating anew reporter yet in the 
method `createReporter()`. I am going to fix a bug exactly in this part of code 
after you. We can still improve the code by small incremental changes.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@maven.apache.org
For additional commands, e-mail: dev-h...@maven.apache.org

[GitHub] maven-surefire pull request #127: [SUREFIRE-1293] Simplify org.apache.maven....

2016-10-10 Thread Tibor17 
Github user Tibor17 commented on a diff in the pull request:

https://github.com/apache/maven-surefire/pull/127#discussion_r82699845
  
--- Diff: 
maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/report/TestSetRunListener.java
 ---
@@ -76,7 +76,7 @@ public TestSetRunListener( ConsoleReporter 
consoleReporter, FileReporter fileRep
StatisticsReporter statisticsReporter, 
boolean trimStackTrace,
boolean isPlainFormat, boolean 
briefOrPlainFormat )
 {
-this.consoleReporter = consoleReporter;
+this.consoleReporter = consoleReporter != null ? consoleReporter : 
new NullConsoleReporter();
--- End diff --

no no, I meant only the scope of this PR. Not whole Surefire of course.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@maven.apache.org
For additional commands, e-mail: dev-h...@maven.apache.org

[GitHub] maven-surefire pull request #127: [SUREFIRE-1293] Simplify org.apache.maven....

2016-10-10 Thread Tibor17 
Github user Tibor17 commented on a diff in the pull request:

https://github.com/apache/maven-surefire/pull/127#discussion_r82699554
  
--- Diff: 
maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/report/NullConsoleReporter.java
 ---
@@ -33,7 +33,10 @@
 class NullConsoleReporter
 extends ConsoleReporter
 {
-NullConsoleReporter() {
+
+static final NullConsoleReporter INSTANCE = new NullConsoleReporter();
+
+private NullConsoleReporter() {
--- End diff --

here as well
pls run the build. it explores checkstyle erros.
Did you get your IntelliJ Idea now working with our Maven checkstyle?


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@maven.apache.org
For additional commands, e-mail: dev-h...@maven.apache.org

[GitHub] maven-surefire pull request #127: [SUREFIRE-1293] Simplify org.apache.maven....

2016-10-10 Thread Tibor17 
Github user Tibor17 commented on a diff in the pull request:

https://github.com/apache/maven-surefire/pull/127#discussion_r82699403
  
--- Diff: 
maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/report/NullConsoleOutputReceiver.java
 ---
@@ -31,7 +31,13 @@
 implements TestcycleConsoleOutputReceiver
 {
 
-public void testSetStarting( ReportEntry reportEntry )
+static final NullConsoleOutputReceiver INSTANCE = new 
NullConsoleOutputReceiver();
+
+private NullConsoleOutputReceiver()
+{
+}
+
+public void testSetStarting(ReportEntry reportEntry )
--- End diff --

here is checkstyle error


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@maven.apache.org
For additional commands, e-mail: dev-h...@maven.apache.org

[GitHub] maven-surefire pull request #127: [SUREFIRE-1293] Simplify org.apache.maven....

2016-10-10 Thread britter 
Github user britter commented on a diff in the pull request:

https://github.com/apache/maven-surefire/pull/127#discussion_r82631034
  
--- Diff: 
maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/report/NullStatisticsReporter.java
 ---
@@ -35,7 +36,7 @@
 
 public NullStatisticsReporter()
 {
-super( FileUtils.getTempDirectory() );
+super( FileUtils.getFile( FileUtils.getTempDirectory(), 
RandomStringUtils.randomAlphabetic( 24 ) ) );
--- End diff --

@Tibor17 makes sense. If we change it like this, we don't need the test to 
make sure the null objects can be created. I'll modify the PR to incorporate 
protected default constructors and remove the empty tests.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@maven.apache.org
For additional commands, e-mail: dev-h...@maven.apache.org

[GitHub] maven-surefire pull request #127: [SUREFIRE-1293] Simplify org.apache.maven....

2016-10-10 Thread britter 
Github user britter commented on a diff in the pull request:

https://github.com/apache/maven-surefire/pull/127#discussion_r82631691
  
--- Diff: 
maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/report/TestSetRunListener.java
 ---
@@ -76,7 +76,7 @@ public TestSetRunListener( ConsoleReporter 
consoleReporter, FileReporter fileRep
StatisticsReporter statisticsReporter, 
boolean trimStackTrace,
boolean isPlainFormat, boolean 
briefOrPlainFormat )
 {
-this.consoleReporter = consoleReporter;
+this.consoleReporter = consoleReporter != null ? consoleReporter : 
new NullConsoleReporter();
--- End diff --

> All of these classes are in surefire-common project. Therefore it would 
be nice not to create a new object new NullConsoleReporter() but instead keep 
it as static final constant in abstract Factory in surefire-common.

Agreed, I can change this.

> I would say that the caller should pass non-null consistent object into 
this class TestSetRunListener via constructor

Yes it would certainly be better to enforce this kinds of invariants. We 
can do it like this, but then the constructor of `TestSetRunListener` has to 
check for null parameters and throw an exception if null is passed.

> If we accepted such paradigm then all such occurrences in this PR would 
have to change it.

I don't understand. Change the whole surefire code base for all occurrences 
of this pattern?


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@maven.apache.org
For additional commands, e-mail: dev-h...@maven.apache.org

[GitHub] maven-surefire pull request #127: [SUREFIRE-1293] Simplify org.apache.maven....

2016-10-09 Thread Tibor17 
Github user Tibor17 commented on a diff in the pull request:

https://github.com/apache/maven-surefire/pull/127#discussion_r82527079
  
--- Diff: 
maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/report/TestSetRunListener.java
 ---
@@ -76,7 +76,7 @@ public TestSetRunListener( ConsoleReporter 
consoleReporter, FileReporter fileRep
StatisticsReporter statisticsReporter, 
boolean trimStackTrace,
boolean isPlainFormat, boolean 
briefOrPlainFormat )
 {
-this.consoleReporter = consoleReporter;
+this.consoleReporter = consoleReporter != null ? consoleReporter : 
new NullConsoleReporter();
--- End diff --

All of these classes are in `surefire-common` project. Therefor it would be 
nice not to create a new object 
`new NullConsoleReporter()` but instead keep it as static final constant in 
abstract Factory in surefire-common.
This would mean that such null-check may be embedded in the factory, e.g. 
`buildConsoleReporter()`. I would say that the caller should pass non-null 
consistent object into this class `TestSetRunListener` via constructor and the 
called class should not make any guess about it (means if == null => guess hide 
the error). If we accepted such paradigm then all such occurrences in this PR 
would have to change it.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@maven.apache.org
For additional commands, e-mail: dev-h...@maven.apache.org

[GitHub] maven-surefire pull request #127: [SUREFIRE 1293] Simplify org.apache.maven....

2016-10-09 Thread britter 
GitHub user britter opened a pull request:

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

[SUREFIRE 1293] Simplify 
org.apache.maven.plugin.surefire.report.TestSetRunListener by using the null 
object pattern

Added null object implementations for fields used by TestSetRunListener in 
order to simplify TestSetRunListener code.

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

$ git pull https://github.com/britter/maven-surefire SUREFIRE-1293

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

https://github.com/apache/maven-surefire/pull/127.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 #127


commit fa5f480fbe02dbd401b98ca39b56a9a0db930f48
Author: Benedikt Ritter 
Date:   2016-10-09T14:41:49Z

Introduce NullConsoleReporter to simplify TesSetRunListener

commit a811fb618b4382feff6ee991d253c816f62e7d94
Author: Benedikt Ritter 
Date:   2016-10-09T14:47:31Z

Introduce NullFileReporter to simplify TesSetRunListener

commit d419b7be567546fe615bdcd770fe03229c141ca2
Author: Benedikt Ritter 
Date:   2016-10-09T14:53:34Z

Introduce NullStatisticsReporter to simplify TesSetRunListener

commit e56269cfce8b626e0e2667982891068251beedf6
Author: Benedikt Ritter 
Date:   2016-10-09T14:57:16Z

Introduce NullStatelessXmlReporter to simplify TesSetRunListener

commit 42dd834e098c3bbf8f103db52fb3edc125cd7276
Author: Benedikt Ritter 
Date:   2016-10-09T15:00:39Z

Introduce NullConsoleOutputReceiver to simplify TesSetRunListener

commit cbf8e0948a76f82e3ef7c124e21b2edb9b96f636
Author: Benedikt Ritter 
Date:   2016-10-09T15:07:38Z

Make sure all the Null object classes can be instantiated




---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@maven.apache.org
For additional commands, e-mail: dev-h...@maven.apache.org