[GitHub] nifi issue #2780: NIFI-5289 - Changed nifi-mock junit Dependency to Compile ...

2018-07-10 Thread mattyb149
Github user mattyb149 commented on the issue:

https://github.com/apache/nifi/pull/2780
  
+1 LGTM (thanks for the reviews and comments!), I ran a full build and 
verified that no junit JARs are present anywhere in the NiFi assembly. Thanks 
for the improvement! Merging to master


---


[GitHub] nifi issue #2780: NIFI-5289 - Changed nifi-mock junit Dependency to Compile ...

2018-07-09 Thread joewitt
Github user joewitt commented on the issue:

https://github.com/apache/nifi/pull/2780
  
it should be fine for junit dep to be compile scope for nifi-mock.  
nifi-mock will always be used under test scope so the transitive compile scoped 
deps will be ok.


---


[GitHub] nifi issue #2780: NIFI-5289 - Changed nifi-mock junit Dependency to Compile ...

2018-07-08 Thread MartinPayne
Github user MartinPayne commented on the issue:

https://github.com/apache/nifi/pull/2780
  
> The global declaration for JUnit is JUnit 4.12. If someone is using v5, 
they'll have to work around that.

Yes, we're using a workaround at the moment. We have to declare JUnit 4 as 
a dependency with "test" scope, and configure the Maven Dependency Plugin to 
force JUnit 4 as used to prevent it from being reported as a declared and 
unused dependency. I would prefer to have the fix upstream so that we can 
remove the workaround.

> nifi-mock is designed to support JUnit tests and I'm not aware of any 
components that use JUnit 5.

I am using JUnit 5, and I imagine over time many other people will be 
migrating to it too. Code wise there is nothing in nifi-mock which means it 
shouldn't work with JUnit 5, and I would expect it to work with other test 
frameworks too.

I've provided a link to a table of Maven dependency scopes to demonstrate 
that this won't result in JUnit ending up in non-test code. Are there any 
further concerns about this fix?


---


[GitHub] nifi issue #2780: NIFI-5289 - Changed nifi-mock junit Dependency to Compile ...

2018-07-08 Thread MikeThomsen
Github user MikeThomsen commented on the issue:

https://github.com/apache/nifi/pull/2780
  
> (it won't be on the test classpath if you're using JUnit 5)

The global declaration for JUnit is JUnit 4.12. If someone

> It should be "compile" scope, because nifi-mock is making use of JUnit 
assertions in non-test code.

nifi-mock is designed to support JUnit tests and I'm not aware of any 
components that use JUnit 5.


---


[GitHub] nifi issue #2780: NIFI-5289 - Changed nifi-mock junit Dependency to Compile ...

2018-07-08 Thread MartinPayne
Github user MartinPayne commented on the issue:

https://github.com/apache/nifi/pull/2780
  
@MikeThomsen I'm not too sure what you mean. At the moment you'll get a 
NoClassDefFoundError unless JUnit 4 happens to be on the test classpath (it 
won't be on the test classpath if you're using JUnit 5). That's because 
nifi-mock declares JUnit 4 with a "provided" scope, when there is nothing 
providing JUnit 4. It should be "compile" scope, because nifi-mock is making 
use of JUnit assertions in non-test code.


---


[GitHub] nifi issue #2780: NIFI-5289 - Changed nifi-mock junit Dependency to Compile ...

2018-06-11 Thread MartinPayne
Github user MartinPayne commented on the issue:

https://github.com/apache/nifi/pull/2780
  
@joewitt Compile scope is the correct scope in this case. If JUnit 4 was 
only used in the tests for NiFi Mock, test scope would be correct. However, the 
[NiFi Mock code uses JUnit 4 as a compile time 
dependency](https://github.com/apache/nifi/blob/f8466cb16d6723ddc3bf5f0e7f8ce8a47d27cbe5/nifi-mock/src/main/java/org/apache/nifi/util/StandardProcessorTestRunner.java#L74),
 so JUnit 4 needs to be brought into consuming projects as a transitive 
dependency.

As per the [Maven dependency scope 
table](https://maven.apache.org/guides/introduction/introduction-to-dependency-mechanism.html#Dependency_Scope),
 making it a compile scope dependency means it is added to the test classpath 
of a consuming project if that project declares NiFi Mock with test scope. It 
would only get included all over the place if consuming projects have declared 
NiFi Mock with a compile or runtime scope. If that was the case here, NiFi Mock 
would also be getting included all over the place.


---


[GitHub] nifi issue #2780: NIFI-5289 - Changed nifi-mock junit Dependency to Compile ...

2018-06-11 Thread joewitt
Github user joewitt commented on the issue:

https://github.com/apache/nifi/pull/2780
  
we need to ensure this change doesnt make this lib get included all over 
the place as part of the build.  I dont understand why it should not have been 
test but compile seems wrong..


---