Skip to content

ESQL: Tools for testing Operators#141778

Merged
nik9000 merged 10 commits intoelastic:mainfrom
nik9000:esql_test_utils
Feb 3, 2026
Merged

ESQL: Tools for testing Operators#141778
nik9000 merged 10 commits intoelastic:mainfrom
nik9000:esql_test_utils

Conversation

@nik9000
Copy link
Copy Markdown
Member

@nik9000 nik9000 commented Feb 3, 2026

We were growing more and more and more options to OperatorTestCase.runDriver. I need another option in #141672 so I built a builder-style test utility. This removes the original methods, migrating all callers.

I've been quite liberal adding utility methods. Those are cheap in a builder-style helper because you don't have to think in terms of combinatorial explosions of parameter - just in terms of "how do I set up all the bits". Now there are ten ways to set the inputs.

It's tempting to make some higher level utility methods that call these - or make the common call sites shorter. You init the most common helper like:

new TestDriverRunner().builder(driverContext())

But I didn't want it to look like magic. Readers should see this and think, "I can add things before builder" and "I can add things to this builder."

@nik9000 nik9000 added >test Issues or PRs that are addressing/adding tests :Analytics/ES|QL AKA ESQL v9.4.0 labels Feb 3, 2026
@elasticsearchmachine elasticsearchmachine added the Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) label Feb 3, 2026
@elasticsearchmachine
Copy link
Copy Markdown
Collaborator

Pinging @elastic/es-analytical-engine (Team:Analytics)

return results;
}

public static void runDriver(Driver driver) {
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I'm running down why we had these duplicate methods. Don't merge this yet.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

It looks like it was created when we built union types. @craigtaverner, do you remember why we have these methods instead of calling the standard utilities? Do we need a longer timeout here? That looks like the only difference.

I think it should be safe to zap that. At worse we'll get some test failure in CI, but we can work through those.

Copy link
Copy Markdown
Member

@dnhatn dnhatn left a comment

Choose a reason for hiding this comment

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

👍

@nik9000 nik9000 enabled auto-merge (squash) February 3, 2026 20:18
@nik9000 nik9000 merged commit 3e24ec3 into elastic:main Feb 3, 2026
35 checks passed
jfreden pushed a commit to jfreden/elasticsearch that referenced this pull request Feb 4, 2026
We were growing more and more and more options to
`OperatorTestCase.runDriver`. I need another option in elastic#141672 so I built
a builder-style test utility. This removes the original methods, migrating
all callers.

I've been quite liberal adding utility methods. Those are cheap in a
builder-style helper because you don't have to think in terms of
combinatorial explosions of parameter - just in terms of "how do I set
up all the bits". Now there are ten ways to set the inputs.

It's tempting to make some higher level utility methods that call these -
or make the common call sites shorter. You init the most common helper like:
```
new TestDriverRunner().builder(driverContext())
```
But I didn't want it to look like magic. Readers should see this and
think, "I can add things before `builder`" and "I can add things to
this `builder`."
spinscale pushed a commit to spinscale/elasticsearch that referenced this pull request Feb 4, 2026
We were growing more and more and more options to
`OperatorTestCase.runDriver`. I need another option in elastic#141672 so I built
a builder-style test utility. This removes the original methods, migrating
all callers.

I've been quite liberal adding utility methods. Those are cheap in a
builder-style helper because you don't have to think in terms of
combinatorial explosions of parameter - just in terms of "how do I set
up all the bits". Now there are ten ways to set the inputs.

It's tempting to make some higher level utility methods that call these -
or make the common call sites shorter. You init the most common helper like:
```
new TestDriverRunner().builder(driverContext())
```
But I didn't want it to look like magic. Readers should see this and
think, "I can add things before `builder`" and "I can add things to
this `builder`."
mamazzol pushed a commit to mamazzol/elasticsearch that referenced this pull request Feb 5, 2026
We were growing more and more and more options to
`OperatorTestCase.runDriver`. I need another option in elastic#141672 so I built
a builder-style test utility. This removes the original methods, migrating
all callers.

I've been quite liberal adding utility methods. Those are cheap in a
builder-style helper because you don't have to think in terms of
combinatorial explosions of parameter - just in terms of "how do I set
up all the bits". Now there are ten ways to set the inputs.

It's tempting to make some higher level utility methods that call these -
or make the common call sites shorter. You init the most common helper like:
```
new TestDriverRunner().builder(driverContext())
```
But I didn't want it to look like magic. Readers should see this and
think, "I can add things before `builder`" and "I can add things to
this `builder`."
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Analytics/ES|QL AKA ESQL Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) >test Issues or PRs that are addressing/adding tests v9.4.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants