[ 
https://issues.apache.org/jira/browse/OPENNLP-1211?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16578976#comment-16578976
 ] 

ASF GitHub Bot commented on OPENNLP-1211:
-----------------------------------------

kojisekig closed pull request #326: OPENNLP-1211: Improve 
WindowFeatureGeneratorTest
URL: https://github.com/apache/opennlp/pull/326
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

diff --git 
a/opennlp-tools/src/test/java/opennlp/tools/util/featuregen/WindowFeatureGeneratorTest.java
 
b/opennlp-tools/src/test/java/opennlp/tools/util/featuregen/WindowFeatureGeneratorTest.java
index aff43c06b..12af2c8ef 100644
--- 
a/opennlp-tools/src/test/java/opennlp/tools/util/featuregen/WindowFeatureGeneratorTest.java
+++ 
b/opennlp-tools/src/test/java/opennlp/tools/util/featuregen/WindowFeatureGeneratorTest.java
@@ -55,7 +55,7 @@ public void testWithoutWindow() {
 
     Assert.assertEquals(1, features.size());
 
-    Assert.assertEquals(features.get(0), testSentence[testTokenIndex]);
+    Assert.assertEquals("c", features.get(0));
   }
 
   @Test
@@ -68,6 +68,10 @@ public void testWindowSizeOne() {
     windowFeatureGenerator.createFeatures(features, testSentence, 
testTokenIndex, null);
 
     Assert.assertEquals(3, features.size());
+
+    Assert.assertEquals("c", features.get(0));
+    Assert.assertEquals("p1b", features.get(1));
+    Assert.assertEquals("n1d", features.get(2));
   }
 
   @Test
@@ -78,7 +82,7 @@ public void testWindowAtBeginOfSentence() {
     int testTokenIndex = 0;
     windowFeatureGenerator.createFeatures(features, testSentence, 
testTokenIndex, null);
     Assert.assertEquals(1, features.size());
-    Assert.assertEquals(features.get(0), testSentence[testTokenIndex]);
+    Assert.assertEquals("a", features.get(0));
   }
 
   @Test
@@ -89,7 +93,7 @@ public void testWindowAtEndOfSentence() {
     int testTokenIndex = testSentence.length - 1;
     windowFeatureGenerator.createFeatures(features, testSentence, 
testTokenIndex, null);
     Assert.assertEquals(1, features.size());
-    Assert.assertEquals(features.get(0), testSentence[testTokenIndex]);
+    Assert.assertEquals("h", features.get(0));
   }
 
   /**
@@ -104,16 +108,10 @@ public void testForCorrectFeatures() {
     windowFeatureGenerator.createFeatures(features, testSentence, 
testTokenIndex, null);
     Assert.assertEquals(5, features.size());
 
-    Assert.assertTrue(features.contains(WindowFeatureGenerator.PREV_PREFIX + 
"2" +
-        testSentence[testTokenIndex - 2]));
-    Assert.assertTrue(features.contains(WindowFeatureGenerator.PREV_PREFIX + 
"1" +
-        testSentence[testTokenIndex - 1]));
-
-    Assert.assertTrue(features.contains(testSentence[testTokenIndex]));
-
-    Assert.assertTrue(features.contains(WindowFeatureGenerator.NEXT_PREFIX + 
"1" +
-        testSentence[testTokenIndex + 1]));
-    Assert.assertTrue(features.contains(WindowFeatureGenerator.NEXT_PREFIX + 
"2" +
-        testSentence[testTokenIndex + 2]));
+    Assert.assertEquals("d", features.get(0));
+    Assert.assertEquals("p1c", features.get(1));
+    Assert.assertEquals("p2b", features.get(2));
+    Assert.assertEquals("n1e", features.get(3));
+    Assert.assertEquals("n2f", features.get(4));
   }
 }


 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
[email protected]


> Improve WindowFeatureGeneratorTest
> ----------------------------------
>
>                 Key: OPENNLP-1211
>                 URL: https://issues.apache.org/jira/browse/OPENNLP-1211
>             Project: OpenNLP
>          Issue Type: Test
>          Components: Build, Packaging and Test
>    Affects Versions: 1.9.0
>            Reporter: Koji Sekiguchi
>            Priority: Trivial
>             Fix For: 1.9.1
>
>
> I'd like to improve WindowFeatureGeneratorTest from the following perspective:
> * testWindowSizeOne should check the contents of the returned features. It 
> checks the length of the features only now
> * most of test methods uses Assert.assertEquals(expected, actual) in opposite 
> way for its arguments when checking the contents of the returned features
> {code}
> Assert.assertEquals(features.get(0), testSentence[testTokenIndex]);
> {code}
> should be
> {code}
> Assert.assertEquals(testSentence[testTokenIndex], features.get(0));
> {code}
> * Though I pointed out the arguments in assertEquals() above, I think we'd 
> better use exact concrete string rather than expression such like 
> testSentence[testTokenIndex] for the expected. And also, 
> testForCorrectFeatures uses contains method when checking the contents of the 
> returned features but I think we should avoid using contains when checking 
> the items in a List, rather than writing like this:
> {code}
>     Assert.assertTrue(features.contains(WindowFeatureGenerator.PREV_PREFIX + 
> "2" +
>         testSentence[testTokenIndex - 2]));
>     Assert.assertTrue(features.contains(WindowFeatureGenerator.PREV_PREFIX + 
> "1" +
>         testSentence[testTokenIndex - 1]));
>     Assert.assertTrue(features.contains(testSentence[testTokenIndex]));
>     Assert.assertTrue(features.contains(WindowFeatureGenerator.NEXT_PREFIX + 
> "1" +
>         testSentence[testTokenIndex + 1]));
>     Assert.assertTrue(features.contains(WindowFeatureGenerator.NEXT_PREFIX + 
> "2" +
>         testSentence[testTokenIndex + 2]));
> {code}
> but I'd like to rewrite them like this:
> {code}
>     Assert.assertEquals("d",features.get(0));
>     Assert.assertEquals("p1c",features.get(1));
>     Assert.assertEquals("p2b",features.get(2));
>     Assert.assertEquals("n1e",features.get(3));
>     Assert.assertEquals("n2f",features.get(4));
> {code}
> The second form helps us to understand how WindowFeatureGenerator works and 
> it's easier to read.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

Reply via email to