Skip to content

Exclude invalid url-encoded strings from randomized tests #71085

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conversation

danhermann
Copy link
Contributor

The randomized testing framework produces some input strings that are not valid URL-encoded strings so the URLDecodeProcessor correctly throws an exception. This change retries any tests that use a string that cannot be URL decoded.

Fixes #71077.

@danhermann danhermann added >test Issues or PRs that are addressing/adding tests :Data Management/Ingest Node Execution or management of Ingest Pipelines including GeoIP v8.0.0 v7.13.0 v7.12.1 labels Mar 30, 2021
@elasticmachine elasticmachine added the Team:Data Management Meta label for data/management team label Mar 30, 2021
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-features (Team:Core/Features)

processor.execute(ingestDocument);
assertThat(ingestDocument.getFieldValue(targetFieldName, expectedResultType()), equalTo(expectedResult(fieldValue)));
}

protected boolean isSupportedValue(Object value) {
return false;
Copy link
Member

Choose a reason for hiding this comment

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

I think the default should return true here? Because now if this method isn't overwritten, the do-while loop never ends (i think this is why the some of these test fail).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, thanks. I renamed the isSupportedValue method from isUnsupportedValue but forgot to change the default return value. 🤦‍♂️

@danhermann
Copy link
Contributor Author

@elasticmachine update branch

Copy link
Member

@jbaiera jbaiera left a comment

Choose a reason for hiding this comment

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

LGTM

@danhermann danhermann merged commit 370ce5d into elastic:master Apr 1, 2021
@danhermann danhermann deleted the 71077_urldecodeprocessortests_testtargetfield_failure branch April 1, 2021 16:12
@gwbrown gwbrown added v7.12.2 and removed v7.12.1 labels Apr 14, 2021
danhermann added a commit to danhermann/elasticsearch that referenced this pull request May 20, 2021
danhermann added a commit to danhermann/elasticsearch that referenced this pull request Jun 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Data Management/Ingest Node Execution or management of Ingest Pipelines including GeoIP Team:Data Management Meta label for data/management team >test Issues or PRs that are addressing/adding tests v7.12.2 v7.13.0 v8.0.0-alpha1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[CI] URLDecodeProcessorTests testTargetField failing
6 participants