[GitHub] [metron] mmiklavc commented on a change in pull request #1554: METRON-2307: Migrate to JUnit5
mmiklavc commented on a change in pull request #1554: METRON-2307: Migrate to JUnit5 URL: https://github.com/apache/metron/pull/1554#discussion_r354600534 ## File path: metron-platform/metron-solr/metron-solr-common/pom.xml ## @@ -199,10 +199,15 @@ + +org.hamcrest Review comment: I think it's probably better to keep it as it was before this PR if there isn't a technical reason for the change. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [metron] mmiklavc commented on a change in pull request #1554: METRON-2307: Migrate to JUnit5
mmiklavc commented on a change in pull request #1554: METRON-2307: Migrate to JUnit5 URL: https://github.com/apache/metron/pull/1554#discussion_r354598708 ## File path: metron-interface/pom.xml ## @@ -48,12 +48,6 @@ metron-rest-client - Review comment: I think we can consider how best to deal with this in a clean up. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [metron] mmiklavc commented on a change in pull request #1554: METRON-2307: Migrate to JUnit5
mmiklavc commented on a change in pull request #1554: METRON-2307: Migrate to
JUnit5
URL: https://github.com/apache/metron/pull/1554#discussion_r354597565
##
File path: metron-analytics/metron-profiler-client/pom.xml
##
@@ -122,8 +122,7 @@
org.mockito
-mockito-all
-${global_mockito_version}
+mockito-core
Review comment:
More specifically, did they change the recommendation from depending on
mockito-all to mockito-core?
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [metron] mmiklavc commented on a change in pull request #1554: METRON-2307: Migrate to JUnit5
mmiklavc commented on a change in pull request #1554: METRON-2307: Migrate to JUnit5 URL: https://github.com/apache/metron/pull/1554#discussion_r349218988 ## File path: pom.xml ## @@ -111,21 +111,21 @@ 5.6.14 1.1.1 3.0.2 -4.12 +5.5.2 +2.2 17.0 12.0 2.2.5 1.7.7 3.7 1.8 6.6.2 -1.10.19 -1.7.0 +3.1.0 3.2.0 2.7.4 2.0.14 3.0.2 -2.18 +3.0.0-M3 Review comment: Should we consider M4? https://maven.apache.org/surefire/maven-surefire-plugin/index.html > Provided 3 extensions of reporters which can be used to customize XML report, console and file reporters. It is very useful for JUnit5 users. > We reworked the internal implementation so that new commands and events can be easily added. The impl is located in a center point and it is a prerequisite in next versions. > Provided bug fixes for Docker Alpine/BusyBox Linux, JUnit5 and 43 more. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [metron] mmiklavc commented on a change in pull request #1554: METRON-2307: Migrate to JUnit5
mmiklavc commented on a change in pull request #1554: METRON-2307: Migrate to JUnit5 URL: https://github.com/apache/metron/pull/1554#discussion_r349192630 ## File path: metron-platform/metron-solr/metron-solr-common/pom.xml ## @@ -199,10 +199,15 @@ + +org.hamcrest Review comment: Is this re-ordering significant? Should we add a comment? 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [metron] mmiklavc commented on a change in pull request #1554: METRON-2307: Migrate to JUnit5
mmiklavc commented on a change in pull request #1554: METRON-2307: Migrate to
JUnit5
URL: https://github.com/apache/metron/pull/1554#discussion_r349194570
##
File path:
metron-platform/metron-solr/metron-solr-common/src/test/java/org/apache/metron/solr/dao/SolrSearchDaoTest.java
##
@@ -255,11 +210,11 @@ public void getAllLatestShouldProperlyReturnDocuments()
throws Exception {
SolrDocumentList snortList = new SolrDocumentList();
snortList.add(snortSolrDoc1);
snortList.add(snortSolrDoc2);
-when(client.getById((Collection) argThat(hasItems("bro-1",
"bro-2")),
+when(client.getById((List)
org.mockito.hamcrest.MockitoHamcrest.argThat(hasItems("bro-1", "bro-2")),
Review comment:
Not sure about the `MockitoHamcrest` matchers. Can we just use the static
import we already have? `org.mockito.ArgumentMatchers.argThat`
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [metron] mmiklavc commented on a change in pull request #1554: METRON-2307: Migrate to JUnit5
mmiklavc commented on a change in pull request #1554: METRON-2307: Migrate to
JUnit5
URL: https://github.com/apache/metron/pull/1554#discussion_r348192923
##
File path:
metron-platform/metron-management/src/test/java/org/apache/metron/management/EnrichmentConfigFunctionsTest.java
##
@@ -92,15 +84,15 @@ public void setup() {
}
return ret;
}
- private int size(Map stellarFunctions) {
+ private int size(Map stellarFunctions, String group) {
Review comment:
Can you explain the method signature changes in this class? Also line 104
w/enrichmentType.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [metron] mmiklavc commented on a change in pull request #1554: METRON-2307: Migrate to JUnit5
mmiklavc commented on a change in pull request #1554: METRON-2307: Migrate to JUnit5 URL: https://github.com/apache/metron/pull/1554#discussion_r348610276 ## File path: metron-platform/metron-pcap-backend/src/test/java/org/apache/metron/pcap/integration/PcapTopologyIntegrationTest.java ## @@ -220,13 +208,13 @@ public static void setupTopology(Function updatePropertiesCall clearOutDirs(inputDir, interimResultDir, outputDir); File baseDir = new File(new File(targetDir), BASE_DIR); -//Assert.assertEquals(0, numFiles(outDir)); -Assert.assertNotNull(topologiesDir); -Assert.assertNotNull(targetDir); +//assertEquals(0, numFiles(outDir)); Review comment: I know it was previously commented out, but should we keep this assertion? 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [metron] mmiklavc commented on a change in pull request #1554: METRON-2307: Migrate to JUnit5
mmiklavc commented on a change in pull request #1554: METRON-2307: Migrate to
JUnit5
URL: https://github.com/apache/metron/pull/1554#discussion_r348607675
##
File path:
metron-platform/metron-parsing/metron-parsing-storm/src/test/java/org/apache/metron/parsers/topology/ParserTopologyBuilderTest.java
##
@@ -22,55 +22,46 @@
import org.apache.metron.common.configuration.ParserConfigurations;
import org.apache.metron.common.configuration.SensorParserConfig;
import org.apache.metron.common.configuration.writer.ParserWriterConfiguration;
+import org.apache.metron.common.writer.BulkMessageWriter;
import org.apache.metron.parsers.bolt.WriterHandler;
import org.apache.metron.writer.NoopWriter;
import org.apache.metron.writer.kafka.KafkaWriter;
-import org.junit.Before;
-import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.mockito.Mock;
-import org.powermock.core.classloader.annotations.PrepareForTest;
-import org.powermock.modules.junit4.PowerMockRunner;
+import org.json.simple.JSONObject;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
import java.util.HashMap;
import java.util.Map;
import java.util.Optional;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertTrue;
-import static org.mockito.Matchers.any;
-import static org.mockito.Matchers.eq;
-import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.times;
-import static org.mockito.Mockito.verify;
-import static org.mockito.Mockito.verifyNoMoreInteractions;
-import static org.powermock.api.mockito.PowerMockito.spy;
-import static org.powermock.api.mockito.PowerMockito.when;
-
-@RunWith(PowerMockRunner.class)
-@PrepareForTest(ParserTopologyBuilder.class)
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.mockito.Mockito.*;
+
public class ParserTopologyBuilderTest {
- @Mock
- private ParserConfigurations configs;
+ private static ParserConfigurations configs;
+ private static KafkaWriter kafkaWriter;
- @Mock
- private KafkaWriter kafkaWriter;
+ @BeforeAll
+ public static void setupAll() {
+ configs = mock(ParserConfigurations.class);
+ kafkaWriter = mock(KafkaWriter.class);
+ }
- @Before
+ @BeforeEach
public void setup() {
-spy(ParserTopologyBuilder.class);
-when(ParserTopologyBuilder.createKafkaWriter(Optional.of("brokerUrl"),
"zookeeperUrl", Optional.of("securityProtocol")))
-.thenReturn(kafkaWriter);
+//spy(ParserTopologyBuilder.class);
Review comment:
Can we remove the setup altogether now that it's empty?
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [metron] mmiklavc commented on a change in pull request #1554: METRON-2307: Migrate to JUnit5
mmiklavc commented on a change in pull request #1554: METRON-2307: Migrate to
JUnit5
URL: https://github.com/apache/metron/pull/1554#discussion_r348194133
##
File path:
metron-platform/metron-management/src/test/java/org/apache/metron/management/FileSystemFunctionsTest.java
##
@@ -78,8 +77,23 @@ public static void setupFS() throws IOException {
}
}
- @Before
- public void setup() throws IOException {
+ @BeforeEach
+ public void setup() {
Review comment:
See other comment below regarding `setupFsTypeAndFunctions(type);`. If it
makes sense to keep those as-is, then it might make sense to delete this method
altogether.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [metron] mmiklavc commented on a change in pull request #1554: METRON-2307: Migrate to JUnit5
mmiklavc commented on a change in pull request #1554: METRON-2307: Migrate to
JUnit5
URL: https://github.com/apache/metron/pull/1554#discussion_r348193833
##
File path:
metron-platform/metron-management/src/test/java/org/apache/metron/management/FileSystemFunctionsTest.java
##
@@ -101,75 +114,74 @@ public void setup() throws IOException {
rm.initialize(null);
}
- @AfterClass
- public static void teardown() {
-{
- hdfsCluster.shutdown();
- FileUtil.fullyDelete(hdfsBaseDir);
-}
-{
- new File(localPrefix).delete();
-}
- }
-
- @Test
- public void testHappyPath() {
+ @ParameterizedTest
+ @MethodSource("types")
+ public void testHappyPath(FileSystemFunctions.FS_TYPE type) {
+setupFsTypeAndFunctions(type);
Review comment:
What's the reason for duplicating this in each test instead of keeping it in
```@BeforeEach
public void setup() throws IOException {
}
```
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [metron] mmiklavc commented on a change in pull request #1554: METRON-2307: Migrate to JUnit5
mmiklavc commented on a change in pull request #1554: METRON-2307: Migrate to
JUnit5
URL: https://github.com/apache/metron/pull/1554#discussion_r348189357
##
File path:
metron-platform/metron-indexing/metron-indexing-common/src/test/java/org/apache/metron/indexing/dao/metaalert/MetaAlertIntegrationTest.java
##
@@ -1039,18 +1018,18 @@ public void shouldThrowExceptionIfPatchStatusField()
throws Exception {
String statusPatch = statusPatchRequest.replace(META_INDEX_FLAG,
getMetaAlertIndex());
PatchRequest patchRequest = JSONUtils.INSTANCE.load(statusPatch,
PatchRequest.class);
metaDao.patch(metaDao, patchRequest,
Optional.of(System.currentTimeMillis()));
- Assert.fail("A patch on the status field should throw an exception");
+ fail("A patch on the status field should throw an exception");
Review comment:
Might be worth using the new idiom for exception testing here.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [metron] mmiklavc commented on a change in pull request #1554: METRON-2307: Migrate to JUnit5
mmiklavc commented on a change in pull request #1554: METRON-2307: Migrate to
JUnit5
URL: https://github.com/apache/metron/pull/1554#discussion_r348189144
##
File path:
metron-platform/metron-indexing/metron-indexing-common/src/test/java/org/apache/metron/indexing/dao/metaalert/MetaAlertIntegrationTest.java
##
@@ -931,9 +910,9 @@ public void shouldThrowExceptionOnMetaAlertUpdate() throws
Exception {
try {
// Verify a meta alert cannot be updated in the meta alert dao
metaDao.update(metaAlert, Optional.empty());
- Assert.fail("Direct meta alert update should throw an exception");
+ fail("Direct meta alert update should throw an exception");
Review comment:
Might be worth using the new idiom for exception testing here.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [metron] mmiklavc commented on a change in pull request #1554: METRON-2307: Migrate to JUnit5
mmiklavc commented on a change in pull request #1554: METRON-2307: Migrate to
JUnit5
URL: https://github.com/apache/metron/pull/1554#discussion_r348189222
##
File path:
metron-platform/metron-indexing/metron-indexing-common/src/test/java/org/apache/metron/indexing/dao/metaalert/MetaAlertIntegrationTest.java
##
@@ -998,18 +977,18 @@ public void shouldThrowExceptionIfPatchAlertField()
throws Exception {
String alertPatch = alertPatchRequest.replace(META_INDEX_FLAG,
getMetaAlertIndex());
PatchRequest patchRequest = JSONUtils.INSTANCE.load(alertPatch,
PatchRequest.class);
metaDao.patch(metaDao, patchRequest,
Optional.of(System.currentTimeMillis()));
- Assert.fail("A patch on the alert field should throw an exception");
+ fail("A patch on the alert field should throw an exception");
Review comment:
Might be worth using the new idiom for exception testing here.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [metron] mmiklavc commented on a change in pull request #1554: METRON-2307: Migrate to JUnit5
mmiklavc commented on a change in pull request #1554: METRON-2307: Migrate to
JUnit5
URL: https://github.com/apache/metron/pull/1554#discussion_r348189052
##
File path:
metron-platform/metron-indexing/metron-indexing-common/src/test/java/org/apache/metron/indexing/dao/metaalert/MetaAlertIntegrationTest.java
##
@@ -584,9 +561,9 @@ public void
addRemoveAlertsShouldThrowExceptionForInactiveMetaAlert() throws Exc
try {
metaDao.removeAlertsFromMetaAlert("meta_alert",
Collections.singletonList(new GetRequest("message_0",
SENSOR_NAME)));
-Assert.fail("Removing alerts from an inactive meta alert should throw
an exception");
+fail("Removing alerts from an inactive meta alert should throw an
exception");
Review comment:
Might be worth using the new idiom for exception testing here.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [metron] mmiklavc commented on a change in pull request #1554: METRON-2307: Migrate to JUnit5
mmiklavc commented on a change in pull request #1554: METRON-2307: Migrate to
JUnit5
URL: https://github.com/apache/metron/pull/1554#discussion_r348188806
##
File path:
metron-platform/metron-indexing/metron-indexing-common/src/test/java/org/apache/metron/indexing/dao/metaalert/MetaAlertIntegrationTest.java
##
@@ -246,18 +223,18 @@ public void shouldSortByThreatTriageScore() throws
Exception {
srAsc.setSort(Collections.singletonList(sfAsc));
result = metaDao.search(srAsc);
results = result.getResults();
-Assert.assertEquals("message_1",
results.get((0)).getSource().get(Constants.GUID));
-Assert.assertEquals("meta_active_0",
results.get((1)).getSource().get(Constants.GUID));
-Assert.assertEquals(2, results.size());
+assertEquals("message_1",
results.get((0)).getSource().get(Constants.GUID));
+assertEquals("meta_active_0",
results.get((1)).getSource().get(Constants.GUID));
+assertEquals(2, results.size());
}
@Test
public void getAllMetaAlertsForAlertShouldThrowExceptionForEmptyGuid()
throws Exception {
try {
metaDao.getAllMetaAlertsForAlert("");
- Assert.fail("An exception should be thrown for empty guid");
+ fail("An exception should be thrown for empty guid");
Review comment:
Might be worth using the new idiom for exception testing here.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [metron] mmiklavc commented on a change in pull request #1554: METRON-2307: Migrate to JUnit5
mmiklavc commented on a change in pull request #1554: METRON-2307: Migrate to
JUnit5
URL: https://github.com/apache/metron/pull/1554#discussion_r348122344
##
File path:
metron-platform/metron-elasticsearch/metron-elasticsearch-common/src/test/java/org/apache/metron/elasticsearch/writer/ElasticsearchWriterTest.java
##
@@ -260,9 +240,9 @@ public void shouldWriteManySuccessfullyWithSetDocumentId()
{
when(writerConfiguration.isSetDocumentId("bro")).thenReturn(true);
when(writerConfiguration.getFieldNameConverter("bro")).thenReturn("NOOP");
-mockStatic(ElasticsearchUtils.class);
-when(ElasticsearchUtils.getIndexFormat(globals())).thenReturn(new
SimpleDateFormat());
-when(ElasticsearchUtils.getIndexName(eq("bro"), any(),
eq(writerConfiguration))).thenReturn("bro_index");
+//mockStatic(ElasticsearchUtils.class);
Review comment:
Missed commented out code. How is this not needed?
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [metron] mmiklavc commented on a change in pull request #1554: METRON-2307: Migrate to JUnit5
mmiklavc commented on a change in pull request #1554: METRON-2307: Migrate to
JUnit5
URL: https://github.com/apache/metron/pull/1554#discussion_r348187588
##
File path:
metron-platform/metron-indexing/metron-indexing-common/src/main/java/org/apache/metron/indexing/dao/search/SortField.java
##
@@ -37,6 +39,11 @@ public void setSortOrder(String sortOrder) {
this.sortOrder = SortOrder.fromString(sortOrder);
}
+ @Override
+ public int hashCode() {
Review comment:
:-O
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [metron] mmiklavc commented on a change in pull request #1554: METRON-2307: Migrate to JUnit5
mmiklavc commented on a change in pull request #1554: METRON-2307: Migrate to
JUnit5
URL: https://github.com/apache/metron/pull/1554#discussion_r348189008
##
File path:
metron-platform/metron-indexing/metron-indexing-common/src/test/java/org/apache/metron/indexing/dao/metaalert/MetaAlertIntegrationTest.java
##
@@ -572,9 +549,9 @@ public void
addRemoveAlertsShouldThrowExceptionForInactiveMetaAlert() throws Exc
try {
metaDao.addAlertsToMetaAlert("meta_alert",
Collections.singletonList(new GetRequest("message_1",
SENSOR_NAME)));
-Assert.fail("Adding alerts to an inactive meta alert should throw an
exception");
+fail("Adding alerts to an inactive meta alert should throw an
exception");
Review comment:
Might be worth using the new idiom for exception testing here.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [metron] mmiklavc commented on a change in pull request #1554: METRON-2307: Migrate to JUnit5
mmiklavc commented on a change in pull request #1554: METRON-2307: Migrate to JUnit5 URL: https://github.com/apache/metron/pull/1554#discussion_r346963180 ## File path: metron-interface/pom.xml ## @@ -48,12 +48,6 @@ metron-rest-client - Review comment: Inheriting from somewhere else 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [metron] mmiklavc commented on a change in pull request #1554: METRON-2307: Migrate to JUnit5
mmiklavc commented on a change in pull request #1554: METRON-2307: Migrate to
JUnit5
URL: https://github.com/apache/metron/pull/1554#discussion_r346965595
##
File path:
metron-platform/metron-common-streaming/metron-common-storm/src/test/java/org/apache/metron/storm/common/utils/ErrorUtilsTest.java
##
@@ -21,25 +21,23 @@
import org.apache.metron.common.error.MetronError;
import org.apache.metron.test.error.MetronErrorJSONMatcher;
import org.apache.storm.task.OutputCollector;
-import org.junit.Test;
+import org.junit.jupiter.api.Test;
+import org.mockito.ArgumentMatchers;
-import static org.mockito.Matchers.any;
-import static org.mockito.Matchers.argThat;
-import static org.mockito.Matchers.eq;
-import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.times;
-import static org.mockito.Mockito.verify;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.argThat;
+import static org.mockito.Mockito.*;
public class ErrorUtilsTest {
@Test
- public void handleErrorShouldEmitAndReportError() throws Exception {
+ public void handleErrorShouldEmitAndReportError() {
Throwable e = new Exception("error");
MetronError error = new MetronError().withMessage("error
message").withThrowable(e);
OutputCollector collector = mock(OutputCollector.class);
StormErrorUtils.handleError(collector, error);
-verify(collector, times(1)).emit(eq(Constants.ERROR_STREAM), argThat(new
MetronErrorJSONMatcher(error.getJSONObject(;
+verify(collector,
times(1)).emit(ArgumentMatchers.eq(Constants.ERROR_STREAM), argThat(new
MetronErrorJSONMatcher(error.getJSONObject(;
Review comment:
Can we keep the `eq` as a static import for readability?
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [metron] mmiklavc commented on a change in pull request #1554: METRON-2307: Migrate to JUnit5
mmiklavc commented on a change in pull request #1554: METRON-2307: Migrate to
JUnit5
URL: https://github.com/apache/metron/pull/1554#discussion_r346952052
##
File path:
metron-analytics/metron-statistics/src/test/java/org/apache/metron/statistics/outlier/MedianAbsoluteDeviationTest.java
##
@@ -93,23 +94,23 @@ public void test() {
}
{
Double score = (Double) run("OUTLIER_MAD_SCORE(currentState, value)",
ImmutableMap.of("currentState", currentState, "value", stats.getMin()));
- Assert.assertTrue("Score: " + score + " is not an outlier despite being
a minimum.", score > 3.5);
+ assertTrue(score > 3.5, "Score: " + score + " is not an outlier despite
being a minimum.");
Review comment:
Aw geeze, they reversed the test case and the failure message in the new
API. Meanwhile, Hamcrest still has the other order:
```
public static void assertTrue(boolean condition, String message) {
AssertTrue.assertTrue(condition, message);
}
vs
public static void assertThat(String reason, T actual, Matcher matcher)
```
ಠ_ಠ
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [metron] mmiklavc commented on a change in pull request #1554: METRON-2307: Migrate to JUnit5
mmiklavc commented on a change in pull request #1554: METRON-2307: Migrate to
JUnit5
URL: https://github.com/apache/metron/pull/1554#discussion_r346952601
##
File path: metron-analytics/pom.xml
##
@@ -61,9 +61,33 @@
${global_slf4j_version}
- junit
- junit
- ${global_junit_version}
+ org.junit.jupiter
+ junit-jupiter-api
+ ${global_junit_jupiter_version}
+ test
+
+
+ org.junit.jupiter
+ junit-jupiter-params
+ ${global_junit_jupiter_version}
+ test
+
+
+ org.junit.jupiter
+ junit-jupiter-engine
+ ${global_junit_jupiter_version}
+ test
+
+
+ org.junit.jupiter
+ junit-jupiter-migrationsupport
+ ${global_junit_jupiter_version}
+ test
+
+
+ org.hamcrest
Review comment:
This bc of changing mockito deps? re: hamcrest dep here
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [metron] mmiklavc commented on a change in pull request #1554: METRON-2307: Migrate to JUnit5
mmiklavc commented on a change in pull request #1554: METRON-2307: Migrate to
JUnit5
URL: https://github.com/apache/metron/pull/1554#discussion_r346926591
##
File path:
metron-analytics/metron-statistics/src/test/java/org/apache/metron/statistics/approximation/HyperLogLogPlusFunctionsTest.java
##
@@ -18,37 +18,33 @@
package org.apache.metron.statistics.approximation;
import com.google.common.collect.ImmutableList;
-import org.junit.Assert;
-import org.junit.Rule;
-import org.junit.Test;
-import org.junit.rules.ExpectedException;
+import org.junit.jupiter.api.Test;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import static org.hamcrest.CoreMatchers.*;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
public class HyperLogLogPlusFunctionsTest {
@Test
public void hllp_init_creates_HyperLogLogPlus_set() {
HyperLogLogPlus hllp = (HyperLogLogPlus) new
HyperLogLogPlusFunctions.HLLPInit().apply(ImmutableList.of());
-Assert.assertThat(hllp.getSp(), equalTo(25));
-Assert.assertThat(hllp.getP(), equalTo(14));
-Assert.assertThat("instance types should match for constructor with
default precision values", new
HyperLogLogPlusFunctions.HLLPInit().apply(ImmutableList.of(5)),
instanceOf(HyperLogLogPlus.class));
-Assert.assertThat("instance types should match for constructor with sparse
set disabled", new
HyperLogLogPlusFunctions.HLLPInit().apply(ImmutableList.of(5)),
instanceOf(HyperLogLogPlus.class));
-Assert.assertThat("instance types should match for full constructor", new
HyperLogLogPlusFunctions.HLLPInit().apply(ImmutableList.of(5, 6)),
instanceOf(HyperLogLogPlus.class));
+assertThat(hllp.getSp(), equalTo(25));
+assertThat(hllp.getP(), equalTo(14));
+assertThat("instance types should match for constructor with default
precision values", new
HyperLogLogPlusFunctions.HLLPInit().apply(ImmutableList.of(5)),
instanceOf(HyperLogLogPlus.class));
+assertThat("instance types should match for constructor with sparse set
disabled", new HyperLogLogPlusFunctions.HLLPInit().apply(ImmutableList.of(5)),
instanceOf(HyperLogLogPlus.class));
+assertThat("instance types should match for full constructor", new
HyperLogLogPlusFunctions.HLLPInit().apply(ImmutableList.of(5, 6)),
instanceOf(HyperLogLogPlus.class));
}
- @Rule
- public ExpectedException thrown = ExpectedException.none();
-
@Test
public void hllp_init_with_incorrect_args_throws_exception() {
-thrown.expect(IllegalArgumentException.class);
-thrown.expectMessage("Unable to get p value from 'turkey'");
-new HyperLogLogPlusFunctions.HLLPInit().apply(ImmutableList.of("turkey"));
+Exception e = assertThrows(IllegalArgumentException.class, () -> new
HyperLogLogPlusFunctions.HLLPInit().apply(ImmutableList.of("turkey")));
+assertEquals("Unable to get p value from 'turkey'", e.getMessage());
Review comment:
Ah, that's how they do it re: exception message checking. Not bad. I think
this is definitely clearer.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [metron] mmiklavc commented on a change in pull request #1554: METRON-2307: Migrate to JUnit5
mmiklavc commented on a change in pull request #1554: METRON-2307: Migrate to
JUnit5
URL: https://github.com/apache/metron/pull/1554#discussion_r346974980
##
File path:
metron-platform/metron-common/src/test/java/org/apache/metron/common/performance/PerformanceLoggerTest.java
##
@@ -62,29 +57,29 @@ public void setup() {
}
@Test
- public void logs_on_threshold() throws Exception {
+ public void logs_on_threshold() {
when(timing.getElapsed("t1")).thenReturn(111L).thenReturn(222L).thenReturn(333L);
perfLogger.mark("t1");
perfLogger.log("t1");
perfLogger.log("t1");
perfLogger.log("t1");
verify(timing).mark("t1");
-verify(logger, times(1)).debug(anyString(), anyObject(), eq(111L), eq(""));
+verify(logger, times(1)).debug(any(String.class), any(), eq(111L), eq(""));
}
@Test
- public void logs_on_threshold_with_message() throws Exception {
+ public void logs_on_threshold_with_message() {
when(timing.getElapsed("t1")).thenReturn(111L).thenReturn(222L).thenReturn(333L);
perfLogger.mark("t1");
perfLogger.log("t1", "my message");
perfLogger.log("t1", "my message");
perfLogger.log("t1", "my message");
verify(timing).mark("t1");
-verify(logger, times(1)).debug(anyString(), anyObject(), eq(111L), eq("my
message"));
+verify(logger, times(1)).debug(any(String.class), any(), eq(111L), eq("my
message"));
Review comment:
Agreed on swapping `any()` for `anyObject()` per the javadoc. Why'd you take
out `anyString()`? It also checks for non-null (which is what we want).
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [metron] mmiklavc commented on a change in pull request #1554: METRON-2307: Migrate to JUnit5
mmiklavc commented on a change in pull request #1554: METRON-2307: Migrate to
JUnit5
URL: https://github.com/apache/metron/pull/1554#discussion_r346959562
##
File path:
metron-interface/metron-rest/src/test/java/org/apache/metron/rest/service/impl/GrokServiceImplTest.java
##
@@ -84,44 +78,37 @@ public void
getCommonGrokPattersShouldCallGrokToGetPatternsAndNotAlterValue() th
}
@Test
- public void
validateGrokStatementShouldThrowExceptionWithNullStringAsPatternLabel() throws
Exception {
-exception.expect(RestException.class);
-exception.expectMessage("Pattern label is required");
-
+ public void
validateGrokStatementShouldThrowExceptionWithNullStringAsPatternLabel() {
GrokValidation grokValidation = new GrokValidation();
grokValidation.setResults(new HashMap<>());
grokValidation.setSampleData("asdf asdf");
grokValidation.setStatement("LABEL %{WORD:word1} %{WORD:word2}");
-grokService.validateGrokStatement(grokValidation);
+RestException e = assertThrows(RestException.class, () ->
grokService.validateGrokStatement(grokValidation));
+assertEquals("Pattern label is required", e.getMessage());
}
@Test
- public void
validateGrokStatementShouldThrowExceptionWithEmptyStringAsStatement() throws
Exception {
-exception.expect(RestException.class);
-exception.expectMessage("Grok statement is required");
-
+ public void
validateGrokStatementShouldThrowExceptionWithEmptyStringAsStatement() {
GrokValidation grokValidation = new GrokValidation();
grokValidation.setResults(new HashMap<>());
grokValidation.setSampleData("asdf asdf");
grokValidation.setPatternLabel("LABEL");
grokValidation.setStatement("");
-grokService.validateGrokStatement(grokValidation);
+RestException e = assertThrows(RestException.class, () ->
grokService.validateGrokStatement(grokValidation));
Review comment:
Missing this? `assertEquals("Grok statement is required", e.getMessage());`
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [metron] mmiklavc commented on a change in pull request #1554: METRON-2307: Migrate to JUnit5
mmiklavc commented on a change in pull request #1554: METRON-2307: Migrate to
JUnit5
URL: https://github.com/apache/metron/pull/1554#discussion_r346957416
##
File path:
metron-interface/metron-rest/src/test/java/org/apache/metron/rest/config/KnoxSSOAuthenticationFilterTest.java
##
@@ -100,19 +75,18 @@ public void doFilterShouldProperlySetAuthentication()
throws Exception {
ServletResponse response = mock(ServletResponse.class);
FilterChain chain = mock(FilterChain.class);
SignedJWT signedJWT = mock(SignedJWT.class);
-mockStatic(SignedJWT.class);
JWTClaimsSet jwtClaimsSet = new
JWTClaimsSet.Builder().subject("userName").build();
Authentication authentication = mock(Authentication.class);
SecurityContext securityContext = mock(SecurityContext.class);
-mockStatic(SecurityContextHolder.class);
when(request.getHeader("Authorization")).thenReturn(null);
doReturn("serializedJWT").when(knoxSSOAuthenticationFilter).getJWTFromCookie(request);
-when(SignedJWT.parse("serializedJWT")).thenReturn(signedJWT);
+//when(SignedJWT.parse("serializedJWT")).thenReturn(signedJWT);
Review comment:
commented out code
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [metron] mmiklavc commented on a change in pull request #1554: METRON-2307: Migrate to JUnit5
mmiklavc commented on a change in pull request #1554: METRON-2307: Migrate to
JUnit5
URL: https://github.com/apache/metron/pull/1554#discussion_r346952840
##
File path: metron-contrib/metron-performance/pom.xml
##
@@ -54,12 +54,6 @@
${global_kafka_version}
provided
Review comment:
Do we not have any real junit use here?
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [metron] mmiklavc commented on a change in pull request #1554: METRON-2307: Migrate to JUnit5
mmiklavc commented on a change in pull request #1554: METRON-2307: Migrate to
JUnit5
URL: https://github.com/apache/metron/pull/1554#discussion_r346959183
##
File path:
metron-interface/metron-rest/src/test/java/org/apache/metron/rest/service/impl/DockerStormCLIWrapperTest.java
##
@@ -71,12 +64,11 @@ public void
getProcessBuilderShouldProperlyGenerateProcessorBuilder() throws Exc
ProcessBuilder actualBuilder =
dockerStormCLIWrapper.getProcessBuilder("oo", "ooo");
+assertThat(actualBuilder.environment(), hasEntry("METRON_VERSION", "1"));
Review comment:
Thank you.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [metron] mmiklavc commented on a change in pull request #1554: METRON-2307: Migrate to JUnit5
mmiklavc commented on a change in pull request #1554: METRON-2307: Migrate to
JUnit5
URL: https://github.com/apache/metron/pull/1554#discussion_r346971932
##
File path:
metron-platform/metron-common/src/test/java/org/apache/metron/common/field/transformation/StellarTransformationTest.java
##
@@ -28,39 +28,16 @@
import org.apache.metron.stellar.common.CachingStellarProcessor;
import org.apache.metron.stellar.dsl.Context;
import org.json.simple.JSONObject;
-import org.junit.Assert;
-import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.junit.runners.Parameterized;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.MethodSource;
-import java.util.Arrays;
-import java.util.Collection;
import java.util.HashMap;
+import java.util.stream.Stream;
-@RunWith(Parameterized.class)
-public class StellarTransformationTest {
- Context context;
- public StellarTransformationTest(Cache
cache) {
-if(cache == null) {
- context = Context.EMPTY_CONTEXT();
-}
-else {
- context = new Context.Builder().with(Context.Capabilities.CACHE, () ->
cache).build();
-}
- }
-
- @Parameterized.Parameters
Review comment:
Won't removing this change the types of validations this suite of tests will
perform?
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [metron] mmiklavc commented on a change in pull request #1554: METRON-2307: Migrate to JUnit5
mmiklavc commented on a change in pull request #1554: METRON-2307: Migrate to
JUnit5
URL: https://github.com/apache/metron/pull/1554#discussion_r346408715
##
File path:
metron-analytics/metron-profiler-client/src/test/java/org/apache/metron/profiler/client/stellar/GetProfileTest.java
##
@@ -284,7 +275,7 @@ public void testMissingContext() {
// validate - function should be unable to initialize
String expr = "PROFILE_GET('profile1', 'entity1', PROFILE_FIXED(1000,
'SECONDS'), groups)";
-run(expr, List.class);
+assertThrows(ParseException.class, () -> run(expr, List.class));
Review comment:
That's an interesting new idiom for exception checking. I haven't seen the
new model for checking the message contents yet, so I'm curious to see how that
looks.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [metron] mmiklavc commented on a change in pull request #1554: METRON-2307: Migrate to JUnit5
mmiklavc commented on a change in pull request #1554: METRON-2307: Migrate to
JUnit5
URL: https://github.com/apache/metron/pull/1554#discussion_r346405820
##
File path: metron-analytics/metron-profiler-client/pom.xml
##
@@ -122,8 +122,7 @@
org.mockito
-mockito-all
-${global_mockito_version}
+mockito-core
Review comment:
Is this general dep cleanup for Mockito, or is there a new preferred idiom
for using Mockito deps?
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [metron] mmiklavc commented on a change in pull request #1554: METRON-2307: Migrate to JUnit5
mmiklavc commented on a change in pull request #1554: METRON-2307: Migrate to
JUnit5
URL: https://github.com/apache/metron/pull/1554#discussion_r346583199
##
File path:
metron-analytics/metron-profiler-common/src/test/java/org/apache/metron/profiler/DefaultProfileBuilderTest.java
##
@@ -73,7 +71,7 @@ public void setup() throws Exception {
* }
*/
@Multiline
- private String testInitProfile;
+ private static String testInitProfile;
Review comment:
I'm surprised this even ran without those Strings being `static` before
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [metron] mmiklavc commented on a change in pull request #1554: METRON-2307: Migrate to JUnit5
mmiklavc commented on a change in pull request #1554: METRON-2307: Migrate to
JUnit5
URL: https://github.com/apache/metron/pull/1554#discussion_r346599789
##
File path:
metron-analytics/metron-statistics/src/test/java/org/apache/metron/statistics/OnlineStatisticsProviderTest.java
##
@@ -96,19 +98,22 @@ private void validateEquality(Iterable values) {
validateStatisticsProvider(aggregatedProvider, summaryStats, stats);
}
- @Test(expected = IllegalStateException.class)
+ @Test
public void testOverflow() {
OnlineStatisticsProvider statsProvider = new OnlineStatisticsProvider();
-statsProvider.addValue(Double.MAX_VALUE + 1);
+assertThrows(IllegalStateException.class, () ->
statsProvider.addValue(Double.MAX_VALUE + 1));
}
- @Test(expected = IllegalStateException.class)
+ @Test
public void testUnderflow() {
OnlineStatisticsProvider statsProvider = new OnlineStatisticsProvider();
double d = 3e-305;
-for(int i = 0;i < 5;++i,d/=10) {
- statsProvider.addValue(d);
-}
+//for(int i = 0;i < 5;++i,d/=10) {
+assertThrows(IllegalStateException.class, () -> statsProvider.addValue(d));
+//}
+//for(int i = 0;i < 5;++i,d/=10) {
Review comment:
Looks like we changed the test semantics a bit here. Are we sure those loops
should be removed? If so, can we remove the commented out code?
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [metron] mmiklavc commented on a change in pull request #1554: METRON-2307: Migrate to JUnit5
mmiklavc commented on a change in pull request #1554: METRON-2307: Migrate to
JUnit5
URL: https://github.com/apache/metron/pull/1554#discussion_r346411604
##
File path:
metron-analytics/metron-profiler-client/src/test/java/org/apache/metron/profiler/client/window/WindowProcessorTest.java
##
@@ -262,66 +264,80 @@ public void testDateDaySpecifier() throws ParseException
{
SimpleDateFormat sdf = new SimpleDateFormat("/MM/dd HH:mm");
Date now = sdf.parse("2017/12/26 12:00");
List> intervals = w.toIntervals(now.getTime());
- Assert.assertEquals(1, intervals.size());
+ assertEquals(1, intervals.size());
Date includedDate = new Date(intervals.get(0).getMinimum());
SimpleDateFormat equalityFormat = new SimpleDateFormat("MMdd");
- Assert.assertEquals("20171225", equalityFormat.format(includedDate));
+ assertEquals("20171225", equalityFormat.format(includedDate));
}
{
Window w = WindowProcessor.process("30 minute window every 24 hours from
14 days ago excluding date:2017/12/25");
SimpleDateFormat sdf = new SimpleDateFormat("/MM/dd HH:mm");
Date now = sdf.parse("2017/12/26 12:00");
List> intervals = w.toIntervals(now.getTime());
- Assert.assertEquals(13, intervals.size());
+ assertEquals(13, intervals.size());
}
{
Window w = WindowProcessor.process("30 minute window every 24 hours from
14 days ago including date:2017/12/25, date:2017/12/24");
SimpleDateFormat sdf = new SimpleDateFormat("/MM/dd HH:mm");
Date now = sdf.parse("2017/12/26 12:00");
List> intervals = w.toIntervals(now.getTime());
- Assert.assertEquals(2, intervals.size());
+ assertEquals(2, intervals.size());
{
Date includedDate = new Date(intervals.get(0).getMinimum());
SimpleDateFormat equalityFormat = new SimpleDateFormat("MMdd");
-Assert.assertEquals("20171224", equalityFormat.format(includedDate));
+assertEquals("20171224", equalityFormat.format(includedDate));
}
{
Date includedDate = new Date(intervals.get(1).getMinimum());
SimpleDateFormat equalityFormat = new SimpleDateFormat("MMdd");
-Assert.assertEquals("20171225", equalityFormat.format(includedDate));
+assertEquals("20171225", equalityFormat.format(includedDate));
}
}
}
- @Test(expected=org.apache.metron.stellar.dsl.ParseException.class)
- public void testWithInvalidDaySpecifier() throws ParseException {
-WindowProcessor.process("30 minute window every 24 hours from 14 days ago
excluding hoolidays:us");
+ @Test
+ public void testWithInvalidDaySpecifier() {
+assertThrows(
+org.apache.metron.stellar.dsl.ParseException.class,
+() ->
+WindowProcessor.process(
+"30 minute window every 24 hours from 14 days ago excluding
hoolidays:us"));
}
- @Test(expected=org.apache.metron.stellar.dsl.ParseException.class)
- public void testWithInvalidTimeUnit() throws ParseException {
-WindowProcessor.process("30 minute window every 24 months from 14 days
ago");
+ @Test
+ public void testWithInvalidTimeUnit() {
+assertThrows(
+org.apache.metron.stellar.dsl.ParseException.class,
+() -> WindowProcessor.process("30 minute window every 24 months from
14 days ago"));
}
- @Test(expected=org.apache.metron.stellar.dsl.ParseException.class)
- public void testWithInvalidWindowUnit() throws ParseException {
-WindowProcessor.process("30 minuete window every 24 hours from 14 days
ago");
+ @Test
+ public void testWithInvalidWindowUnit() {
+assertThrows(
+org.apache.metron.stellar.dsl.ParseException.class,
+() -> WindowProcessor.process("30 minuete window every 24 hours from
14 days ago"));
}
- @Test(expected=org.apache.metron.stellar.dsl.ParseException.class)
- public void testWithInvalidTimeNumber() throws ParseException {
-WindowProcessor.process("30p minute window every 24 hours from 14 days
ago");
+ @Test
+ public void testWithInvalidTimeNumber() {
+assertThrows(
+org.apache.metron.stellar.dsl.ParseException.class,
+() -> WindowProcessor.process("30p minute window every 24 hours from
14 days ago"));
}
- @Test(expected=org.apache.metron.stellar.dsl.ParseException.class)
- public void testInvalidDaySpecifier() throws ParseException {
-WindowProcessor.process("30 minute window every 14 hours from 14 days ago
including date");
+ @Test
+ public void testInvalidDaySpecifier() {
+assertThrows(
+org.apache.metron.stellar.dsl.ParseException.class,
+() ->
+WindowProcessor.process(
+"30 minute window every 14 hours from 14 days ago including
date"));
}
- private static void assertEquals(long expected, long actual) {
+ private static void assertTimeEquals(long expected, long actual) {
Review comment:
[GitHub] [metron] mmiklavc commented on a change in pull request #1554: METRON-2307: Migrate to JUnit5
mmiklavc commented on a change in pull request #1554: METRON-2307: Migrate to
JUnit5
URL: https://github.com/apache/metron/pull/1554#discussion_r346598621
##
File path: metron-analytics/metron-profiler-storm/pom.xml
##
@@ -301,14 +301,7 @@
org.mockito
mockito-core
-${global_mockito_version}
test
Review comment:
Sure about removing this exclusion? At one point there was some drama
between versions of mockito and what it pulled in for hamcrest.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [metron] mmiklavc commented on a change in pull request #1554: METRON-2307: Migrate to JUnit5
mmiklavc commented on a change in pull request #1554: METRON-2307: Migrate to
JUnit5
URL: https://github.com/apache/metron/pull/1554#discussion_r346403489
##
File path:
metron-analytics/metron-maas-service/src/test/java/org/apache/metron/maas/service/MaasIntegrationTest.java
##
@@ -234,13 +238,13 @@ public void run() {
}
Thread.sleep(2000);
}
-Assert.assertTrue(passed);
+assertTrue(passed);
}
{
List endpoints = discoverer.getEndpoints(new
Model("dummy", "1.0"));
-Assert.assertNotNull(endpoints);
-Assert.assertEquals(1, endpoints.size());
+assertNotNull(endpoints);
Review comment:
Haven't had a chance to see if you replaced with static imports everywhere,
but I'm a fan. Thanks for this improvement.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [metron] mmiklavc commented on a change in pull request #1554: METRON-2307: Migrate to JUnit5
mmiklavc commented on a change in pull request #1554: METRON-2307: Migrate to
JUnit5
URL: https://github.com/apache/metron/pull/1554#discussion_r346598891
##
File path:
metron-analytics/metron-profiler-storm/src/test/java/org/apache/metron/hbase/bolt/HBaseBoltTest.java
##
@@ -67,18 +65,15 @@ public void setupTuples() throws Exception {
when(tuple2.getValueByField(eq("widget"))).thenReturn(widget2);
}
- @Before
- public void setup() throws Exception {
-tuple1 = mock(Tuple.class);
-tuple2 = mock(Tuple.class);
-client = mock(HBaseClient.class);
-provider = mock(TableProvider.class);
+ @BeforeEach
+ public void setup() {
Review comment:
Looks like the redundant setup method should be removed entirely?
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [metron] mmiklavc commented on a change in pull request #1554: METRON-2307: Migrate to JUnit5
mmiklavc commented on a change in pull request #1554: METRON-2307: Migrate to
JUnit5
URL: https://github.com/apache/metron/pull/1554#discussion_r346582680
##
File path: metron-analytics/metron-profiler-common/pom.xml
##
@@ -79,8 +79,26 @@
org.mockito
-mockito-all
-${global_mockito_version}
+mockito-core
+test
+
+
+
+org.junit.jupiter
Review comment:
Should we make these Junit Jupiter deps all inherited by default from the
common parent pom? At the very least, this is a great candidate for using the
`dependencyManagement` section to specify the version numbers so we only have
to declare it once.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [metron] mmiklavc commented on a change in pull request #1554: METRON-2307: Migrate to JUnit5
mmiklavc commented on a change in pull request #1554: METRON-2307: Migrate to JUnit5 URL: https://github.com/apache/metron/pull/1554#discussion_r346400874 ## File path: dependencies_with_url.csv ## @@ -52,6 +52,7 @@ javax.xml.stream:stax-api:jar:1.0-2:compile,COMMON DEVELOPMENT AND DISTRIBUTION jline:jline:jar:0.9.94:compile,BSD,http://jline.sourceforge.net junit:junit:jar:4.12:compile,Eclipse Public License 1.0,http://junit.org junit:junit:jar:4.4:compile,Common Public License Version 1.0,http://junit.org +junit:junit:jar:4.10:compile,Common Public License Version 1.0,http://junit.org Review comment: I was going to ask if we could remove all the junit 4.x references now that we're moving to JUnit 5. Seeing this addition of 4.10, I'm curious what's happening here. Does 5 still depend on 4? Or is this not a complete transition to 5? If there's anything leftover depending on 4, it would be good to get a list for house cleaning. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [metron] mmiklavc commented on a change in pull request #1554: METRON-2307: Migrate to JUnit5
mmiklavc commented on a change in pull request #1554: METRON-2307: Migrate to
JUnit5
URL: https://github.com/apache/metron/pull/1554#discussion_r346589232
##
File path: metron-analytics/metron-profiler-spark/pom.xml
##
@@ -151,6 +151,18 @@
${project.parent.version}
test
+
+org.junit.jupiter
Review comment:
Same as other dep mgmt comment for junit
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
