[TEXT-170] Add String lookup for host names and IP addresses by garydgregory · Pull Request #118 · apache/commons-text

garydgregory · 2019-07-27T20:30:21Z

Adds DnsStringLookup.

(DnsStringLookup).

src/test/java/org/apache/commons/text/lookup/DnsStringLookupTest.java

michael-o · 2019-07-27T20:37:09Z

src/test/java/org/apache/commons/text/lookup/DnsStringLookupTest.java

I see these tests as problematic because they require network connection or you rely on the fact that DNS entries are correct. You'd actually need a mock DNS server.

A mock DNS server is over the top here IMO for what is essentially a JRE API facade.

That said, I won't stock you from scratching that itch ;-) But then what about the rest of Commons?

I know that this is over the top, but it would make the tests reliable.

pom.xml

(DnsStringLookup). Do not use hardcoded IP addresses.

PascalSchumacher · 2019-07-28T08:30:14Z

Hi Gary,

please fix the checkstyle violations:

[INFO] There are 4 errors reported by Checkstyle 8.21 with /home/travis/build/apache/commons-text/checkstyle.xml ruleset.
[ERROR] src/main/java/org/apache/commons/text/lookup/InetAddressKeys.java:[22,5] (javadoc) JavadocVariable: Missing a Javadoc comment.
[ERROR] src/main/java/org/apache/commons/text/lookup/InetAddressKeys.java:[23,5] (javadoc) JavadocVariable: Missing a Javadoc comment.
[ERROR] src/main/java/org/apache/commons/text/lookup/InetAddressKeys.java:[24,5] (javadoc) JavadocVariable: Missing a Javadoc comment.
[ERROR] src/main/java/org/apache/commons/text/lookup/DefaultStringLookup.java:[53] (regexp) RegexpSingleline: Line has trailing spaces.

see: https://travis-ci.org/apache/commons-text/jobs/564503192

Thank you very much!

(DnsStringLookup). Do not use hardcoded IP addresses.

(DnsStringLookup). Fix Checkstyle issues.

coveralls · 2019-07-28T13:00:58Z

Coverage decreased (-0.01%) to 97.98% when pulling a2cd312 on garydgregory:text-170 into 4e8197f on apache:master.

src/main/java/org/apache/commons/text/lookup/StringLookupFactory.java

src/test/java/org/apache/commons/text/StringSubstitutorWithInterpolatorStringLookupTest.java

michael-o · 2019-07-28T18:09:50Z

src/test/java/org/apache/commons/text/lookup/DnsStringLookupTest.java

+    public void testCanonicalNameFromHostAddress() throws UnknownHostException {
+        final InetAddress localHost = InetAddress.getLocalHost();
+        Assertions.assertEquals(localHost.getCanonicalHostName(),
+            DnsStringLookup.INSTANCE.lookup("canonical-name|" + localHost.getHostAddress()));


Is that really always true?

Easier said than done:
Mockito.spy(InetAddress.getLocalHost());
Yields:

org.mockito.exceptions.base.MockitoException: Cannot mock/spy class java.net.Inet4Address Mockito cannot mock/spy because : - final class at org.apache.commons.text.lookup.DnsStringLookupTest.testAddressFromHostAddress(DnsStringLookupTest.java:34) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at java.lang.reflect.Method.invoke(Method.java:498) at org.junit.platform.commons.util.ReflectionUtils.invokeMethod(ReflectionUtils.java:675) at org.junit.jupiter.engine.execution.MethodInvocation.proceed(MethodInvocation.java:60) at org.junit.jupiter.engine.execution.InvocationInterceptorChain$ValidatingInvocation.proceed(InvocationInterceptorChain.java:125) at org.junit.jupiter.engine.extension.TimeoutExtension.intercept(TimeoutExtension.java:132) at org.junit.jupiter.engine.extension.TimeoutExtension.interceptTestableMethod(TimeoutExtension.java:124) at org.junit.jupiter.engine.extension.TimeoutExtension.interceptTestMethod(TimeoutExtension.java:74) at org.junit.jupiter.engine.execution.ExecutableInvoker$ReflectiveInterceptorCall.lambda$ofVoidMethod$0(ExecutableInvoker.java:115) at org.junit.jupiter.engine.execution.ExecutableInvoker.lambda$invoke$0(ExecutableInvoker.java:105) at org.junit.jupiter.engine.execution.InvocationInterceptorChain$InterceptedInvocation.proceed(InvocationInterceptorChain.java:104) at org.junit.jupiter.engine.execution.InvocationInterceptorChain.proceed(InvocationInterceptorChain.java:62) at org.junit.jupiter.engine.execution.InvocationInterceptorChain.chainAndInvoke(InvocationInterceptorChain.java:43) at org.junit.jupiter.engine.execution.InvocationInterceptorChain.invoke(InvocationInterceptorChain.java:35) at org.junit.jupiter.engine.execution.ExecutableInvoker.invoke(ExecutableInvoker.java:104) at org.junit.jupiter.engine.execution.ExecutableInvoker.invoke(ExecutableInvoker.java:98) at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.lambda$invokeTestMethod$6(TestMethodTestDescriptor.java:202) at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73) at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.invokeTestMethod(TestMethodTestDescriptor.java:198) at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.execute(TestMethodTestDescriptor.java:135) at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.execute(TestMethodTestDescriptor.java:69) at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$5(NodeTestTask.java:135) at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73) at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$7(NodeTestTask.java:125) at org.junit.platform.engine.support.hierarchical.Node.around(Node.java:135) at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$8(NodeTestTask.java:123) at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73) at org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:122) at org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:80) at java.util.ArrayList.forEach(ArrayList.java:1257) at org.junit.platform.engine.support.hierarchical.SameThreadHierarchicalTestExecutorService.invokeAll(SameThreadHierarchicalTestExecutorService.java:38) at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$5(NodeTestTask.java:139) at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73) at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$7(NodeTestTask.java:125) at org.junit.platform.engine.support.hierarchical.Node.around(Node.java:135) at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$8(NodeTestTask.java:123) at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73) at org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:122) at org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:80) at java.util.ArrayList.forEach(ArrayList.java:1257) at org.junit.platform.engine.support.hierarchical.SameThreadHierarchicalTestExecutorService.invokeAll(SameThreadHierarchicalTestExecutorService.java:38) at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$5(NodeTestTask.java:139) at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73) at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$7(NodeTestTask.java:125) at org.junit.platform.engine.support.hierarchical.Node.around(Node.java:135) at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$8(NodeTestTask.java:123) at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73) at org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:122) at org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:80) at org.junit.platform.engine.support.hierarchical.SameThreadHierarchicalTestExecutorService.submit(SameThreadHierarchicalTestExecutorService.java:32) at org.junit.platform.engine.support.hierarchical.HierarchicalTestExecutor.execute(HierarchicalTestExecutor.java:57) at org.junit.platform.engine.support.hierarchical.HierarchicalTestEngine.execute(HierarchicalTestEngine.java:51) at org.junit.platform.launcher.core.DefaultLauncher.execute(DefaultLauncher.java:229) at org.junit.platform.launcher.core.DefaultLauncher.lambda$execute$6(DefaultLauncher.java:197) at org.junit.platform.launcher.core.DefaultLauncher.withInterceptedStreams(DefaultLauncher.java:211) at org.junit.platform.launcher.core.DefaultLauncher.execute(DefaultLauncher.java:191) at org.junit.platform.launcher.core.DefaultLauncher.execute(DefaultLauncher.java:137) at org.eclipse.jdt.internal.junit5.runner.JUnit5TestReference.run(JUnit5TestReference.java:89) at org.eclipse.jdt.internal.junit.runner.TestExecution.run(TestExecution.java:41) at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:541) at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:763) at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.run(RemoteTestRunner.java:463) at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.main(RemoteTestRunner.java:209)

I think we should go as is unless you can come up with Mocking that works...

michael-o · 2019-07-28T18:09:59Z

src/test/java/org/apache/commons/text/lookup/DnsStringLookupTest.java

+    public void testCanonicalNameFromHostName() throws UnknownHostException {
+        final InetAddress localHost = InetAddress.getLocalHost();
+        Assertions.assertEquals(localHost.getCanonicalHostName(),
+            DnsStringLookup.INSTANCE.lookup("canonical-name|" + localHost.getHostName()));


This test look OK to me, it testing converting the host name to its canonical version. Or am I missing something?

How do you know that the hostname and canonical hostname are the same?

(DnsStringLookup). Update test per PR review.

Merge remote-tracking branch 'origin/master'.

Make test more reliable.

[TEXT-170] Add String lookup for host names and IP addresses

b46ea74

(DnsStringLookup).

michael-o requested changes Jul 27, 2019

View reviewed changes

ggregory-rocket added 2 commits July 27, 2019 19:42

Merge remote-tracking branch 'upstream/master' into text-170

b93c929

[TEXT-170] Add String lookup for host names and IP addresses

c15f580

(DnsStringLookup). Do not use hardcoded IP addresses.

ggregory-rocket added 2 commits July 28, 2019 08:51

[TEXT-170] Add String lookup for host names and IP addresses

f245ec8

(DnsStringLookup). Do not use hardcoded IP addresses.

[TEXT-170] Add String lookup for host names and IP addresses

3e96741

(DnsStringLookup). Fix Checkstyle issues.

michael-o reviewed Jul 28, 2019

View reviewed changes

ggregory-rocket added 5 commits July 28, 2019 18:31

Fix version number in Javadoc.

ca839d5

Update Javadoc per PR review.

177ef4b

[TEXT-170] Add String lookup for host names and IP addresses

7dd142b

(DnsStringLookup). Update test per PR review.

[TEXT-170] Add String lookup for host names and IP addresses

52849a9

(DnsStringLookup). Update test per PR review.

Merge remote-tracking branch 'origin/master' into text-170

a2cd312

garydgregory merged commit d81879e into apache:master Aug 4, 2019

garydgregory deleted the text-170 branch August 4, 2019 15:27

asfgit pushed a commit that referenced this pull request Aug 4, 2019

[TEXT-170] Add String lookup for host names and IP addresses (#118)

3ba2268

Merge remote-tracking branch 'origin/master'.

asfgit pushed a commit that referenced this pull request Aug 4, 2019

[TEXT-170] Add String lookup for host names and IP addresses (#118)

8f720f2

Make test more reliable.

Conversation

garydgregory commented Jul 27, 2019

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

PascalSchumacher commented Jul 28, 2019

Uh oh!

coveralls commented Jul 28, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Comments

coveralls commented Jul 28, 2019 •

edited

Loading