Skip to content
This repository has been archived by the owner on Aug 2, 2022. It is now read-only.

Enforce comparison test in new SQL module #536

Conversation

dai-chen
Copy link
Member

Issue #, if available: #535

Description of changes: Last year we developed a test framework which verifies result by comparing with other SQL implementation. It was integrated with Gradle task to automate the testing. However, due to problems in old codebase, it was only used for reporting instead of being mandatory for plugin build. Now as we start shift to new query engine, the success of comparison test results should be enforced. Reference: https://github.com/opendistro-for-elasticsearch/sql/blob/develop/docs/dev/Testing.md.

  1. Add SQL test base class and a new IT class SQLCorrectnessIT to read all queries in specific folder.
  2. Improve test framework by explaining the difference that caused the test case failure.

For now test case file is simply a txt file organized in folders under src/resources/correctness in integ-test module. There are 3 categories:

  1. expressions: test cases for all expressions such as arithmetic, predicate and function expressions.
  2. queries: test cases for different queries such as SELECT-FROM-WHERE, GROUP BY, JOINs etc.
  3. bugfixes: test cases for bug fix to make sure no regression in future release.
$ tree -L 2 -I "resources" integ-test/src/test/resources/correctness
integ-test/src/test/resources/correctness
├── bugfixes
│   └── 521.txt
├── expressions
│   ├── arithmetics.txt
│   ├── functions.txt
│   ├── literals.txt
│   └── predicates.txt
├── queries
│   ├── groupby.txt
│   ├── joins.txt
│   ├── select.txt
│   └── subqueries.txt
├── kibana_sample_data_ecommerce.csv
├── kibana_sample_data_ecommerce.json
├── kibana_sample_data_flights.csv
└── kibana_sample_data_flights.json

For example, for failed test case, a new explain field is added to describe which column type or data row is different.

{
   "summary":{
      "total":1,
      "success":0,
      "failure":1
   },
   "tests":[
      {
         "result":"Failed",
         "explain":"Data row at [0] is different: this=[Row(values=[hello])], other=[Row(values=[world])]",
         "resultSets":[
            {
               "schema":[
                  {
                     "name":"firstName",
                     "type":"text"
                  }
               ],
               "dataRows":[
                  [
                     "hello"
                  ]
               ],
               "database":"Elasticsearch"
            },
            {
               "schema":[
                  {
                     "name":"firstName",
                     "type":"text"
                  }
               ],
               "dataRows":[
                  [
                     "world"
                  ]
               ],
               "database":"H2"
            }
         ],
         "id":1,
         "sql":"SELECT * FROM accounts"
      }
   ]
}

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@dai-chen dai-chen added maintenance Improves code quality, but not the product SQL labels Jun 29, 2020
@dai-chen dai-chen self-assigned this Jun 29, 2020
@dai-chen dai-chen marked this pull request as ready for review June 29, 2020 16:59
@dai-chen dai-chen requested review from chloe-zh and penghuo June 29, 2020 17:00
@dai-chen dai-chen changed the title Enforce comparison test merged Enforce comparison test in new SQL module Jun 30, 2020
Copy link
Contributor

@penghuo penghuo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

Copy link
Member

@chloe-zh chloe-zh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@dai-chen dai-chen merged commit d60997b into opendistro-for-elasticsearch:develop Jul 1, 2020
@dai-chen dai-chen deleted the enforce-comparison-test-merged branch July 1, 2020 16:44
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
maintenance Improves code quality, but not the product SQL
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants