Skip to content

Refactor Tests for test_client directory #1656

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
Closed
wants to merge 3 commits into from
Closed

Refactor Tests for test_client directory #1656

wants to merge 3 commits into from

Conversation

V1NAY8
Copy link
Contributor

@V1NAY8 V1NAY8 commented Aug 1, 2021 •
edited
Loading

Converts tests to use pytest framework and optimize them.

related_task : "Task" = #1647

@elasticmachine
Copy link
Collaborator

Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually?

@V1NAY8 V1NAY8 marked this pull request as draft August 1, 2021 15:43
@V1NAY8 V1NAY8 force-pushed the refactor-tests-1 branch from c4fc48d to 136b690 Compare August 2, 2021 13:59
@V1NAY8 V1NAY8 marked this pull request as ready for review August 2, 2021 14:01
@sethmlarson
Copy link
Contributor

jenkins test this please

@V1NAY8 V1NAY8 changed the title WIP => Refactor Tests for test_client directory Refactor Tests for test_client directory Aug 2, 2021
@sethmlarson
Copy link
Contributor

There are a lot of improvements that are really great in this PR! I'm working on a big overhaul removing all usage of unitest and removing elasticsearch.helpers.test here: #1692 for 8.0.0. This might be a pretty nasty rebase but if there are improvements you can keep after the mentioned PR lands that'd be awesome 🎉

@sethmlarson
Copy link
Contributor

jenkins test this please

sethmlarson
sethmlarson suggested changes Aug 30, 2021
View reviewed changes
Copy link
Contributor

@sethmlarson sethmlarson left a comment •
edited
Loading

Choose a reason for hiding this comment

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

One comment, otherwise this PR looks good to me.

@V1NAY8 V1NAY8 requested a review from sethmlarson August 31, 2021 12:46
@sethmlarson
Copy link
Contributor

Jenkins test this please

@pquentin
Copy link
Member

pquentin commented Sep 8, 2021

Jenkins test this please

@V1NAY8
Copy link
Contributor Author

V1NAY8 commented Sep 21, 2021

@sethmlarson Is this looking good? 😀

@sethmlarson
Copy link
Contributor

Going to close this for now as it's quite out of date in main. Perhaps once the dust settles on 8.0 we can revisit this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sethmlarson sethmlarson Awaiting requested review from sethmlarson

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@V1NAY8 @elasticmachine @sethmlarson @pquentin