Ensure requested capability exists#142695
Conversation
idegtiarenko
added
>test
|
Pinging @elastic/es-analytical-engine (Team:Analytics) |
luigidellaquila
left a comment
luigidellaquila
left a comment
There was a problem hiding this comment.
Thanks @idegtiarenko, I think it makes a lot of sense to have a check for typos.
About the -Ignore, that's an acceptable option. An alternative could be to actually declare the capabilities and disable them, eg.
RERANK_COMBINE(false),
In both cases, it would be good to have a comment (on all the ignored tests if we use -Ignore or on the capability if we decide for that option) that explains why it's disabled.
| combine | ||
| combine-Ignore | ||
| required_capability: rerank | ||
| required_capability: rerank_combine |
There was a problem hiding this comment.
@nik9000 in case you remember, was this introduced intentionally?
There was a problem hiding this comment.
I have no memory of this, sorry.
There was a problem hiding this comment.
@afoucret, it looks like these weren't being run and I think I broke them. You own this function, right? Could you get these running?
@idegtiarenko, I think you should merge this.
| count_over_time_of_date_nanos_promql | ||
| count_over_time_of_date_nanos_promql-Ignore | ||
| required_capability: promql_command_v0 | ||
| required_capability: promql_date_nanos_support_v0 |
There was a problem hiding this comment.
@sidosera in case you remember, was this introduced intentionally?
Possibly. I would like to make sure we really need them first. I believe we are not supposed to remove them once they are added. |
| combine | ||
| combine-Ignore | ||
| required_capability: rerank | ||
| required_capability: rerank_combine |
There was a problem hiding this comment.
I have no memory of this, sorry.
| combine | ||
| combine-Ignore | ||
| required_capability: rerank | ||
| required_capability: rerank_combine |
There was a problem hiding this comment.
@afoucret, it looks like these weren't being run and I think I broke them. You own this function, right? Could you get these running?
@idegtiarenko, I think you should merge this.
…on-sliced-reindex * upstream/main: Activity logging improvements (elastic#142901) Fix serialization of NodeGpuStatsResponse when no GPU is present (elastic#142937) Add doc on master elections in DistributedArchitectureGuide (elastic#142435) ESQL: Account for missing StubRelation due to SurrogateExpressions replacement (elastic#142882) Add BulkByScrollTask Serialization Tests (elastic#142697) Rebalance CI test partitions to reduce Part3 bottleneck (elastic#142930) Mute org.elasticsearch.xpack.esql.qa.multi_node.EsqlClientYamlIT test {p0=esql/40_tsdb/to_aggregate_metric_double with multi_values} elastic#142964 Bump OpenTelemetry dependencies (elastic#142323) SQL: add support for API key to JDBC and CLI (elastic#142021) Ensure requested capability exists (elastic#142695) Warn and fall back to local branches.json (elastic#142606) [CI] Mute testWithFetchFailures, testAddCompletionListenerScheduleErr… (elastic#142926) ESQL: Add support for ORC file format (elastic#142900) Update wolfi (versioned) (elastic#142948) Add BulkByScrollResponse Serialization Tests (elastic#142688) Run 25_id_generation with and without synthetic id (elastic#142770)
4 participants
This change verifies that requested capability actually exists.
Otherwise it is possible to make a typo in a capability and silently skip test in all contexts.
Existing examples relying on missing capabilities are explicitly ignored.