Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[ML] Transform test fix for testCreateLatestFunction #66293

Merged
merged 5 commits into from
Dec 15, 2020
Merged

[ML] Transform test fix for testCreateLatestFunction #66293

merged 5 commits into from
Dec 15, 2020

Conversation

benwtrent
Copy link
Member

closes #66292

@benwtrent benwtrent added >test Issues or PRs that are addressing/adding tests v8.0.0 :ml/Transform Transform v7.11.0 v7.12.0 labels Dec 14, 2020
@elasticmachine
Copy link
Collaborator

Pinging @elastic/ml-core (:ml/Transform)

@benwtrent
Copy link
Member Author

@elasticmachine update branch

hendrikmuhs
hendrikmuhs reviewed Dec 15, 2020
View reviewed changes
...rc/test/java/org/elasticsearch/xpack/core/transform/transforms/latest/LatestConfigTests.java Outdated
return new LatestConfig(randomList(1, 10, () -> randomAlphaOfLengthBetween(1, 10)), randomAlphaOfLengthBetween(1, 10));
return new LatestConfig(
new ArrayList<>(
new HashSet<>(randomList(1, 10, () -> randomAlphaOfLengthBetween(1, 10))
Copy link

Choose a reason for hiding this comment

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

nit, ESTestCase has a method randomUnique:
randomUnique(() -> randomAlphaOfLengthBetween(1, 10), randomIntBetween(1, 10))

hendrikmuhs
hendrikmuhs approved these changes Dec 15, 2020
View reviewed changes
Copy link

@hendrikmuhs hendrikmuhs left a comment

Choose a reason for hiding this comment

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

LGTM

przemekwitek
przemekwitek approved these changes Dec 15, 2020
View reviewed changes
Copy link
Contributor

@przemekwitek przemekwitek left a comment

Choose a reason for hiding this comment

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

LGTM

@hendrikmuhs
Copy link

the same problem exists in 2 other places:

org.elasticsearch.client.transform.transforms.latest.hlrc.LatestConfigTests
org.elasticsearch.client.transform.transforms.latest.LatestConfigTests

@benwtrent can you fix those, too?

@benwtrent benwtrent merged commit 940bd86 into elastic:master Dec 15, 2020
@benwtrent benwtrent mentioned this pull request Dec 15, 2020
Merged
benwtrent added a commit to benwtrent/elasticsearch that referenced this pull request Dec 15, 2020
@benwtrent
[ML] Transform test fix for testCreateLatestFunction (elastic#66293)
972edf6 
closes elastic#66292
benwtrent added a commit that referenced this pull request Dec 15, 2020
@benwtrent
[ML] Transform test fix for testCreateLatestFunction (#66293) (#66338)
4fa1f96 
closes #66292
@benwtrent benwtrent deleted the test-fix/ml-fix-testCreateLatestFunction branch March 3, 2021 11:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@przemekwitek przemekwitek przemekwitek approved these changes

@hendrikmuhs hendrikmuhs hendrikmuhs approved these changes

Assignees
No one assigned
Labels
:ml/Transform Transform >test Issues or PRs that are addressing/adding tests v7.11.0 v7.12.0 v8.0.0-alpha1
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Transform FunctionFactoryTests. testCreateLatestFunction fails
5 participants
@benwtrent @elasticmachine @hendrikmuhs @przemekwitek @jakelandis