Uploaded image for project: 'Solr'
  1. Solr
  2. SOLR-2451

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

    Details

    • Type: Improvement
    • Status: Closed
    • Priority: 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
          hossman 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
          hossman 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
          hossman Hoss Man added a comment -

          sorry, last patch was stale

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

          Ok, I like it.

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

          Committed revision 1102907.

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

          thanks for bringing this up david

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

          forgot i was in the middle of backporting

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

          Committed revision 1102910. - 3x

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

          Bulk close for 3.2

          Show
          rcmuir Robert Muir added a comment - Bulk close for 3.2
          Hide
          dsmiley 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
          dsmiley 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
          hossman 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
          hossman 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:
              hossman Hoss Man
              Reporter:
              dsmiley David Smiley
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development