You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@nifi.apache.org by GitBox <gi...@apache.org> on 2022/12/30 14:23:02 UTC
[GitHub] [nifi] dan-s1 opened a new pull request, #6813: [NIFI-11006] Upgraded from Junit4 to Junit5
dan-s1 opened a new pull request, #6813:
URL: https://github.com/apache/nifi/pull/6813
<!-- Licensed to the Apache Software Foundation (ASF) under one or more -->
<!-- contributor license agreements. See the NOTICE file distributed with -->
<!-- this work for additional information regarding copyright ownership. -->
<!-- The ASF licenses this file to You under the Apache License, Version 2.0 -->
<!-- (the "License"); you may not use this file except in compliance with -->
<!-- the License. You may obtain a copy of the License at -->
<!-- http://www.apache.org/licenses/LICENSE-2.0 -->
<!-- Unless required by applicable law or agreed to in writing, software -->
<!-- distributed under the License is distributed on an "AS IS" BASIS, -->
<!-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -->
<!-- See the License for the specific language governing permissions and -->
<!-- limitations under the License. -->
# Summary
[NIFI-11006](https://issues.apache.org/jira/browse/NIFI-11006)
# Tracking
Please complete the following tracking steps prior to pull request creation.
### Issue Tracking
- [ ] [Apache NiFi Jira](https://issues.apache.org/jira/browse/NIFI) issue created
### Pull Request Tracking
- [ ] Pull Request title starts with Apache NiFi Jira issue number, such as `NIFI-00000`
- [ ] Pull Request commit message starts with Apache NiFi Jira issue number, as such `NIFI-00000`
### Pull Request Formatting
- [ ] Pull Request based on current revision of the `main` branch
- [ ] Pull Request refers to a feature branch with one commit containing changes
# Verification
Please indicate the verification steps performed prior to pull request creation.
### Build
- [ ] Build completed using `mvn clean install -P contrib-check`
- [ ] JDK 8
- [ ] JDK 11
- [ ] JDK 17
### Licensing
- [ ] New dependencies are compatible with the [Apache License 2.0](https://apache.org/licenses/LICENSE-2.0) according to the [License Policy](https://www.apache.org/legal/resolved.html)
- [ ] New dependencies are documented in applicable `LICENSE` and `NOTICE` files
### Documentation
- [ ] Documentation formatting appears as expected in rendered files
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] dan-s1 commented on a diff in pull request #6813: [NIFI-11006] Upgraded from Junit4 to Junit5
Posted by GitBox <gi...@apache.org>.
dan-s1 commented on code in PR #6813:
URL: https://github.com/apache/nifi/pull/6813#discussion_r1060774811
##########
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/test/java/org/apache/nifi/web/search/attributematchers/SearchableMatcherTest.java:
##########
@@ -25,14 +25,17 @@
import org.apache.nifi.search.SearchContext;
import org.apache.nifi.search.SearchResult;
import org.apache.nifi.search.Searchable;
-import org.junit.Before;
-import org.junit.Test;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
import org.mockito.Mock;
import org.mockito.Mockito;
+import org.mockito.junit.jupiter.MockitoSettings;
+import org.mockito.quality.Strictness;
import java.util.Collection;
import java.util.HashSet;
+@MockitoSettings(strictness = Strictness.LENIENT)
Review Comment:
@ChrisSamo632 I am not aware of what you are referring to in reference to Mockito functionality. But with the upgrade to JUnit5 the unit tests were failing with an exception like the following where it actually suggests using the lenient option which I did to get these unit cases working.
```
org.mockito.exceptions.misusing.UnnecessaryStubbingException:
Unnecessary stubbings detected.
Clean & maintainable test code requires zero unnecessary code.
...
...
Please remove unnecessary stubbings or use 'lenient' strictness. More info: javadoc for UnnecessaryStubbingException class.
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] dan-s1 commented on a diff in pull request #6813: [NIFI-11006] Upgraded from Junit4 to Junit5
Posted by GitBox <gi...@apache.org>.
dan-s1 commented on code in PR #6813:
URL: https://github.com/apache/nifi/pull/6813#discussion_r1060786499
##########
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/test/java/org/apache/nifi/web/search/attributematchers/SearchableMatcherTest.java:
##########
@@ -25,14 +25,17 @@
import org.apache.nifi.search.SearchContext;
import org.apache.nifi.search.SearchResult;
import org.apache.nifi.search.Searchable;
-import org.junit.Before;
-import org.junit.Test;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
import org.mockito.Mock;
import org.mockito.Mockito;
+import org.mockito.junit.jupiter.MockitoSettings;
+import org.mockito.quality.Strictness;
import java.util.Collection;
import java.util.HashSet;
+@MockitoSettings(strictness = Strictness.LENIENT)
Review Comment:
@exceptionfactory I ended up using `@MockitoSettings(strictness = Strictness.LENIENT)` where there were multiple mocks and it was hard determining which one was causing the exception so I used this more as a "hammer" solution. The ones where I used on `@Mock(lenient = true)` is where one or two mocks were involved so I can look at those again to see if I can remove the unnecessary stubbing. Does the sound reasonable?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] MikeThomsen commented on a diff in pull request #6813: [NIFI-11006] Upgraded from Junit4 to Junit5
Posted by GitBox <gi...@apache.org>.
MikeThomsen commented on code in PR #6813:
URL: https://github.com/apache/nifi/pull/6813#discussion_r1059677168
##########
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/test/java/org/apache/nifi/web/search/attributematchers/SearchableMatcherTest.java:
##########
@@ -25,14 +25,17 @@
import org.apache.nifi.search.SearchContext;
import org.apache.nifi.search.SearchResult;
import org.apache.nifi.search.Searchable;
-import org.junit.Before;
-import org.junit.Test;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
import org.mockito.Mock;
import org.mockito.Mockito;
+import org.mockito.junit.jupiter.MockitoSettings;
+import org.mockito.quality.Strictness;
import java.util.Collection;
import java.util.HashSet;
+@MockitoSettings(strictness = Strictness.LENIENT)
Review Comment:
I wouldn't consider it a blocker for a merge. AFAIK it doesn't actually create real problems aside from runtime bloat which isn't a critical issue for us. It's something that we could put in the backlog as a TODO for the whole code base as a cleanup activity.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] dan-s1 commented on pull request #6813: [NIFI-11006] Upgraded from Junit4 to Junit5
Posted by GitBox <gi...@apache.org>.
dan-s1 commented on PR #6813:
URL: https://github.com/apache/nifi/pull/6813#issuecomment-1370182827
@exceptionfactory I was able to refactor most of the lenient code to avoid the `org.mockito.exceptions.misusing.UnnecessaryStubbingException`
There is one class though which was too complex
`org.apache.nifi.web.controller.ControllerSearchServiceTest` as it has 17 tests and it seems each test has different cause for `org.mockito.exceptions.misusing.UnnecessaryStubbingException`. So I will leave it as is.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] ChrisSamo632 commented on a diff in pull request #6813: [NIFI-11006] Upgraded from Junit4 to Junit5
Posted by GitBox <gi...@apache.org>.
ChrisSamo632 commented on code in PR #6813:
URL: https://github.com/apache/nifi/pull/6813#discussion_r1059619260
##########
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/test/java/org/apache/nifi/web/search/attributematchers/SearchableMatcherTest.java:
##########
@@ -25,14 +25,17 @@
import org.apache.nifi.search.SearchContext;
import org.apache.nifi.search.SearchResult;
import org.apache.nifi.search.Searchable;
-import org.junit.Before;
-import org.junit.Test;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
import org.mockito.Mock;
import org.mockito.Mockito;
+import org.mockito.junit.jupiter.MockitoSettings;
+import org.mockito.quality.Strictness;
import java.util.Collection;
import java.util.HashSet;
+@MockitoSettings(strictness = Strictness.LENIENT)
Review Comment:
There's a lot of `lenient` settings being used with Mockito, is this necessary for all the classes changed in this MR (whether as part of the `@MacktioSettings` annotation on a class or a `@Mock` annotation on a single bean)?
The Mockito default is `strict` rather than `lenient` to try and encourage (what they believe to be) better testing practices, e.g. don't define unnecessary stubs
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] exceptionfactory commented on a diff in pull request #6813: [NIFI-11006] Upgraded from Junit4 to Junit5
Posted by GitBox <gi...@apache.org>.
exceptionfactory commented on code in PR #6813:
URL: https://github.com/apache/nifi/pull/6813#discussion_r1060789464
##########
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/test/java/org/apache/nifi/web/search/attributematchers/SearchableMatcherTest.java:
##########
@@ -25,14 +25,17 @@
import org.apache.nifi.search.SearchContext;
import org.apache.nifi.search.SearchResult;
import org.apache.nifi.search.Searchable;
-import org.junit.Before;
-import org.junit.Test;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
import org.mockito.Mock;
import org.mockito.Mockito;
+import org.mockito.junit.jupiter.MockitoSettings;
+import org.mockito.quality.Strictness;
import java.util.Collection;
import java.util.HashSet;
+@MockitoSettings(strictness = Strictness.LENIENT)
Review Comment:
Thanks, that sounds good.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] exceptionfactory commented on a diff in pull request #6813: [NIFI-11006] Upgraded from Junit4 to Junit5
Posted by GitBox <gi...@apache.org>.
exceptionfactory commented on code in PR #6813:
URL: https://github.com/apache/nifi/pull/6813#discussion_r1060780901
##########
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/test/java/org/apache/nifi/web/search/attributematchers/SearchableMatcherTest.java:
##########
@@ -25,14 +25,17 @@
import org.apache.nifi.search.SearchContext;
import org.apache.nifi.search.SearchResult;
import org.apache.nifi.search.Searchable;
-import org.junit.Before;
-import org.junit.Test;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
import org.mockito.Mock;
import org.mockito.Mockito;
+import org.mockito.junit.jupiter.MockitoSettings;
+import org.mockito.quality.Strictness;
import java.util.Collection;
import java.util.HashSet;
+@MockitoSettings(strictness = Strictness.LENIENT)
Review Comment:
@dan-s1 The ideal solution is to remove the unnecessary stubbing instead of using the lenient mocking mode. If it ends up being too involved, it could be revisited as others noted.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] dan-s1 commented on a diff in pull request #6813: [NIFI-11006] Upgraded from Junit4 to Junit5
Posted by GitBox <gi...@apache.org>.
dan-s1 commented on code in PR #6813:
URL: https://github.com/apache/nifi/pull/6813#discussion_r1060786499
##########
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/test/java/org/apache/nifi/web/search/attributematchers/SearchableMatcherTest.java:
##########
@@ -25,14 +25,17 @@
import org.apache.nifi.search.SearchContext;
import org.apache.nifi.search.SearchResult;
import org.apache.nifi.search.Searchable;
-import org.junit.Before;
-import org.junit.Test;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
import org.mockito.Mock;
import org.mockito.Mockito;
+import org.mockito.junit.jupiter.MockitoSettings;
+import org.mockito.quality.Strictness;
import java.util.Collection;
import java.util.HashSet;
+@MockitoSettings(strictness = Strictness.LENIENT)
Review Comment:
@exceptionfactory I ended up using `@MockitoSettings(strictness = Strictness.LENIENT)` where there were multiple mocks and it was hard determining which one was causing the exception so I used this more as a "hammer" solution. The ones where I used `@Mock(lenient = true)` is where one or two mocks were involved so I can look at those again to see if I can remove the unnecessary stubbing. Does the sound reasonable?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] dan-s1 commented on a diff in pull request #6813: [NIFI-11006] Upgraded from Junit4 to Junit5
Posted by GitBox <gi...@apache.org>.
dan-s1 commented on code in PR #6813:
URL: https://github.com/apache/nifi/pull/6813#discussion_r1060774811
##########
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/test/java/org/apache/nifi/web/search/attributematchers/SearchableMatcherTest.java:
##########
@@ -25,14 +25,17 @@
import org.apache.nifi.search.SearchContext;
import org.apache.nifi.search.SearchResult;
import org.apache.nifi.search.Searchable;
-import org.junit.Before;
-import org.junit.Test;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
import org.mockito.Mock;
import org.mockito.Mockito;
+import org.mockito.junit.jupiter.MockitoSettings;
+import org.mockito.quality.Strictness;
import java.util.Collection;
import java.util.HashSet;
+@MockitoSettings(strictness = Strictness.LENIENT)
Review Comment:
@ChrisSamo632 I am not aware of what you are referring to in reference to Mockito functionality. But with the upgrade to JUnit5 the unit tests were failing with an exception like the following where it actually suggests using the lenient option which I did to get these unit cases working.
```
org.mockito.exceptions.misusing.UnnecessaryStubbingException:
Unnecessary stubbings detected.
Clean & maintainable test code requires zero unnecessary code.
...
...
Please remove unnecessary stubbings or use 'lenient' strictness. More info: javadoc for UnnecessaryStubbingException class.
```
@MikeThomsen Is the scope of this ticket in addition to upgrading to Junit5 to remove unnecessary stubbing or is using lenient sufficient to get these unit tests working with JUnit5?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] ChrisSamo632 commented on a diff in pull request #6813: [NIFI-11006] Upgraded from Junit4 to Junit5
Posted by GitBox <gi...@apache.org>.
ChrisSamo632 commented on code in PR #6813:
URL: https://github.com/apache/nifi/pull/6813#discussion_r1059721468
##########
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/test/java/org/apache/nifi/web/search/attributematchers/SearchableMatcherTest.java:
##########
@@ -25,14 +25,17 @@
import org.apache.nifi.search.SearchContext;
import org.apache.nifi.search.SearchResult;
import org.apache.nifi.search.Searchable;
-import org.junit.Before;
-import org.junit.Test;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
import org.mockito.Mock;
import org.mockito.Mockito;
+import org.mockito.junit.jupiter.MockitoSettings;
+import org.mockito.quality.Strictness;
import java.util.Collection;
import java.util.HashSet;
+@MockitoSettings(strictness = Strictness.LENIENT)
Review Comment:
I agree it's not a blocker. If the lenient setting can simply be removed now then let's do that (has it been applied by default during some automated upgrade routine?)
If it'll be more rework of the tests then fine to raise a tidy-up ticket on the backlog
I only raised it because it looks like a change in mock behaviors during the junit 5 upgrade - Mockito has had strict on by default since version 2 and that shouldn't have changed between junit 4 and 5 runners (that I'm aware of, but I could well have missed such a thing)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] dan-s1 commented on a diff in pull request #6813: [NIFI-11006] Upgraded from Junit4 to Junit5
Posted by GitBox <gi...@apache.org>.
dan-s1 commented on code in PR #6813:
URL: https://github.com/apache/nifi/pull/6813#discussion_r1060774811
##########
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/test/java/org/apache/nifi/web/search/attributematchers/SearchableMatcherTest.java:
##########
@@ -25,14 +25,17 @@
import org.apache.nifi.search.SearchContext;
import org.apache.nifi.search.SearchResult;
import org.apache.nifi.search.Searchable;
-import org.junit.Before;
-import org.junit.Test;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
import org.mockito.Mock;
import org.mockito.Mockito;
+import org.mockito.junit.jupiter.MockitoSettings;
+import org.mockito.quality.Strictness;
import java.util.Collection;
import java.util.HashSet;
+@MockitoSettings(strictness = Strictness.LENIENT)
Review Comment:
@ChrisSamo632 I am not aware of what you are referring to in reference to Mockito functionality. But with the upgrade to JUnit5 in my cases the unit tests were failing with an exception like the following where it actually suggests using the lenient option which I did to get these unit cases working.
```
org.mockito.exceptions.misusing.UnnecessaryStubbingException:
Unnecessary stubbings detected.
Clean & maintainable test code requires zero unnecessary code.
...
...
Please remove unnecessary stubbings or use 'lenient' strictness. More info: javadoc for UnnecessaryStubbingException class.
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] asfgit closed pull request #6813: [NIFI-11006] Upgraded from Junit4 to Junit5
Posted by GitBox <gi...@apache.org>.
asfgit closed pull request #6813: [NIFI-11006] Upgraded from Junit4 to Junit5
URL: https://github.com/apache/nifi/pull/6813
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org