Skip to content

Include human readable responses in response parsing tests#22717

Merged
cbuescher merged 2 commits intoelastic:masterfrom
cbuescher:testParsingHumanReadable
Jan 24, 2017
Merged

Include human readable responses in response parsing tests#22717
cbuescher merged 2 commits intoelastic:masterfrom
cbuescher:testParsingHumanReadable

Conversation

@cbuescher
Copy link
Copy Markdown
Member

As a follow up to #22649, this changes the resent tests for parsing parts of search responses to randomly set the humanReadable() flag of
the XContentBuilder that is used to render the responses. This should help to test that we can parse back thoses classes if the user specifies ?human=true in the request url.

As a follow up to elastic#22649, this changes the resent tests for
parsing parts of search responses to randomly set the humanReadable() flag of
the XContentBuilder that is used to render the responses. This should
help to test that we can parse back thoses classes if the user specifies
`?human=true` in the request url.
@cbuescher cbuescher added >enhancement review >test Issues or PRs that are addressing/adding tests v6.0.0-alpha1 labels Jan 20, 2017
@cbuescher cbuescher requested review from javanna and tlrx January 20, 2017 14:03
Copy link
Copy Markdown
Contributor

@javanna javanna left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, I take it that you didn't find any problems when adding this new check? @tlrx want to have a quick look too?

@cbuescher
Copy link
Copy Markdown
Member Author

No, all good, I think the human flag only makes a difference in the Profile section output at the moment. These tests are mostly added to protect us from suprises in the future.

Copy link
Copy Markdown
Member

@tlrx tlrx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, left two minor comments

String id = randomAsciiOfLength(5);
long seqNo = randomIntBetween(-2, 5);
long version = (long) randomIntBetween(0, 5);
long seqNo = randomBoolean() ? randomNonNegativeLong() : randomIntBetween(0, 10000);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we use
randomFrom(SequenceNumbersService.UNASSIGNED_SEQ_NO, randomNonNegativeLong(), randomIntBetween(0, 10000))

instead?

String id = randomAsciiOfLength(5);
long seqNo = randomIntBetween(-2, 5);
long version = (long) randomIntBetween(0, 5);
long seqNo = randomBoolean() ? randomNonNegativeLong() : randomIntBetween(0, 10000);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here

@cbuescher cbuescher merged commit 59aefe5 into elastic:master Jan 24, 2017
cbuescher added a commit that referenced this pull request Jan 24, 2017
As a follow up to #22649, this changes the resent tests for parsing parts of search
responses to randomly set the humanReadable() flag of the XContentBuilder that
is used to render the responses. This should help to test that we can parse back
thoses classes if the user specifies `?human=true` in the request url.
@cbuescher
Copy link
Copy Markdown
Member Author

Added to 5.x with 25a219f

@cbuescher cbuescher deleted the testParsingHumanReadable branch November 27, 2025 10:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

>test Issues or PRs that are addressing/adding tests v5.3.0 v6.0.0-alpha1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants