Skip to content

Conversation

@cbuescher
Copy link
Member

Under very unfortunate conditions tests that check xContent objects roundtrip parsing
(like i.e. SearchHitsTests testFromXContent can fail when we happen to randomly pick YAML xContent type and create random (realistic)Unicode character sequences that
may contain the character U+0085 (133) from the Latin1 code page.

That specific character doesn't get parsed back to its original form for YAML xContent, which can lead to
rare but hard to diagnose test failures.

This change adds logic to AbstractXContentTestCase#test() which lies at the core of most of our
xContent roundtrip tests that disallows test instances containing that particular character
when using YAML xContent type.

Closes #97716

@cbuescher cbuescher added >test Issues or PRs that are addressing/adding tests auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) auto-backport Automatically create backport pull requests when merged :Search Foundations/Search Catch all for Search Foundations v9.0.0 v8.18.1 v8.19.0 v9.1.0 labels Jan 31, 2025
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-search-foundations (Team:Search Foundations)

@elasticsearchmachine elasticsearchmachine added the Team:Search Foundations Meta label for the Search Foundations team in Elasticsearch label Jan 31, 2025
@elasticsearchmachine elasticsearchmachine merged commit fd1bd79 into elastic:main Jan 31, 2025
17 checks passed
@cbuescher cbuescher deleted the fix-97716-b branch January 31, 2025 22:57
This was referenced Jan 31, 2025
cbuescher added a commit to cbuescher/elasticsearch that referenced this pull request Jan 31, 2025
Under very unfortunate conditions tests that check xContent objects
roundtrip parsing  (like i.e. [SearchHitsTests
testFromXContent](elastic#97716)
can fail when we happen to randomly pick YAML xContent type and create
random (realistic)Unicode character sequences that may contain the
character U+0085 (133) from the [Latin1 code
page](https://de.wikipedia.org/wiki/Unicodeblock_Lateinisch-1,_Erg%C3%A4nzung).

That specific character doesn't get parsed back to its original form for
YAML xContent, which can lead to [rare but hard to diagnose test
failures](elastic#97716 (comment)).

This change adds logic to AbstractXContentTestCase#test() which lies at
the core of most of our  xContent roundtrip tests that disallows test
instances containing that particular character  when using YAML xContent
type.

Closes elastic#97716
cbuescher added a commit to cbuescher/elasticsearch that referenced this pull request Jan 31, 2025
Under very unfortunate conditions tests that check xContent objects
roundtrip parsing  (like i.e. [SearchHitsTests
testFromXContent](elastic#97716)
can fail when we happen to randomly pick YAML xContent type and create
random (realistic)Unicode character sequences that may contain the
character U+0085 (133) from the [Latin1 code
page](https://de.wikipedia.org/wiki/Unicodeblock_Lateinisch-1,_Erg%C3%A4nzung).

That specific character doesn't get parsed back to its original form for
YAML xContent, which can lead to [rare but hard to diagnose test
failures](elastic#97716 (comment)).

This change adds logic to AbstractXContentTestCase#test() which lies at
the core of most of our  xContent roundtrip tests that disallows test
instances containing that particular character  when using YAML xContent
type.

Closes elastic#97716
@cbuescher cbuescher mentioned this pull request Jan 31, 2025
@elasticsearchmachine
Copy link
Collaborator

💚 Backport successful

Status Branch Result
9.0
8.18
8.x

cbuescher added a commit to cbuescher/elasticsearch that referenced this pull request Jan 31, 2025
Under very unfortunate conditions tests that check xContent objects
roundtrip parsing  (like i.e. [SearchHitsTests
testFromXContent](elastic#97716)
can fail when we happen to randomly pick YAML xContent type and create
random (realistic)Unicode character sequences that may contain the
character U+0085 (133) from the [Latin1 code
page](https://de.wikipedia.org/wiki/Unicodeblock_Lateinisch-1,_Erg%C3%A4nzung).

That specific character doesn't get parsed back to its original form for
YAML xContent, which can lead to [rare but hard to diagnose test
failures](elastic#97716 (comment)).

This change adds logic to AbstractXContentTestCase#test() which lies at
the core of most of our  xContent roundtrip tests that disallows test
instances containing that particular character  when using YAML xContent
type.

Closes elastic#97716
elasticsearchmachine pushed a commit that referenced this pull request Feb 1, 2025
Under very unfortunate conditions tests that check xContent objects
roundtrip parsing  (like i.e. [SearchHitsTests
testFromXContent](#97716)
can fail when we happen to randomly pick YAML xContent type and create
random (realistic)Unicode character sequences that may contain the
character U+0085 (133) from the [Latin1 code
page](https://de.wikipedia.org/wiki/Unicodeblock_Lateinisch-1,_Erg%C3%A4nzung).

That specific character doesn't get parsed back to its original form for
YAML xContent, which can lead to [rare but hard to diagnose test
failures](#97716 (comment)).

This change adds logic to AbstractXContentTestCase#test() which lies at
the core of most of our  xContent roundtrip tests that disallows test
instances containing that particular character  when using YAML xContent
type.

Closes #97716
elasticsearchmachine pushed a commit that referenced this pull request Feb 1, 2025
Under very unfortunate conditions tests that check xContent objects
roundtrip parsing  (like i.e. [SearchHitsTests
testFromXContent](#97716)
can fail when we happen to randomly pick YAML xContent type and create
random (realistic)Unicode character sequences that may contain the
character U+0085 (133) from the [Latin1 code
page](https://de.wikipedia.org/wiki/Unicodeblock_Lateinisch-1,_Erg%C3%A4nzung).

That specific character doesn't get parsed back to its original form for
YAML xContent, which can lead to [rare but hard to diagnose test
failures](#97716 (comment)).

This change adds logic to AbstractXContentTestCase#test() which lies at
the core of most of our  xContent roundtrip tests that disallows test
instances containing that particular character  when using YAML xContent
type.

Closes #97716
@cbuescher
Copy link
Member Author

Sorry folks, I didn't intend to merge this without review (nor backport). Will revert and re-open to get proper review.

cbuescher added a commit that referenced this pull request Feb 3, 2025
This reverts commit fd1bd79.
PR was merged by a mistake, still needs to get reviewed.
@cbuescher
Copy link
Member Author

Reopened with #121515

cbuescher added a commit that referenced this pull request Feb 3, 2025
cbuescher added a commit that referenced this pull request Feb 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

auto-backport Automatically create backport pull requests when merged auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) :Search Foundations/Search Catch all for Search Foundations Team:Search Foundations Meta label for the Search Foundations team in Elasticsearch >test Issues or PRs that are addressing/adding tests v8.18.1 v8.19.0 v9.0.0 v9.1.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[CI] SearchHitsTests testFromXContent failing

2 participants