From 28a9f68236ef3c196e448784b8c4247dab64a0f2 Mon Sep 17 00:00:00 2001 From: Tom Morris Date: Thu, 2 Jul 2020 14:29:21 -0400 Subject: [PATCH] Unit test improvements (#2856) * Fix two deprecated methods usages * Test ToNumber conversions * Test behavior of all functions when passed 0 or 8 arguments There are 16 which fail currently on 0 args (return null or False instead of EvalError), but have been whitelisted until we can verify whether it's safe to change them without introducing compatibility issues. There are 19 which fail to return an error on too many (ie 8) args. --- .../src/com/google/refine/RefineTest.java | 3 +- .../refine/expr/functions/ToNumberTests.java | 21 ++++++- .../com/google/refine/grel/FunctionTests.java | 58 +++++++++++++++++++ .../refine/importers/JsonImporterTests.java | 2 +- 4 files changed, 81 insertions(+), 3 deletions(-) diff --git a/main/tests/server/src/com/google/refine/RefineTest.java b/main/tests/server/src/com/google/refine/RefineTest.java index af4f27bc6..ccecc9839 100644 --- a/main/tests/server/src/com/google/refine/RefineTest.java +++ b/main/tests/server/src/com/google/refine/RefineTest.java @@ -95,7 +95,8 @@ public class RefineTest extends PowerMockTestCase { FileUtils.writeStringToFile(jsonPath, "{\"projectIDs\":[]\n" + ",\"preferences\":{\"entries\":{\"scripting.starred-expressions\":" + "{\"class\":\"com.google.refine.preference.TopList\",\"top\":2147483647," + - "\"list\":[]},\"scripting.expressions\":{\"class\":\"com.google.refine.preference.TopList\",\"top\":100,\"list\":[]}}}}"); + "\"list\":[]},\"scripting.expressions\":{\"class\":\"com.google.refine.preference.TopList\",\"top\":100,\"list\":[]}}}}", + "UTF-8"); // JSON is always UTF-8 FileProjectManager.initialize(workspaceDir); diff --git a/main/tests/server/src/com/google/refine/expr/functions/ToNumberTests.java b/main/tests/server/src/com/google/refine/expr/functions/ToNumberTests.java index e94ba1dd9..8274392bf 100644 --- a/main/tests/server/src/com/google/refine/expr/functions/ToNumberTests.java +++ b/main/tests/server/src/com/google/refine/expr/functions/ToNumberTests.java @@ -26,16 +26,35 @@ ******************************************************************************/ package com.google.refine.expr.functions; +import static org.testng.Assert.assertEquals; +import static org.testng.Assert.assertTrue; + +import java.util.Properties; + import org.testng.annotations.Test; -import com.google.refine.expr.functions.ToNumber; +import com.google.refine.expr.EvalError; +import com.google.refine.grel.Function; import com.google.refine.util.TestUtils; public class ToNumberTests { + + private static final Double EPSILON = 0.000001; + static Properties bindings = new Properties(); + @Test public void serializeToNumber() { String json = "{\"description\":\"Returns o converted to a number\",\"params\":\"o\",\"returns\":\"number\"}"; TestUtils.isSerializedTo(new ToNumber(), json); } + + @Test + public void testConversions() { + Function f = new ToNumber(); + assertEquals(f.call(bindings, new Object[] {Long.valueOf(11)}), Long.valueOf(11)); + assertEquals(f.call(bindings, new Object[] {"12"}), Long.valueOf(12)); + assertTrue((Double)f.call(bindings, new Object[] {"12345.6789"}) - Double.valueOf(12345.6789) < EPSILON); + assertTrue(f.call(bindings, new Object[] {"abc"}) instanceof EvalError); + } } diff --git a/main/tests/server/src/com/google/refine/grel/FunctionTests.java b/main/tests/server/src/com/google/refine/grel/FunctionTests.java index dfb987d5a..5e671e5c2 100644 --- a/main/tests/server/src/com/google/refine/grel/FunctionTests.java +++ b/main/tests/server/src/com/google/refine/grel/FunctionTests.java @@ -33,8 +33,16 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. package com.google.refine.grel; +import static org.testng.Assert.assertEquals; +import static org.testng.Assert.assertNull; +import static org.testng.Assert.assertTrue; + import java.io.IOException; +import java.util.Arrays; +import java.util.HashSet; +import java.util.Map.Entry; import java.util.Properties; +import java.util.Set; import org.slf4j.LoggerFactory; import org.testng.Assert; @@ -118,4 +126,54 @@ public class FunctionTests extends RefineTest { Assert.assertEquals(invoke("facetCount", new Integer(1), "value", "Column A"),Integer.valueOf(5)); Assert.assertEquals(invoke("facetCount", new Integer(2), "value+1", "Column A"),Integer.valueOf(5)); } + + @Test + void testZeroArgs() { + Set valid0args = new HashSet<>(Arrays.asList("now")); // valid 0-arg returns datetype + // Not sure which, if any, of these are intended, but fixing them may break existing scripts + Set returnsNull = new HashSet<>(Arrays.asList("chomp", "contains", "escape", "unescape", "exp", + "fingerprint", "get", "jsonize", "parseJson", "partition", "pow", "rpartition", + "slice", "substring", // synonyms for Slice + "unicode", "unicodeType" + )); + Set returnsFalse = new HashSet<>(Arrays.asList("hasField")); + + for (Entry entry : ControlFunctionRegistry.getFunctionMapping()) { + Function func = entry.getValue(); + Object result = func.call(bindings, new Object[0]); + if (returnsNull.contains(ControlFunctionRegistry.getFunctionName(func))) { + assertNull(result, ControlFunctionRegistry.getFunctionName(func) + " didn't return null on 0 args"); + } else if (returnsFalse.contains(ControlFunctionRegistry.getFunctionName(func))) { + assertEquals(result, Boolean.FALSE, ControlFunctionRegistry.getFunctionName(func) + " didn't return false on 0 args"); + } else if (!valid0args.contains(ControlFunctionRegistry.getFunctionName(func))) { + assertTrue(result instanceof EvalError, ControlFunctionRegistry.getFunctionName(func) + " didn't error on 0 args"); + } + } + } + + @Test + void testTooManyArgs() { + // Not sure which, if any, of these are intended, but fixing them may break existing scripts + Set returnsNull = new HashSet<>(Arrays.asList("chomp", "contains", "coalesce", "escape", "unescape", + "exp", "fingerprint", "get", "now", "parseJson", "partition", "pow", "rpartition", + "slice", "substring", // synonyms for Slice + "unicode", "unicodeType" + )); + Set returnsFalse = new HashSet<>(Arrays.asList("hasField")); + Set exempt = new HashSet<>(Arrays.asList( + "jsonize" // returns literal string "null" + )); + for (Entry entry : ControlFunctionRegistry.getFunctionMapping()) { + Function func = entry.getValue(); + // No functions take 8 arguments, so they should all error + Object result = func.call(bindings, new Object[] {null, null, null, null, null, null, null, null}); + if (returnsNull.contains(ControlFunctionRegistry.getFunctionName(func))) { + assertNull(result, ControlFunctionRegistry.getFunctionName(func) + " didn't return null on 8 args"); + } else if (returnsFalse.contains(ControlFunctionRegistry.getFunctionName(func))) { + assertEquals(result, Boolean.FALSE, ControlFunctionRegistry.getFunctionName(func) + " didn't return false on 8 args"); + } else if (!exempt.contains(ControlFunctionRegistry.getFunctionName(func))) { + assertTrue(result instanceof EvalError, ControlFunctionRegistry.getFunctionName(func) + " didn't error on 8 args"); + } + } + } } diff --git a/main/tests/server/src/com/google/refine/importers/JsonImporterTests.java b/main/tests/server/src/com/google/refine/importers/JsonImporterTests.java index 65bcdf294..dc6740f41 100644 --- a/main/tests/server/src/com/google/refine/importers/JsonImporterTests.java +++ b/main/tests/server/src/com/google/refine/importers/JsonImporterTests.java @@ -639,7 +639,7 @@ public class JsonImporterTests extends ImporterTest { private String getComplexJSON(String fileName) throws IOException { InputStream in = this.getClass().getClassLoader() .getResourceAsStream(fileName); - String content = org.apache.commons.io.IOUtils.toString(in); + String content = org.apache.commons.io.IOUtils.toString(in, "UTF-8"); return content; }