Tests: Improve size regex in documentation test #26879
Conversation
The regex has been changed to not only be able to deal with something like `260b`, but also support `1.1kb`.
There was a problem hiding this comment.
@spinscale the change in the regex looks good to me, however I'm also curious how this tripped since I cannot reproduce the failure locally (tried on 6x since thats where the CI error occured). Do you know if this happen more often recently in tests? If not, might be worth to hold to see what causes it?
|
Okay, I see failures like this across all branches (6.0, 6x, master) since Sep 15th, not very common but probably still worth fixing. |
|
@spinscale just fyi, I was curious and started adding empty indices to a local cluster running off the 6.x branch, then doing a |
| @@ -352,7 +352,7 @@ And the response: | |||
| health status index uuid pri rep docs.count docs.deleted store.size pri.store.size | |||
| yellow open customer 95SQ4TSUT7mWBT7VNHH67A 5 1 0 0 260b 260b | |||
| -------------------------------------------------- | |||
| // TESTRESPONSE[s/95SQ4TSUT7mWBT7VNHH67A/.+/ s/260b/\\d+b/ _cat] | |||
| // TESTRESPONSE[s/95SQ4TSUT7mWBT7VNHH67A/.+/ s/260b/\\d+(\\.\\d+)?b/ _cat] | |||
There was a problem hiding this comment.
shouldn't we also cover the case where the unit changes from b to kb ?
There was a problem hiding this comment.
does \d+(\.\d)?[kbg]?b sound better? With that 1.1b, 100b, 12.1gb, 1.1kb should match
There was a problem hiding this comment.
I think restricting it to kb and b only for this test would be better, we should detect if the value suddenly jumps to mb here for an almost empty index. So adding k? to the mix should be enough IMHO.
There was a problem hiding this comment.
@cbuescher ++ that's what I had in mind too.
| @@ -368,7 +368,7 @@ And the response: | |||
| health status index uuid pri rep docs.count docs.deleted store.size pri.store.size | |||
| yellow open customer 95SQ4TSUT7mWBT7VNHH67A 5 1 0 0 260b 260b | |||
| -------------------------------------------------- | |||
| // TESTRESPONSE[s/95SQ4TSUT7mWBT7VNHH67A/.+/ s/260b/\\d+b/ _cat] | |||
| // TESTRESPONSE[s/95SQ4TSUT7mWBT7VNHH67A/.+/ s/260b/\\d+(\\.\\d)?k?b/ _cat] | |||
There was a problem hiding this comment.
Ah, damn regex! This looks like its going to work for sth. like 1234b and 1.1kb but will not match if we get two or more digits after the comma at some point. At least according to http://www.regexplanet.com/advanced/java/index.html, \\d+(\\.\\d)?k?b doesn't match e.g. 1.123kb. I think \\d+\\.?\\d+?k?b might, and I don't think we need the group?
There was a problem hiding this comment.
indeed, the grouping can be removed. No need to support .123, because the formatting is cut after one decimal by the ByteSizeValue class
The regex has been changed to not only be able to deal with something like `260b`, but also support `1.1kb`.
The regex has been changed to not only be able to deal with something like `260b`, but also support `1.1kb`.
* es/master: (24 commits) Reduce synchronization on field data cache add json-processor support for non-map json types (#27335) Properly format IndexGraveyard deletion date as date (#27362) Upgrade AWS SDK Jackson Databind to 2.6.7.1 Stop responding to ping requests before master abdication (#27329) [Test] Fix POI version in packaging tests Allow affix settings to specify dependencies (#27161) Tests: Improve size regex in documentation test (#26879) reword comment Remove unnecessary logger creation for doc values field data [Geo] Decouple geojson parse logic from ShapeBuilders [DOCS] Fixed link to docker content Plugins: Add versionless alias to all security policy codebase properties (#26756) [Test] #27342 Fix SearchRequests#testValidate [DOCS] Move X-Pack-specific Docker content (#27333) Fail queries with scroll that explicitely set request_cache (#27342) [Test] Fix S3BlobStoreContainerTests.testNumberOfMultiparts() Set minimum_master_nodes to all nodes for REST tests (#27344) [Tests] Relax allowed delta in extended_stats aggregation (#27171) Remove S3 output stream (#27280) ...
* 6.x: (27 commits) Reduce synchronization on field data cache [Test] #27342 Fix SearchRequests#testValidate Fail queries with scroll that explicitely set request_cache (#27342) add json-processor support for non-map json types (#27335) Upgrade AWS SDK Jackson Databind to 2.6.7.1 Properly format IndexGraveyard deletion date as date (#27362) Stop responding to ping requests before master abdication (#27329) [Test] Fix POI version in packaging tests Added release notes for 6.0.0 Update 6.0.0-beta1.asciidoc Allow affix settings to specify dependencies (#27161) Tests: Improve size regex in documentation test (#26879) reword comment Ensure external refreshes will also refresh internal searcher to minimize segment creation (#27253) Remove unnecessary logger creation for doc values field data [DOCS] Fixed link to docker content Plugins: Add versionless alias to all security policy codebase properties (#26756) [DOCS] Move X-Pack-specific Docker content (#27333) [Test] Fix S3BlobStoreContainerTests.testNumberOfMultiparts() Set minimum_master_nodes to all nodes for REST tests (#27344) ...
The regex has been changed to not only be able to deal with something
like
260b, but also support1.1kb.This little stems from a test failure in 6.x, which can be seen here https://elasticsearch-ci.elastic.co/job/elastic+elasticsearch+6.x+multijob-unix-compatibility/os=amazon/203/consoleText
I am not a hundred percent sure, if there should be more investigations regarding that increased index size, but I would assume that this is not the culprit.