Update regex for reason matching in Elasticsearch spec to support JRuby 10

[jsvd]

No description provided.

[elasticmachine]

💚 Build Succeeded

• Buildkite Build
• Commit: 6b9dbaa

History

• 💚 Build #32 succeeded 437f266
• 💚 Build #31 succeeded 534bc47

[donoghuc]

I wonder if this is a good opportunity to move away from having assertions about string representations of ruby objects as the regex is gettting a bit complex to support both jruby 9/10. Without having looked too closely at what produces the data in question, maybe we could print individual fields or use to_json and parse that?

[jsvd]

@claude do what Cas said:

I wonder if this is a good opportunity to move away from having assertions about string representations of ruby objects as the regex is gettting a bit complex to support both jruby 9/10. Without having looked too closely at what produces the data in question, maybe we could print individual fields or use to_json and parse that?

[jsvd]

@copilot what about you, can you suggest a fix according to what Cas said in #1250 (comment) ?

[Copilot]

@jsvd I've opened a new pull request, #1251, to work on those changes. Once the pull request is ready, I'll request review from you.

[jsvd]

@donoghuc unfortunately we're passing a string dlq_writer.write's second argument "reason", so we can't change what that string has (e.g. contain a marshalled json) without changing the shape of what currently goes to the dlq.
I think we can still change it and may not be necessarily a breaking change, but I think it becomes out of scope for this PR.

[donoghuc]

Given the complexity in the regex is confined to a single test assertion I think its low risk. Thanks for looking in to the alternative.