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/09 20:35:14 UTC
[GitHub] [nifi] dan-s1 opened a new pull request, #6775: [NIFI-10877] Conversion of nifi-framework-core tests from Junit4 to J…
dan-s1 opened a new pull request, #6775:
URL: https://github.com/apache/nifi/pull/6775
…unit5
<!-- 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-00000](https://issues.apache.org/jira/browse/NIFI-00000)
# 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 pull request #6775: [NIFI-10877] Conversion of nifi-framework-core tests from Junit4 to J…
Posted by GitBox <gi...@apache.org>.
dan-s1 commented on PR #6775:
URL: https://github.com/apache/nifi/pull/6775#issuecomment-1348862970
@exceptionfactory I took care of those. Let me know if there is anything else. Thank you for pointing out the the Checkstyle issues as from the CI-workflow it did not give any information other than error.
--
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 #6775: [NIFI-10877] Conversion of nifi-framework-core tests from Junit4 to J…
Posted by GitBox <gi...@apache.org>.
dan-s1 commented on code in PR #6775:
URL: https://github.com/apache/nifi/pull/6775#discussion_r1046502289
##########
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/test/groovy/org/apache/nifi/controller/repository/crypto/EncryptedFileSystemRepositoryTest.groovy:
##########
@@ -64,9 +61,9 @@ class EncryptedFileSystemRepositoryTest {
(NiFiProperties.CONTENT_REPOSITORY_ENCRYPTION_KEY_PROVIDER_LOCATION) : ""
]
- @BeforeClass
+ @BeforeAll
static void setUpOnce() throws Exception {
- Assume.assumeTrue("Test only runs on *nix", !SystemUtils.IS_OS_WINDOWS)
+ Assumptions.assumeTrue(!SystemUtils.IS_OS_WINDOWS, "Test only runs on *nix")
Review Comment:
This may be the cause [Why does JUnit run tests twice with different results](https://stackoverflow.com/questions/50751861/why-does-junit-run-tests-twice-with-different-resuls)
I am not sure what to do as the GroovyTestCase extends TestCase
--
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 #6775: [NIFI-10877] Conversion of nifi-framework-core tests from Junit4 to J…
Posted by GitBox <gi...@apache.org>.
dan-s1 commented on code in PR #6775:
URL: https://github.com/apache/nifi/pull/6775#discussion_r1046356567
##########
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/test/groovy/org/apache/nifi/controller/repository/crypto/EncryptedFileSystemRepositoryTest.groovy:
##########
@@ -64,9 +61,9 @@ class EncryptedFileSystemRepositoryTest {
(NiFiProperties.CONTENT_REPOSITORY_ENCRYPTION_KEY_PROVIDER_LOCATION) : ""
]
- @BeforeClass
+ @BeforeAll
static void setUpOnce() throws Exception {
- Assume.assumeTrue("Test only runs on *nix", !SystemUtils.IS_OS_WINDOWS)
+ Assumptions.assumeTrue(!SystemUtils.IS_OS_WINDOWS, "Test only runs on *nix")
Review Comment:
@exceptionfactory I made the changes to this class and TestFileSystemRepository. I noticed that EncryptedSequentialAccessWriteAheadLogTest has this also but it has not been converted over to Junit 5. Do you mind if make the changes for JUnit 5 on this class also?
--
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 #6775: [NIFI-10877] Conversion of nifi-framework-core tests from Junit4 to J…
Posted by GitBox <gi...@apache.org>.
dan-s1 commented on code in PR #6775:
URL: https://github.com/apache/nifi/pull/6775#discussion_r1046532506
##########
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/test/groovy/org/apache/nifi/controller/repository/crypto/EncryptedFileSystemRepositoryTest.groovy:
##########
@@ -64,9 +61,9 @@ class EncryptedFileSystemRepositoryTest {
(NiFiProperties.CONTENT_REPOSITORY_ENCRYPTION_KEY_PROVIDER_LOCATION) : ""
]
- @BeforeClass
+ @BeforeAll
static void setUpOnce() throws Exception {
- Assume.assumeTrue("Test only runs on *nix", !SystemUtils.IS_OS_WINDOWS)
+ Assumptions.assumeTrue(!SystemUtils.IS_OS_WINDOWS, "Test only runs on *nix")
Review Comment:
@exceptionfactory That was it. Thanks! I just had to import all the Assertions in assertions to cover the ones defined in GroovyTestCase
--
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 pull request #6775: [NIFI-10877] Conversion of nifi-framework-core tests from Junit4 to J…
Posted by GitBox <gi...@apache.org>.
exceptionfactory commented on PR #6775:
URL: https://github.com/apache/nifi/pull/6775#issuecomment-1349491017
> I had tried to expand last night and nothing expanded and the only thing I saw was error. Today I see the logs. Is there a delay of some sort for that to be populated?
The logs are available immediately, but you have to be logged in to GitHub in order to view the logs.
--
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 #6775: [NIFI-10877] Conversion of nifi-framework-core tests from Junit4 to J…
Posted by GitBox <gi...@apache.org>.
exceptionfactory commented on code in PR #6775:
URL: https://github.com/apache/nifi/pull/6775#discussion_r1046493258
##########
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/test/groovy/org/apache/nifi/controller/repository/crypto/EncryptedFileSystemRepositoryTest.groovy:
##########
@@ -64,9 +61,9 @@ class EncryptedFileSystemRepositoryTest {
(NiFiProperties.CONTENT_REPOSITORY_ENCRYPTION_KEY_PROVIDER_LOCATION) : ""
]
- @BeforeClass
+ @BeforeAll
static void setUpOnce() throws Exception {
- Assume.assumeTrue("Test only runs on *nix", !SystemUtils.IS_OS_WINDOWS)
+ Assumptions.assumeTrue(!SystemUtils.IS_OS_WINDOWS, "Test only runs on *nix")
Review Comment:
From the stack trace, it appears to be using an older JUnit Runner. Make sure to remove the `extends GroovyTestCase` from the class as part of the changes.
--
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 #6775: [NIFI-10877] Conversion of nifi-framework-core tests from Junit4 to J…
Posted by GitBox <gi...@apache.org>.
dan-s1 commented on PR #6775:
URL: https://github.com/apache/nifi/pull/6775#issuecomment-1346580500
@exceptionfactory Done
--
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 #6775: [NIFI-10877] Conversion of nifi-framework-core tests from Junit4 to J…
Posted by GitBox <gi...@apache.org>.
exceptionfactory commented on code in PR #6775:
URL: https://github.com/apache/nifi/pull/6775#discussion_r1046366136
##########
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/test/java/org/apache/nifi/controller/StandardFlowServiceTest.java:
##########
@@ -111,13 +112,16 @@ public void testLoadWithFlow() throws IOException {
String expectedFlow = new String(flowBytes).trim();
String actualFlow = new String(baos.toByteArray()).trim();
- Assert.assertEquals(expectedFlow, actualFlow);
+ Assertions.assertEquals(expectedFlow, actualFlow);
Review Comment:
Thanks for the note, I missed that detail on the review. In light of that, the replacement approach currently implemented looks good for now.
--
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 #6775: [NIFI-10877] Conversion of nifi-framework-core tests from Junit4 to J…
Posted by GitBox <gi...@apache.org>.
dan-s1 commented on code in PR #6775:
URL: https://github.com/apache/nifi/pull/6775#discussion_r1046364030
##########
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/test/java/org/apache/nifi/controller/StandardFlowServiceTest.java:
##########
@@ -111,13 +112,16 @@ public void testLoadWithFlow() throws IOException {
String expectedFlow = new String(flowBytes).trim();
String actualFlow = new String(baos.toByteArray()).trim();
- Assert.assertEquals(expectedFlow, actualFlow);
+ Assertions.assertEquals(expectedFlow, actualFlow);
Review Comment:
I can't do that as there are overloaded assertEqual methods in that 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 #6775: [NIFI-10877] Conversion of nifi-framework-core tests from Junit4 to J…
Posted by GitBox <gi...@apache.org>.
dan-s1 commented on code in PR #6775:
URL: https://github.com/apache/nifi/pull/6775#discussion_r1046502289
##########
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/test/groovy/org/apache/nifi/controller/repository/crypto/EncryptedFileSystemRepositoryTest.groovy:
##########
@@ -64,9 +61,9 @@ class EncryptedFileSystemRepositoryTest {
(NiFiProperties.CONTENT_REPOSITORY_ENCRYPTION_KEY_PROVIDER_LOCATION) : ""
]
- @BeforeClass
+ @BeforeAll
static void setUpOnce() throws Exception {
- Assume.assumeTrue("Test only runs on *nix", !SystemUtils.IS_OS_WINDOWS)
+ Assumptions.assumeTrue(!SystemUtils.IS_OS_WINDOWS, "Test only runs on *nix")
Review Comment:
This may be the cause [Why does JUnit run tests twice with different results](https://stackoverflow.com/questions/50751861/why-does-junit-run-tests-twice-with-different-resuls)
I am not sure what to do as the GroovyTestCase extends TestCase
--
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 #6775: [NIFI-10877] Conversion of nifi-framework-core tests from Junit4 to J…
Posted by GitBox <gi...@apache.org>.
dan-s1 commented on code in PR #6775:
URL: https://github.com/apache/nifi/pull/6775#discussion_r1046502289
##########
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/test/groovy/org/apache/nifi/controller/repository/crypto/EncryptedFileSystemRepositoryTest.groovy:
##########
@@ -64,9 +61,9 @@ class EncryptedFileSystemRepositoryTest {
(NiFiProperties.CONTENT_REPOSITORY_ENCRYPTION_KEY_PROVIDER_LOCATION) : ""
]
- @BeforeClass
+ @BeforeAll
static void setUpOnce() throws Exception {
- Assume.assumeTrue("Test only runs on *nix", !SystemUtils.IS_OS_WINDOWS)
+ Assumptions.assumeTrue(!SystemUtils.IS_OS_WINDOWS, "Test only runs on *nix")
Review Comment:
This may be the cause [Why does JUnit run tests twice with different results](https://stacko(https://stackoverflow.com/questions/50751861/why-does-junit-run-tests-twice-with-different-resuls)
--
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 #6775: [NIFI-10877] Conversion of nifi-framework-core tests from Junit4 to J…
Posted by GitBox <gi...@apache.org>.
dan-s1 commented on code in PR #6775:
URL: https://github.com/apache/nifi/pull/6775#discussion_r1046487866
##########
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/test/groovy/org/apache/nifi/controller/repository/crypto/EncryptedFileSystemRepositoryTest.groovy:
##########
@@ -64,9 +61,9 @@ class EncryptedFileSystemRepositoryTest {
(NiFiProperties.CONTENT_REPOSITORY_ENCRYPTION_KEY_PROVIDER_LOCATION) : ""
]
- @BeforeClass
+ @BeforeAll
static void setUpOnce() throws Exception {
- Assume.assumeTrue("Test only runs on *nix", !SystemUtils.IS_OS_WINDOWS)
+ Assumptions.assumeTrue(!SystemUtils.IS_OS_WINDOWS, "Test only runs on *nix")
Review Comment:
So I made the changes to EncryptedSequentialAccessWriteAheadLogTest but I am not sure what to do as the 2 tests are run twice (I am not sure why). The first time they both pass and the second time they both fail. This happens in Intellij and with Maven. I am attaching the stacktrace from running it in Maven.
[EncryptedSequentialAccessWriteAheadLogTest_stacktrace.txt](https://github.com/apache/nifi/files/10212804/EncryptedSequentialAccessWriteAheadLogTest_stacktrace.txt)
--
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 #6775: [NIFI-10877] Conversion of nifi-framework-core tests from Junit4 to J…
Posted by GitBox <gi...@apache.org>.
exceptionfactory commented on code in PR #6775:
URL: https://github.com/apache/nifi/pull/6775#discussion_r1046364814
##########
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/test/groovy/org/apache/nifi/controller/repository/crypto/EncryptedFileSystemRepositoryTest.groovy:
##########
@@ -64,9 +61,9 @@ class EncryptedFileSystemRepositoryTest {
(NiFiProperties.CONTENT_REPOSITORY_ENCRYPTION_KEY_PROVIDER_LOCATION) : ""
]
- @BeforeClass
+ @BeforeAll
static void setUpOnce() throws Exception {
- Assume.assumeTrue("Test only runs on *nix", !SystemUtils.IS_OS_WINDOWS)
+ Assumptions.assumeTrue(!SystemUtils.IS_OS_WINDOWS, "Test only runs on *nix")
Review Comment:
Sure, feel free to make the additional changes.
--
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 pull request #6775: [NIFI-10877] Conversion of nifi-framework-core tests from Junit4 to J…
Posted by GitBox <gi...@apache.org>.
exceptionfactory commented on PR #6775:
URL: https://github.com/apache/nifi/pull/6775#issuecomment-1349006249
> @exceptionfactory I took care of those. Let me know if there is anything else. Thank you for pointing out the the Checkstyle issues as from the CI-workflow it did not give any information other than error.
You're welcome, thanks for addressing the issue. The output from the [Static Analysis](https://github.com/apache/nifi/actions/runs/3686971055/jobs/6239929188) job has a `Maven Build` section that can be expanded to show the particular error. This can also be verified in a local build using `mvn validate -P contrib-check`.
--
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 pull request #6775: [NIFI-10877] Conversion of nifi-framework-core tests from Junit4 to J…
Posted by GitBox <gi...@apache.org>.
exceptionfactory commented on PR #6775:
URL: https://github.com/apache/nifi/pull/6775#issuecomment-1347129068
Thanks for resolving the conflicts @dan-s1. I had made some of the related changes under NIFI-10941 as part of removing TestNG references, I will review the changes in this PR.
--
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 #6775: [NIFI-10877] Conversion of nifi-framework-core tests from Junit4 to J…
Posted by GitBox <gi...@apache.org>.
exceptionfactory commented on code in PR #6775:
URL: https://github.com/apache/nifi/pull/6775#discussion_r1046647523
##########
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/test/groovy/org/apache/nifi/wali/EncryptedSequentialAccessWriteAheadLogTest.groovy:
##########
@@ -17,30 +17,26 @@
package org.apache.nifi.wali
-
-import org.apache.commons.lang3.SystemUtils
import org.apache.nifi.controller.queue.FlowFileQueue
import org.apache.nifi.controller.repository.*
import org.apache.nifi.controller.repository.claim.ResourceClaimManager
import org.apache.nifi.controller.repository.claim.StandardResourceClaimManager
import org.apache.nifi.repository.schema.NoOpFieldCache
import org.apache.nifi.security.kms.StaticKeyProvider
import org.apache.nifi.util.NiFiProperties
-import org.bouncycastle.jce.provider.BouncyCastleProvider
-import org.junit.*
-import org.junit.rules.TestName
-import org.junit.runner.RunWith
-import org.junit.runners.JUnit4
+import org.junit.jupiter.api.*
Review Comment:
This should be changed to use explicit imports.
##########
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/test/groovy/org/apache/nifi/wali/EncryptedSequentialAccessWriteAheadLogTest.groovy:
##########
@@ -17,30 +17,26 @@
package org.apache.nifi.wali
-
-import org.apache.commons.lang3.SystemUtils
import org.apache.nifi.controller.queue.FlowFileQueue
import org.apache.nifi.controller.repository.*
import org.apache.nifi.controller.repository.claim.ResourceClaimManager
import org.apache.nifi.controller.repository.claim.StandardResourceClaimManager
import org.apache.nifi.repository.schema.NoOpFieldCache
import org.apache.nifi.security.kms.StaticKeyProvider
import org.apache.nifi.util.NiFiProperties
-import org.bouncycastle.jce.provider.BouncyCastleProvider
-import org.junit.*
-import org.junit.rules.TestName
-import org.junit.runner.RunWith
-import org.junit.runners.JUnit4
+import org.junit.jupiter.api.*
+import org.junit.jupiter.api.condition.DisabledOnOs
+import org.junit.jupiter.api.condition.OS
import org.slf4j.Logger
import org.slf4j.LoggerFactory
import org.wali.SerDe
import org.wali.SerDeFactory
import org.wali.SingletonSerDeFactory
-import java.security.Security
+import static org.junit.jupiter.api.Assertions.*
Review Comment:
This should be changed to explicit imports
##########
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/test/java/org/apache/nifi/controller/TestStandardFlowFileQueue.java:
##########
@@ -37,10 +37,10 @@
import org.apache.nifi.provenance.ProvenanceEventRepository;
import org.apache.nifi.provenance.ProvenanceEventType;
import org.apache.nifi.provenance.StandardProvenanceEventRecord;
-import org.junit.Assert;
-import org.junit.Before;
-import org.junit.BeforeClass;
-import org.junit.Test;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.BeforeAll;
Review Comment:
This import needs to be removed.
--
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 #6775: [NIFI-10877] Conversion of nifi-framework-core tests from Junit4 to J…
Posted by GitBox <gi...@apache.org>.
dan-s1 commented on PR #6775:
URL: https://github.com/apache/nifi/pull/6775#issuecomment-1349308626
> > @exceptionfactory I took care of those. Let me know if there is anything else. Thank you for pointing out the the Checkstyle issues as from the CI-workflow it did not give any information other than error.
>
> You're welcome, thanks for addressing the issue. The output from the [Static Analysis](https://github.com/apache/nifi/actions/runs/3686971055/jobs/6239929188) job has a `Maven Build` section that can be expanded to show the particular error. This can also be verified in a local build using `mvn validate -P contrib-check`.
I had tried to expand last night and nothing expanded and the only thing I saw was error. Today I see the logs. Is there a delay of some sort for that to be populated?
--
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 #6775: [NIFI-10877] Conversion of nifi-framework-core tests from Junit4 to J…
Posted by GitBox <gi...@apache.org>.
dan-s1 commented on code in PR #6775:
URL: https://github.com/apache/nifi/pull/6775#discussion_r1046502289
##########
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/test/groovy/org/apache/nifi/controller/repository/crypto/EncryptedFileSystemRepositoryTest.groovy:
##########
@@ -64,9 +61,9 @@ class EncryptedFileSystemRepositoryTest {
(NiFiProperties.CONTENT_REPOSITORY_ENCRYPTION_KEY_PROVIDER_LOCATION) : ""
]
- @BeforeClass
+ @BeforeAll
static void setUpOnce() throws Exception {
- Assume.assumeTrue("Test only runs on *nix", !SystemUtils.IS_OS_WINDOWS)
+ Assumptions.assumeTrue(!SystemUtils.IS_OS_WINDOWS, "Test only runs on *nix")
Review Comment:
This may be the cause [Why does JUnit run tests twice with different results](https://stackoverflow.com/questions/50751861/why-does-junit-run-tests-twice-with-different-resuls)
--
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 closed pull request #6775: [NIFI-10877] Conversion of nifi-framework-core tests from Junit4 to J…
Posted by GitBox <gi...@apache.org>.
exceptionfactory closed pull request #6775: [NIFI-10877] Conversion of nifi-framework-core tests from Junit4 to J…
URL: https://github.com/apache/nifi/pull/6775
--
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 #6775: [NIFI-10877] Conversion of nifi-framework-core tests from Junit4 to J…
Posted by GitBox <gi...@apache.org>.
exceptionfactory commented on code in PR #6775:
URL: https://github.com/apache/nifi/pull/6775#discussion_r1046275711
##########
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/test/java/org/apache/nifi/controller/StandardProcessorNodeIT.java:
##########
@@ -206,9 +204,9 @@ public void testSinglePropertyDynamicallyModifiesClasspath() throws MalformedURL
}
@Test
- public void testNativeLibLoadedFromDynamicallyModifiesClasspathProperty() throws Exception {
+ public void testNativeLibLoadedFromDynamicallyModifiesClasspathProperty() {
// GIVEN
- assumeTrue("Test only runs on Mac OS", new OSUtil(){}.isOsMac());
+ assumeTrue(new OSUtil(){}.isOsMac(), "Test only runs on Mac OS");
Review Comment:
This approach should be replaced with the `EnabledOnOs` annotation
##########
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/test/java/org/apache/nifi/controller/StandardFlowServiceTest.java:
##########
@@ -111,13 +112,16 @@ public void testLoadWithFlow() throws IOException {
String expectedFlow = new String(flowBytes).trim();
String actualFlow = new String(baos.toByteArray()).trim();
- Assert.assertEquals(expectedFlow, actualFlow);
+ Assertions.assertEquals(expectedFlow, actualFlow);
Review Comment:
This is a good opportunity to replace `Assertions.assertEquals` with the static import of `assertEquals`:
```suggestion
assertEquals(expectedFlow, actualFlow);
```
##########
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/test/java/org/apache/nifi/controller/repository/TestFileSystemRepository.java:
##########
@@ -75,12 +74,12 @@ public class TestFileSystemRepository {
private final File rootFile = new File("target/content_repository");
private NiFiProperties nifiProperties;
- @BeforeClass
+ @BeforeAll
public static void setupClass() {
- Assume.assumeTrue("Test only runs on *nix", !SystemUtils.IS_OS_WINDOWS);
+ assumeTrue(!SystemUtils.IS_OS_WINDOWS, "Test only runs on *nix");
Review Comment:
Replace with `DisabledOnOs(OS.WINDOWS)` at the class level.
##########
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/test/java/org/apache/nifi/controller/scheduling/TestStandardProcessScheduler.java:
##########
@@ -390,16 +380,13 @@ public void validateDisabledServiceCantBeDisabled() throws Exception {
final AtomicBoolean asyncFailed = new AtomicBoolean();
for (int i = 0; i < 1000; i++) {
- executor.execute(new Runnable() {
- @Override
- public void run() {
- try {
- scheduler.disableControllerService(serviceNode);
- assertFalse(serviceNode.isActive());
- } catch (final Exception e) {
- e.printStackTrace();
- asyncFailed.set(true);
- }
+ executor.execute(() -> {
+ try {
+ scheduler.disableControllerService(serviceNode);
+ assertFalse(serviceNode.isActive());
+ } catch (final Exception e) {
+ e.printStackTrace();
Review Comment:
```suggestion
```
##########
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/test/java/org/apache/nifi/controller/scheduling/TestStandardProcessScheduler.java:
##########
@@ -431,16 +418,13 @@ public void validateEnabledServiceCanOnlyBeDisabledOnce() throws Exception {
final AtomicBoolean asyncFailed = new AtomicBoolean();
for (int i = 0; i < 1000; i++) {
- executor.execute(new Runnable() {
- @Override
- public void run() {
- try {
- scheduler.disableControllerService(serviceNode);
- assertFalse(serviceNode.isActive());
- } catch (final Exception e) {
- e.printStackTrace();
- asyncFailed.set(true);
- }
+ executor.execute(() -> {
+ try {
+ scheduler.disableControllerService(serviceNode);
+ assertFalse(serviceNode.isActive());
+ } catch (final Exception e) {
+ e.printStackTrace();
Review Comment:
```suggestion
```
##########
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/test/java/org/apache/nifi/controller/scheduling/TestStandardProcessScheduler.java:
##########
@@ -352,16 +345,13 @@ public void validateServiceEnablementLogicHappensOnlyOnce() throws Exception {
final AtomicBoolean asyncFailed = new AtomicBoolean();
for (int i = 0; i < 1000; i++) {
- executor.execute(new Runnable() {
- @Override
- public void run() {
- try {
- scheduler.enableControllerService(serviceNode).get();
- assertTrue(serviceNode.isActive());
- } catch (final Exception e) {
- e.printStackTrace();
- asyncFailed.set(true);
- }
+ executor.execute(() -> {
+ try {
+ scheduler.enableControllerService(serviceNode).get();
+ assertTrue(serviceNode.isActive());
+ } catch (final Exception e) {
+ e.printStackTrace();
Review Comment:
`e.printStackTrace()` should be removed.
```suggestion
```
##########
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/test/java/org/apache/nifi/controller/TestStandardFlowFileQueue.java:
##########
@@ -71,12 +72,12 @@ public class TestStandardFlowFileQueue {
private List<ProvenanceEventRecord> provRecords = new ArrayList<>();
- @BeforeClass
+ @BeforeAll
public static void setupLogging() {
System.setProperty("org.slf4j.simpleLogger.log.org.apache.nifi", "INFO");
}
Review Comment:
This method should be removed since additional logging should not be enabled as a general rule.
##########
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/test/java/org/apache/nifi/controller/queue/clustered/TestContentRepositoryFlowFileAccess.java:
##########
@@ -121,7 +121,7 @@ public void testEOFExceptionIfNotEnoughData() throws IOException {
try {
repoStream.read();
- Assert.fail("Expected EOFException because not enough bytes were in the InputStream for the FlowFile");
+ fail("Expected EOFException because not enough bytes were in the InputStream for the FlowFile");
} catch (final EOFException eof) {
// expected
}
Review Comment:
It looks like this can be changed to `assertThrows(EOFException.class, () -> repoStream.read());`
##########
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/test/java/org/apache/nifi/integration/FrameworkIntegrationTest.java:
##########
@@ -368,7 +368,6 @@ public Collection<FlowFileQueue> getAllQueues() {
flowController.initializeFlow(queueProvider);
}
- @After
Review Comment:
It looks like `AfterEach` should be added to this method, or it should be consolidated with `shutdown()`.
##########
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/test/java/org/apache/nifi/controller/status/history/EmbeddedQuestDbRolloverHandlerTest.java:
##########
@@ -43,7 +42,10 @@
import java.util.concurrent.TimeUnit;
import java.util.function.Supplier;
-@Ignore("Buggy tests depend on time of day")
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+@Disabled("Buggy tests depend on time of day")
Review Comment:
This is a good opportunity to remove this class based on the comments. This file can be deleted.
##########
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/test/groovy/org/apache/nifi/fingerprint/FingerprintFactoryGroovyTest.groovy:
##########
@@ -57,17 +54,17 @@ class FingerprintFactoryGroovyTest extends GroovyTestCase {
}
}
- @Before
+ @BeforeEach
void setUp() throws Exception {
}
- @After
+ @AfterEach
void tearDown() throws Exception {
}
Review Comment:
These empty methods can be removed.
##########
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/test/groovy/org/apache/nifi/controller/repository/crypto/EncryptedFileSystemRepositoryTest.groovy:
##########
@@ -64,9 +61,9 @@ class EncryptedFileSystemRepositoryTest {
(NiFiProperties.CONTENT_REPOSITORY_ENCRYPTION_KEY_PROVIDER_LOCATION) : ""
]
- @BeforeClass
+ @BeforeAll
static void setUpOnce() throws Exception {
- Assume.assumeTrue("Test only runs on *nix", !SystemUtils.IS_OS_WINDOWS)
+ Assumptions.assumeTrue(!SystemUtils.IS_OS_WINDOWS, "Test only runs on *nix")
Review Comment:
This class and others should be changed to use the new `DisabledOnOs(OS.WINDOWS)` JUnit 5 annotation, will clean up some of these `SystemUtils` and `BeforeAll` setup steps.
##########
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/test/java/org/apache/nifi/integration/FrameworkIntegrationTest.java:
##########
@@ -140,8 +138,8 @@ public class FrameworkIntegrationTest {
//@Rule
public Timeout globalTimeout = Timeout.seconds(20);
- @Rule
- public TestName name = new TestName();
+ /*@Rule
+ public TestName name = new TestName();*/
Review Comment:
This commented line should be removed.
##########
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/test/java/org/apache/nifi/integration/swap/ClusteredSwapFileIT.java:
##########
@@ -34,20 +34,21 @@
import org.apache.nifi.events.EventReporter;
import org.apache.nifi.integration.FrameworkIntegrationTest;
import org.apache.nifi.integration.processors.GenerateProcessor;
-import org.junit.Ignore;
-import org.junit.Test;
+import org.junit.jupiter.api.Disabled;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.Timeout;
import org.mockito.Mockito;
import java.io.IOException;
import java.util.Collections;
import java.util.Set;
import java.util.concurrent.ExecutionException;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertNull;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertNull;
-@Ignore("Tests need to be updated")
+@Disabled("Tests need to be updated")
Review Comment:
This is a good opportunity to remove this test 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