You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@druid.apache.org by GitBox <gi...@apache.org> on 2020/06/14 16:41:46 UTC
[GitHub] [druid] stefanbirkner opened a new pull request #10032: Refurbish BroadcastDistributionRuleSerdeTest
stefanbirkner opened a new pull request #10032:
URL: https://github.com/apache/druid/pull/10032
### Description
We only need to test serialization of a rule object itself instead of a
list with that object.
Since JUnit 4.12 it is possible to return a list of Objects if there is
only one parameter per test.
Parameters can be injected easily by annotating a field with
@Parameter.
<hr>
This PR has:
- [x] been self-reviewed.
<hr>
##### Key changed/added classes in this PR
* `BroadcastDistributionRuleSerdeTest`
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] stefanbirkner commented on pull request #10032: Refurbish BroadcastDistributionRuleSerdeTest
Posted by GitBox <gi...@apache.org>.
stefanbirkner commented on pull request #10032:
URL: https://github.com/apache/druid/pull/10032#issuecomment-655094730
@jihoonson Sorry for not checking Travis. I fixed the two issues. Now Travis fails with a license problem but I don't understand why. It fails for each extension with an error similar to
Encountered error [Command 'mvn -Ddependency.locations.enabled=false -Ddependency.details.enabled=false
project-info-reports:dependencies' returned non-zero exit status 1]
when generating report for /home/travis/build/apache/druid/extensions-core/s3-extensions
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] stale[bot] commented on pull request #10032: Refurbish BroadcastDistributionRuleSerdeTest
Posted by GitBox <gi...@apache.org>.
stale[bot] commented on pull request #10032:
URL: https://github.com/apache/druid/pull/10032#issuecomment-688000374
This pull request has been marked as stale due to 60 days of inactivity. It will be closed in 4 weeks if no further activity occurs. If you think that's incorrect or this pull request should instead be reviewed, please simply write any comment. Even if closed, you can still revive the PR at any time or discuss it on the dev@druid.apache.org list. Thank you for your contributions.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] stale[bot] commented on pull request #10032: Refurbish BroadcastDistributionRuleSerdeTest
Posted by GitBox <gi...@apache.org>.
stale[bot] commented on pull request #10032:
URL: https://github.com/apache/druid/pull/10032#issuecomment-706796108
This pull request/issue has been closed due to lack of activity. If you think that is incorrect, or the pull request requires review, you can revive the PR at any time.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] stefanbirkner commented on a change in pull request #10032: Refurbish BroadcastDistributionRuleSerdeTest
Posted by GitBox <gi...@apache.org>.
stefanbirkner commented on a change in pull request #10032:
URL: https://github.com/apache/druid/pull/10032#discussion_r451045359
##########
File path: server/src/test/java/org/apache/druid/server/coordinator/rules/BroadcastDistributionRuleSerdeTest.java
##########
@@ -19,49 +19,45 @@
package org.apache.druid.server.coordinator.rules;
-import com.fasterxml.jackson.core.type.TypeReference;
import com.fasterxml.jackson.databind.ObjectMapper;
-import com.google.common.collect.Lists;
import org.apache.druid.jackson.DefaultObjectMapper;
import org.apache.druid.java.util.common.Intervals;
import org.joda.time.Period;
import org.junit.Assert;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameter;
+import org.junit.runners.Parameterized.Parameters;
import java.io.IOException;
-import java.util.Collections;
import java.util.List;
+import static java.util.Arrays.asList;
+
@RunWith(Parameterized.class)
public class BroadcastDistributionRuleSerdeTest
{
private static final ObjectMapper MAPPER = new DefaultObjectMapper();
- @Parameterized.Parameters
- public static List<Object[]> constructorFeeder()
+ @Parameters(name ="{0}")
Review comment:
Fixed.
##########
File path: server/src/test/java/org/apache/druid/server/coordinator/rules/BroadcastDistributionRuleSerdeTest.java
##########
@@ -19,49 +19,45 @@
package org.apache.druid.server.coordinator.rules;
-import com.fasterxml.jackson.core.type.TypeReference;
import com.fasterxml.jackson.databind.ObjectMapper;
-import com.google.common.collect.Lists;
import org.apache.druid.jackson.DefaultObjectMapper;
import org.apache.druid.java.util.common.Intervals;
import org.joda.time.Period;
import org.junit.Assert;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameter;
+import org.junit.runners.Parameterized.Parameters;
import java.io.IOException;
-import java.util.Collections;
import java.util.List;
+import static java.util.Arrays.asList;
+
@RunWith(Parameterized.class)
public class BroadcastDistributionRuleSerdeTest
{
private static final ObjectMapper MAPPER = new DefaultObjectMapper();
- @Parameterized.Parameters
- public static List<Object[]> constructorFeeder()
+ @Parameters(name ="{0}")
+ public static List<Object> constructorFeeder()
{
- return Lists.newArrayList(
- new Object[]{new ForeverBroadcastDistributionRule()},
- new Object[]{new IntervalBroadcastDistributionRule(Intervals.of("0/1000"))},
- new Object[]{new PeriodBroadcastDistributionRule(new Period(1000), null)}
+ return asList(
Review comment:
Fixed.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] jihoonson commented on a change in pull request #10032: Refurbish BroadcastDistributionRuleSerdeTest
Posted by GitBox <gi...@apache.org>.
jihoonson commented on a change in pull request #10032:
URL: https://github.com/apache/druid/pull/10032#discussion_r447405390
##########
File path: server/src/test/java/org/apache/druid/server/coordinator/rules/BroadcastDistributionRuleSerdeTest.java
##########
@@ -19,49 +19,45 @@
package org.apache.druid.server.coordinator.rules;
-import com.fasterxml.jackson.core.type.TypeReference;
import com.fasterxml.jackson.databind.ObjectMapper;
-import com.google.common.collect.Lists;
import org.apache.druid.jackson.DefaultObjectMapper;
import org.apache.druid.java.util.common.Intervals;
import org.joda.time.Period;
import org.junit.Assert;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameter;
+import org.junit.runners.Parameterized.Parameters;
import java.io.IOException;
-import java.util.Collections;
import java.util.List;
+import static java.util.Arrays.asList;
+
@RunWith(Parameterized.class)
public class BroadcastDistributionRuleSerdeTest
{
private static final ObjectMapper MAPPER = new DefaultObjectMapper();
- @Parameterized.Parameters
- public static List<Object[]> constructorFeeder()
+ @Parameters(name ="{0}")
+ public static List<Object> constructorFeeder()
{
- return Lists.newArrayList(
- new Object[]{new ForeverBroadcastDistributionRule()},
- new Object[]{new IntervalBroadcastDistributionRule(Intervals.of("0/1000"))},
- new Object[]{new PeriodBroadcastDistributionRule(new Period(1000), null)}
+ return asList(
Review comment:
> [ERROR] /home/travis/build/apache/druid/server/src/test/java/org/apache/druid/server/coordinator/rules/BroadcastDistributionRuleSerdeTest.java:36: Using a static member import should be avoided - java.util.Arrays.asList. [AvoidStaticImport]
One CI failure is because of the static import. Can you use `Arrays.asList()` instead of static import?
##########
File path: server/src/test/java/org/apache/druid/server/coordinator/rules/BroadcastDistributionRuleSerdeTest.java
##########
@@ -19,49 +19,45 @@
package org.apache.druid.server.coordinator.rules;
-import com.fasterxml.jackson.core.type.TypeReference;
import com.fasterxml.jackson.databind.ObjectMapper;
-import com.google.common.collect.Lists;
import org.apache.druid.jackson.DefaultObjectMapper;
import org.apache.druid.java.util.common.Intervals;
import org.joda.time.Period;
import org.junit.Assert;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameter;
+import org.junit.runners.Parameterized.Parameters;
import java.io.IOException;
-import java.util.Collections;
import java.util.List;
+import static java.util.Arrays.asList;
+
@RunWith(Parameterized.class)
public class BroadcastDistributionRuleSerdeTest
{
private static final ObjectMapper MAPPER = new DefaultObjectMapper();
- @Parameterized.Parameters
- public static List<Object[]> constructorFeeder()
+ @Parameters(name ="{0}")
Review comment:
> [ERROR] /home/travis/build/apache/druid/server/src/test/java/org/apache/druid/server/coordinator/rules/BroadcastDistributionRuleSerdeTest.java:43:20: '=' is not followed by whitespace. [WhitespaceAround]
Could you please add a space after `=`?
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] stale[bot] closed pull request #10032: Refurbish BroadcastDistributionRuleSerdeTest
Posted by GitBox <gi...@apache.org>.
stale[bot] closed pull request #10032:
URL: https://github.com/apache/druid/pull/10032
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org