Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding Match Query Unit Tests #56

Merged
merged 6 commits into from
May 18, 2022
Merged

Adding Match Query Unit Tests #56

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
b5f645b
Added unit tests for match function implementation and added check to…
forestmvey May 13, 2022
cd518a0
Add missing copyright to MatchQueryTest
forestmvey May 13, 2022
dc5172e
Fix build not passing checkstyle syntax
forestmvey May 16, 2022
c54a11f
Made invalid function implementations intentions more clear by throwi…
forestmvey May 16, 2022
34d395c
Replaced modular tests for valid arguments with iterative substitute.…
forestmvey May 17, 2022
4fa2578
Reworking valid tests to parameterized tests
forestmvey May 18, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions ...java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/MatchQuery.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,9 @@ public class MatchQuery extends LuceneQuery {

@Override
public QueryBuilder build(FunctionExpression func) {
if (func.getArguments().size() < 2) {
acarbonetto marked this conversation as resolved.
Show resolved Hide resolved
throw new SemanticCheckException("match must have at least two arguments");
}
Iterator<Expression> iterator = func.getArguments().iterator();
NamedArgumentExpression field = (NamedArgumentExpression) iterator.next();
NamedArgumentExpression query = (NamedArgumentExpression) iterator.next();
Expand Down
157 changes: 157 additions & 0 deletions .../test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/MatchQueryTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,157 @@
/*
* Copyright OpenSearch Contributors
* SPDX-License-Identifier: Apache-2.0
*/

package org.opensearch.sql.opensearch.storage.script.filter.lucene;

import static org.junit.jupiter.api.Assertions.assertThrows;

import java.util.List;
import java.util.stream.Stream;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.DisplayNameGeneration;
import org.junit.jupiter.api.DisplayNameGenerator;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.MethodSource;
import org.opensearch.sql.data.model.ExprValue;
import org.opensearch.sql.data.type.ExprType;
import org.opensearch.sql.exception.SemanticCheckException;
import org.opensearch.sql.expression.DSL;
import org.opensearch.sql.expression.Expression;
import org.opensearch.sql.expression.FunctionExpression;
import org.opensearch.sql.expression.NamedArgumentExpression;
import org.opensearch.sql.expression.config.ExpressionConfig;
import org.opensearch.sql.expression.env.Environment;
import org.opensearch.sql.expression.function.FunctionName;
import org.opensearch.sql.opensearch.storage.script.filter.lucene.relevance.MatchQuery;

@DisplayNameGeneration(DisplayNameGenerator.ReplaceUnderscores.class)
public class MatchQueryTest {
private final DSL dsl = new ExpressionConfig().dsl(new ExpressionConfig().functionRepository());
private final MatchQuery matchQuery = new MatchQuery();
private final FunctionName match = FunctionName.of("match");

static Stream<List<Expression>> generateValidData() {
final DSL dsl = new ExpressionConfig().dsl(new ExpressionConfig().functionRepository());
return Stream.of(
Copy link

Choose a reason for hiding this comment

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

Looks good to me.

Another option is to take the list of valid args and build the stream of test cases from it.

Something like:

NamedArgument field = dsl.namedArgument("field", DSL.literal("field_value"));
NamedArgument query = dsl.namedArgument("query", DSL.literal("query_value"));
return List.of(
        dsl.namedArgument("analyzer", "standard"),
        dsl.namedArgument("auto_generate_synonyms_phrase_query", "true"),
        dsl.namedArgument("fuzziness", "AUTO"),
        dsl.namedArgument("max_expansions", "50"),
        dsl.namedArgument("prefix_length", "0"),
        dsl.namedArgument("fuzzy_transpositions", "true"),
        dsl.namedArgument("fuzzy_rewrite", "constant_score"),
        dsl.namedArgument("lenient", "false"),
        dsl.namedArgument("operator", "OR"),
        dsl.namedArgument("minimum_should_match", "3"),
        dsl.namedArgument("zero_terms_query", "NONE"),
        dsl.namedArgument("boost", "1"))
            .stream().map(arg -> List.of(field, query, arg));

build_succeeds_with_two_arguments would need to be kept.

List.of(
dsl.namedArgument("field", DSL.literal("field_value")),
dsl.namedArgument("query", DSL.literal("query_value"))
Copy link

Choose a reason for hiding this comment

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

you could create

NamedArgument fieldArgument = dsl.namedArgument("field", DSL.literal("field_value"));
NamedArgument queryArgument = dsl.namedArgument("query", DSL.literal("query_value"));

once and pass it to all these lists. If you make it static and global to the class, you can use it in other tests ;)

),
List.of(
dsl.namedArgument("field", DSL.literal("field_value")),
dsl.namedArgument("query", DSL.literal("query_value")),
dsl.namedArgument("analyzer", DSL.literal("standard"))
),
List.of(
dsl.namedArgument("field", DSL.literal("field_value")),
dsl.namedArgument("query", DSL.literal("query_value")),
dsl.namedArgument("auto_generate_synonyms_phrase_query", DSL.literal("true"))
),
List.of(
dsl.namedArgument("field", DSL.literal("field_value")),
dsl.namedArgument("query", DSL.literal("query_value")),
dsl.namedArgument("fuzziness", DSL.literal("AUTO"))
),
List.of(
dsl.namedArgument("field", DSL.literal("field_value")),
dsl.namedArgument("query", DSL.literal("query_value")),
dsl.namedArgument("max_expansions", DSL.literal("50"))
),
List.of(
dsl.namedArgument("field", DSL.literal("field_value")),
dsl.namedArgument("query", DSL.literal("query_value")),
dsl.namedArgument("prefix_length", DSL.literal("0"))
),
List.of(
dsl.namedArgument("field", DSL.literal("field_value")),
dsl.namedArgument("query", DSL.literal("query_value")),
dsl.namedArgument("fuzzy_transpositions", DSL.literal("true"))
),
List.of(
dsl.namedArgument("field", DSL.literal("field_value")),
dsl.namedArgument("query", DSL.literal("query_value")),
dsl.namedArgument("fuzzy_rewrite", DSL.literal("constant_score"))
),
List.of(
dsl.namedArgument("field", DSL.literal("field_value")),
dsl.namedArgument("query", DSL.literal("query_value")),
dsl.namedArgument("lenient", DSL.literal("false"))
),
List.of(
dsl.namedArgument("field", DSL.literal("field_value")),
dsl.namedArgument("query", DSL.literal("query_value")),
dsl.namedArgument("operator", DSL.literal("OR"))
),
List.of(
dsl.namedArgument("field", DSL.literal("field_value")),
dsl.namedArgument("query", DSL.literal("query_value")),
dsl.namedArgument("minimum_should_match", DSL.literal("3"))
),
List.of(
dsl.namedArgument("field", DSL.literal("field_value")),
dsl.namedArgument("query", DSL.literal("query_value")),
dsl.namedArgument("zero_terms_query", DSL.literal("NONE"))
),
List.of(
dsl.namedArgument("field", DSL.literal("field_value")),
dsl.namedArgument("query", DSL.literal("query_value")),
dsl.namedArgument("boost", DSL.literal("1"))
)
);
}

@ParameterizedTest
@MethodSource("generateValidData")
public void test_valid_parameters(List<Expression> validArgs) {
Assertions.assertNotNull(matchQuery.build(new MatchExpression(validArgs)));
}

@Test
public void test_SemanticCheckException_when_no_arguments() {
List<Expression> arguments = List.of();
assertThrows(SemanticCheckException.class,
() -> matchQuery.build(new MatchExpression(arguments)));
}

@Test
public void test_SemanticCheckException_when_one_argument() {
List<Expression> arguments = List.of(namedArgument("field", "field_value"));
assertThrows(SemanticCheckException.class,
() -> matchQuery.build(new MatchExpression(arguments)));
}

@Test
public void test_SemanticCheckException_when_invalid_parameter() {
List<Expression> arguments = List.of(
namedArgument("field", "field_value"),
namedArgument("query", "query_value"),
namedArgument("unsupported", "unsupported_value"));
Assertions.assertThrows(SemanticCheckException.class,
() -> matchQuery.build(new MatchExpression(arguments)));
}

private NamedArgumentExpression namedArgument(String name, String value) {
return dsl.namedArgument(name, DSL.literal(value));
}

private class MatchExpression extends FunctionExpression {
MaxKsyunz marked this conversation as resolved.
Show resolved Hide resolved
public MatchExpression(List<Expression> arguments) {
super(MatchQueryTest.this.match, arguments);
}

@Override
public ExprValue valueOf(Environment<Expression, ExprValue> valueEnv) {
MaxKsyunz marked this conversation as resolved.
Show resolved Hide resolved
throw new UnsupportedOperationException("Invalid function call, "
+ "valueOf function need implementation only to support Expression interface");
}

@Override
public ExprType type() {
MaxKsyunz marked this conversation as resolved.
Show resolved Hide resolved
throw new UnsupportedOperationException("Invalid function call, "
+ "type function need implementation only to support Expression interface");
}
}
}