Solr
  1. Solr
  2. SOLR-2451

Enhance SolrTestCaseJ4 to allow tests to account for small deltas when comparing floats/doubles

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 3.2, 4.0-ALPHA
    • Component/s: None
    • Labels:
      None

      Description

      Attached is a patch that adds the following method to SolrTestCaseJ4: (just javadoc & signature shown)

        /**
         * Validates that the document at the specified index in the results has the specified score, within 0.0001.
         */
        public static void assertQScore(SolrQueryRequest req, int docIdx, float targetScore) {
      

      This is especially useful for geospatial in which slightly different precision deltas might occur when trying different geospatial indexing strategies are used, assuming the score is some geospatial distance. This patch makes a simple modification to DistanceFunctionTest to use it.

      1. Fix_SOLR-2451_for_a_custom_delta.patch
        2 kB
        David Smiley
      2. SOLR-2451_assertQScore.patch
        5 kB
        David Smiley
      3. SOLR-2451.patch
        11 kB
        Hoss Man
      4. SOLR-2451.patch
        10 kB
        Hoss Man

        Issue Links

          Activity

          Hide
          Hoss Man added a comment -

          David,

          One concern i have with your impl is that really only works with simple score comparisons from the main result set – for a public api we should probably try to be more general (as is this wouldn't work if people wanted flexible score comparisons using group by for instance – let alone any custom plugins users might want to write tests for)

          The underlying code that processes assertJQ already applies a tolerance level when doing equality tests between the JSON structure and the expected value, but that is currently hardcoded.

          Here's a patch bubbles that tollerance up that up so that it can be specified in the individual assertJQ calls. What do you think of this approach?

          Show
          Hoss Man added a comment - David, One concern i have with your impl is that really only works with simple score comparisons from the main result set – for a public api we should probably try to be more general (as is this wouldn't work if people wanted flexible score comparisons using group by for instance – let alone any custom plugins users might want to write tests for) The underlying code that processes assertJQ already applies a tolerance level when doing equality tests between the JSON structure and the expected value, but that is currently hardcoded. Here's a patch bubbles that tollerance up that up so that it can be specified in the individual assertJQ calls. What do you think of this approach?
          Hide
          Hoss Man added a comment -

          sorry, last patch was stale

          Show
          Hoss Man added a comment - sorry, last patch was stale
          Hide
          David Smiley added a comment -

          Ok, I like it.

          Show
          David Smiley added a comment - Ok, I like it.
          Hide
          Hoss Man added a comment -

          Committed revision 1102907.

          Show
          Hoss Man added a comment - Committed revision 1102907.
          Hide
          Hoss Man added a comment -

          thanks for bringing this up david

          Show
          Hoss Man added a comment - thanks for bringing this up david
          Hide
          Hoss Man added a comment -

          forgot i was in the middle of backporting

          Show
          Hoss Man added a comment - forgot i was in the middle of backporting
          Hide
          Hoss Man added a comment -

          Committed revision 1102910. - 3x

          Show
          Hoss Man added a comment - Committed revision 1102910. - 3x
          Hide
          Robert Muir added a comment -

          Bulk close for 3.2

          Show
          Robert Muir added a comment - Bulk close for 3.2
          Hide
          David Smiley added a comment -

          Delta support was added bit it wasn't completed. In particular, a custom delta provided is ignored, and instead you get the one provided as the default. Furthermore, the condition in which delta is actually looked at wasn't quite right with respect to the Number subclass of "val" and "expected" subclasses. In a minute I will attach a patch fixing it.

          Show
          David Smiley added a comment - Delta support was added bit it wasn't completed. In particular, a custom delta provided is ignored, and instead you get the one provided as the default. Furthermore, the condition in which delta is actually looked at wasn't quite right with respect to the Number subclass of "val" and "expected" subclasses. In a minute I will attach a patch fixing it.
          Hide
          Hoss Man added a comment -

          David: sorry, can't beleive we missed this before.

          Since this issue is already recorded in the changes of a released version, I opened a new issue (SOLR-3024) to track fixing it.

          Show
          Hoss Man added a comment - David: sorry, can't beleive we missed this before. Since this issue is already recorded in the changes of a released version, I opened a new issue ( SOLR-3024 ) to track fixing it.

            People

            • Assignee:
              Hoss Man
              Reporter:
              David Smiley
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development