Skip to content

Commit

Permalink
[ES|QL] Simplify function validation tests (elastic#192415)
Browse files Browse the repository at this point in the history
## Summary

This PR changes our approach to testing function validation. Key changes
- We no longer test the definitions from Elasticsearch. Instead, we use
function definition mocks to check the functionality of the validator.
- We no longer run the test scenarios against Elasticsearch to make sure
we are in-sync.

Hopefully this will make the validation tests clearer, remove a point of
friction from the function sync job, and improve our DX. We can consider
any cross-validation we need to do with Elasticsearch as a separate
issue.


### Checklist
- [x]
[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)
was added for features that require explanation or tutorials
- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
  • Loading branch information
drewdaemon committed Sep 10, 2024
1 parent 7c5f64a commit 68234cc
Show file tree
Hide file tree
Showing 16 changed files with 747 additions and 15,999 deletions.
4 changes: 0 additions & 4 deletions .buildkite/scripts/steps/esql_generate_function_metadata.sh
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,6 @@ main () {

yarn make:defs $PARENT_DIR/elasticsearch

report_main_step "Generate function validation tests"

yarn make:tests

report_main_step "Generate inline function docs"

cd "$KIBANA_DIR/$EDITOR_PACKAGE_DIR"
Expand Down
14 changes: 0 additions & 14 deletions packages/kbn-esql-validation-autocomplete/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -284,20 +284,6 @@ It accepts
2. a list of expected errors (can be empty)
3. a list of expected warnings (can be empty or omitted)

The bulk of the validation tests are auto-generated from function definitions. All the generated function tests are found within the following describe block

```ts
describe(FUNCTION_DESCRIBE_BLOCK_NAME, () => {
...
});
```

They are currently generated in CI when a new function definition is added. The generator script is at `packages/kbn-esql-validation-autocomplete/scripts/generate_function_validation_tests.ts`.

The generator can be run locally using the `cd packages/kbn-esql-validation-autocomplete && yarn make:tests`.

It is not perfect and occasionally creates a test case that is invalid for a particular function. So, humans are allowed to edit the expected assertions for any test case—those edits will not be overwritten by the generator script. However, if a human deletes a test case, it will be added back next time the generator runs. So, we should edit the test cases to make them valid, not delete them.

Running the tests in `validation.test.ts` populates `packages/kbn-esql-validation-autocomplete/src/validation/esql_validation_meta_tests.json` which is then used in `test/api_integration/apis/esql/errors.ts` to make sure our validator isn't giving users false positives. Therefore, the validation test suite should always be run after any changes have been made to it so that the JSON file stays in sync.

#### Autocomplete
Expand Down
2 changes: 0 additions & 2 deletions packages/kbn-esql-validation-autocomplete/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,6 @@
"license": "Elastic License 2.0 OR AGPL-3.0-only OR SSPL-1.0",
"sideEffects": false,
"scripts": {
"make:tests": "ts-node --transpileOnly ./scripts/generate_function_validation_tests.ts",
"postmake:tests": "yarn run lint:fix",
"make:defs": "ts-node --transpileOnly ./scripts/generate_function_definitions.ts",
"postmake:defs": "yarn run lint:fix",
"lint:fix": "cd ../.. && node ./scripts/eslint --fix ./packages/kbn-esql-validation-autocomplete/src/**/*.ts",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -194,14 +194,14 @@ const functionEnrichments: Record<string, RecursivePartial<FunctionDefinition>>
date_diff: {
signatures: [
{
params: [{ literalOptions: dateDiffOptions, literalSuggestions: dateDiffSuggestions }],
params: [{ acceptedValues: dateDiffOptions, literalSuggestions: dateDiffSuggestions }],
},
],
},
date_extract: {
signatures: [
{
params: [{ literalOptions: dateExtractOptions }],
params: [{ acceptedValues: dateExtractOptions }],
},
],
},
Expand Down
Loading

0 comments on commit 68234cc

Please sign in to comment.