elastic / elasticsearch

Free and Open Source, Distributed, RESTful Search Engine
https://www.elastic.co/products/elasticsearch
Other
1.23k stars 24.85k forks source link

Allow setting regex flags on YAML test matches #35805

Open polyfractal opened 5 years ago

polyfractal commented 5 years ago

To make _cat yaml tests more pleasant to read/write (and maybe elsewhere), we implicitly enable the Pattern.COMMENTS flag on match regex matches.

The COMMENTS flag means whitespace is ignored, and if you care about whitespace, you have to specify it directly with \s patterns. This works really nicely for tabular _cat responses, but makes it very difficult to use anywhere else.

For example, I had a test where I wanted to check an error message in the body. There was a state in the message that could have been STARTED or INDEXING, so a perfect for regex. It turns out the only way to specify that sort of regex with a regular exception message is:

/Could \s not \s delete \s job \s \[foo\] \s because \s indexer \s state \s is \s \[(STARTED|INDEXING)\]\. \s+ Job \s must \s be \s \[STOPPED\] \s before \s deletion\./

Which is... not great :)

One option is to support regex flags (/foo.*/x) so that a test can decide if it needs to add a flag or not. This would be the cleanest solution, but it's a breaking change in that all the clients will need to support the new flag syntax. Otherwise all the _cat tests will break.

There are a few other options, like adding a match_regex_strict or something, but they sound clunky. We can't unfortunately add parameters to the match itself based on how tests are structured.

This isn't a enormous limitation (we've survived this long after all), but it is almost entirely unexpected and difficult to work around in some cases. Particularly since the regex matcher on error messages does not have the flag, so we're used to using "normal" regex in errors and then see strange behavior in body matches.

elasticmachine commented 5 years ago

Pinging @elastic/es-core-infra

polyfractal commented 5 years ago

Also /cc @elastic/es-clients