You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by ho...@apache.org on 2018/11/02 05:06:20 UTC
[2/2] lucene-solr:branch_7x: SOLR-7804: remove problematic assertions
on lossy stats.field values that aren't fundemental to the purpose of the
test anyway
SOLR-7804: remove problematic assertions on lossy stats.field values that aren't fundemental to the purpose of the test anyway
(cherry picked from commit 1e0e8d45e5f30ed08853db1e8a941ef82c6977a4)
Project: http://git-wip-us.apache.org/repos/asf/lucene-solr/repo
Commit: http://git-wip-us.apache.org/repos/asf/lucene-solr/commit/a807cd23
Tree: http://git-wip-us.apache.org/repos/asf/lucene-solr/tree/a807cd23
Diff: http://git-wip-us.apache.org/repos/asf/lucene-solr/diff/a807cd23
Branch: refs/heads/branch_7x
Commit: a807cd234b6a8c148a90c1418ee56d31e872f533
Parents: 1320db3
Author: Chris Hostetter <ho...@apache.org>
Authored: Thu Nov 1 22:03:20 2018 -0700
Committer: Chris Hostetter <ho...@apache.org>
Committed: Thu Nov 1 22:03:44 2018 -0700
----------------------------------------------------------------------
.../apache/solr/cloud/TestCloudPivotFacet.java | 182 ++-----------------
1 file changed, 20 insertions(+), 162 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/a807cd23/solr/core/src/test/org/apache/solr/cloud/TestCloudPivotFacet.java
----------------------------------------------------------------------
diff --git a/solr/core/src/test/org/apache/solr/cloud/TestCloudPivotFacet.java b/solr/core/src/test/org/apache/solr/cloud/TestCloudPivotFacet.java
index adbaf2b..97ae487 100644
--- a/solr/core/src/test/org/apache/solr/cloud/TestCloudPivotFacet.java
+++ b/solr/core/src/test/org/apache/solr/cloud/TestCloudPivotFacet.java
@@ -81,6 +81,11 @@ public class TestCloudPivotFacet extends AbstractFullDistribZkTestBase {
private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+ // because floating point addition can depende on the order of operations, we ignore
+ // any stats that can be lossy -- the purpose of testing stats here is just to sanity check
+ // that the basic hooks between pivot faceting and stats.field work, and these let us do that
+ private static final String USE_STATS = "count=true missing=true min=true max=true";
+
// param used by test purely for tracing & validation
private static String TRACE_MIN = "_test_min";
// param used by test purely for tracing & validation
@@ -110,8 +115,6 @@ public class TestCloudPivotFacet extends AbstractFullDistribZkTestBase {
//commented 2-Aug-2018 @BadApple(bugUrl="https://issues.apache.org/jira/browse/SOLR-12028") // 28-June-2018
public void test() throws Exception {
- sanityCheckAssertNumerics();
-
waitForThingsToLevelOut(30000); // TODO: why would we have to wait?
//
handle.clear();
@@ -141,7 +144,6 @@ public class TestCloudPivotFacet extends AbstractFullDistribZkTestBase {
final String[] fieldNames = fieldNameSet.toArray(new String[fieldNameSet.size()]);
Arrays.sort(fieldNames); // need determinism when picking random fields
-
for (int i = 0; i < 5; i++) {
String q = "*:*";
@@ -161,11 +163,11 @@ public class TestCloudPivotFacet extends AbstractFullDistribZkTestBase {
// if we are doing stats, then always generated the same # of STATS_FIELD
// params, using multiple tags from a fixed set, but with diff fieldName values.
// later, each pivot will randomly pick a tag.
- baseP.add(StatsParams.STATS_FIELD, "{!key=sk1 tag=st1,st2}" +
+ baseP.add(StatsParams.STATS_FIELD, "{!key=sk1 tag=st1,st2 "+USE_STATS+"}" +
pickRandomStatsFields(fieldNames));
- baseP.add(StatsParams.STATS_FIELD, "{!key=sk2 tag=st2,st3}" +
+ baseP.add(StatsParams.STATS_FIELD, "{!key=sk2 tag=st2,st3 "+USE_STATS+"}" +
pickRandomStatsFields(fieldNames));
- baseP.add(StatsParams.STATS_FIELD, "{!key=sk3 tag=st3,st4}" +
+ baseP.add(StatsParams.STATS_FIELD, "{!key=sk3 tag=st3,st4 "+USE_STATS+"}" +
pickRandomStatsFields(fieldNames));
// NOTE: there's a chance that some of those stats field names
// will be the same, but if so, all the better to test that edge case
@@ -387,21 +389,18 @@ public class TestCloudPivotFacet extends AbstractFullDistribZkTestBase {
// regular stats, compare everything...
assert actualStats != null;
- String msg = " of " + statsKey + " => " + message;
-
- // no wiggle room, these should always be exactly equals, regardless of field type
- assertEquals("Count" + msg, pivotStats.getCount(), actualStats.getCount());
- assertEquals("Missing" + msg, pivotStats.getMissing(), actualStats.getMissing());
- assertEquals("Min" + msg, pivotStats.getMin(), actualStats.getMin());
- assertEquals("Max" + msg, pivotStats.getMax(), actualStats.getMax());
-
- // precision loss can affect these in some field types depending on shards used
- // and the order that values are accumulated
- assertNumerics("Sum" + msg, pivotStats.getSum(), actualStats.getSum());
- assertNumerics("Mean" + msg, pivotStats.getMean(), actualStats.getMean());
- assertNumerics("Stddev" + msg, pivotStats.getStddev(), actualStats.getStddev());
- assertNumerics("SumOfSquares" + msg,
- pivotStats.getSumOfSquares(), actualStats.getSumOfSquares());
+ try {
+ String msg = " of " + statsKey;
+
+ // no wiggle room, these should always be exactly equals, regardless of field type
+ assertEquals("Count" + msg, pivotStats.getCount(), actualStats.getCount());
+ assertEquals("Missing" + msg, pivotStats.getMissing(), actualStats.getMissing());
+ assertEquals("Min" + msg, pivotStats.getMin(), actualStats.getMin());
+ assertEquals("Max" + msg, pivotStats.getMax(), actualStats.getMax());
+
+ } catch (AssertionError e) {
+ throw new AssertionError("Stats: Pivot[" + pivotStats + "] <==> Actual[" + actualStats + "] => " + message, e);
+ }
}
}
@@ -691,147 +690,6 @@ public class TestCloudPivotFacet extends AbstractFullDistribZkTestBase {
}
/**
- * Given two objects returned as stat values asserts that they are they are either both <code>null</code>
- * or all of the following are true:
- * <ul>
- * <li>They have the exact same class</li>
- * <li>They are both Numbers or they are both Dates -- in the later case, their millisecond's
- * since epoch are used for all subsequent comparisons
- * </li>
- * <li>Either:
- * <ul>
- * <li>They are Integer or Long objects with the exact same <code>longValue()</code></li>
- * <li>They are Float or Double objects and their <code>doubleValue()</code>s
- * are equally-ish with a "small" epsilon (relative to the scale of the expected value)
- * </li>
- * </ul>
- * </li>
- * <ul>
- *
- * @see Date#getTime
- * @see Number#doubleValue
- * @see Number#longValue
- * @see #assertEquals(String,double,double,double)
- */
- private void assertNumerics(String msg, Object expected, Object actual) {
- if (null == expected || null == actual) {
- assertEquals(msg, expected, actual);
- return;
- }
-
- assertEquals(msg + " ... values do not have the same type: " + expected + " vs " + actual,
- expected.getClass(), actual.getClass());
-
- if (expected instanceof Date) {
- expected = ((Date)expected).getTime();
- actual = ((Date)actual).getTime();
- msg = msg + " (w/dates converted to ms)";
- }
-
- assertTrue(msg + " ... expected is not a Number: " +
- expected + "=>" + expected.getClass(),
- expected instanceof Number);
-
- if (expected instanceof Long || expected instanceof Integer) {
- assertEquals(msg, ((Number)expected).longValue(), ((Number)actual).longValue());
-
- } else if (expected instanceof Float || expected instanceof Double) {
- // compute an epsilon relative to the size of the expected value
- double expect = ((Number)expected).doubleValue();
- double epsilon = Math.abs(expect * 0.1E-7D);
-
- assertEquals(msg, expect, ((Number)actual).doubleValue(), epsilon);
-
- } else {
- fail(msg + " ... where did this come from: " + expected.getClass());
- }
- }
-
- /**
- * test the test
- */
- private void sanityCheckAssertNumerics() {
-
- assertNumerics("Null?", null, null);
- assertNumerics("large a",
- 2.3005390038169265E9,
- 2.300539003816927E9);
- assertNumerics("large b",
- 1.2722582464444444E9,
- 1.2722582464444442E9);
- assertNumerics("small",
- 2.3005390038169265E-9,
- 2.300539003816927E-9);
-
- assertNumerics("large a negative",
- -2.3005390038169265E9,
- -2.300539003816927E9);
- assertNumerics("large b negative",
- -1.2722582464444444E9,
- -1.2722582464444442E9);
- assertNumerics("small negative",
- -2.3005390038169265E-9,
- -2.300539003816927E-9);
-
- assertNumerics("high long", Long.MAX_VALUE, Long.MAX_VALUE);
- assertNumerics("high int", Integer.MAX_VALUE, Integer.MAX_VALUE);
- assertNumerics("low long", Long.MIN_VALUE, Long.MIN_VALUE);
- assertNumerics("low int", Integer.MIN_VALUE, Integer.MIN_VALUE);
-
- // NOTE: can't use 'fail' in these try blocks, because we are catching AssertionError
- // (ie: the code we are expecting to 'fail' is an actual test assertion generator)
-
- for (Object num : new Object[] { new Date(42), 42, 42L, 42.0F }) {
- try {
- assertNumerics("non-null", null, num);
- throw new RuntimeException("did not get assertion failure when expected was null");
- } catch (AssertionError e) {}
-
- try {
- assertNumerics("non-null", num, null);
- throw new RuntimeException("did not get assertion failure when actual was null");
- } catch (AssertionError e) {}
- }
-
- try {
- assertNumerics("non-number", "foo", 42);
- throw new RuntimeException("did not get assertion failure when expected was non-number");
- } catch (AssertionError e) {}
-
- try {
- assertNumerics("non-number", 42, "foo");
- throw new RuntimeException("did not get assertion failure when actual was non-number");
- } catch (AssertionError e) {}
-
- try {
- assertNumerics("diff",
- 2.3005390038169265E9,
- 2.267272520100462E9);
- throw new RuntimeException("did not get assertion failure when args are big & too diff");
- } catch (AssertionError e) {}
- try {
- assertNumerics("diff",
- 2.3005390038169265E-9,
- 2.267272520100462E-9);
- throw new RuntimeException("did not get assertion failure when args are small & too diff");
- } catch (AssertionError e) {}
-
- try {
- assertNumerics("diff long", Long.MAX_VALUE, Long.MAX_VALUE-1);
- throw new RuntimeException("did not get assertion failure when args are diff longs");
- } catch (AssertionError e) {}
- try {
- assertNumerics("diff int", Integer.MAX_VALUE, Integer.MAX_VALUE-1);
- throw new RuntimeException("did not get assertion failure when args are diff ints");
- } catch (AssertionError e) {}
- try {
- assertNumerics("diff date", new Date(42), new Date(43));
- throw new RuntimeException("did not get assertion failure when args are diff dates");
- } catch (AssertionError e) {}
-
- }
-
- /**
* @see #assertNumFound
* @see #assertPivotCountsAreCorrect(SolrParams,SolrParams)
*/