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.

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

Fixed flaky CI tests by replacing httpbin with a simple http server #395

Merged

Conversation

saimedhi
Copy link
Collaborator

Description

Fixed flaky CI tests by replacing httpbin with a simple http server

Issues Resolved

Closes #327

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

@codecov
Copy link

codecov bot commented May 16, 2023

Codecov Report

Merging #395 (d68f37c) into main (2abb8c4) will increase coverage by 0.12%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #395      +/-   ##
==========================================
+ Coverage   71.44%   71.57%   +0.12%     
==========================================
  Files          77       77              
  Lines        7190     7190              
==========================================
+ Hits         5137     5146       +9     
+ Misses       2053     2044       -9     
Impacted Files Coverage Δ
opensearchpy/_async/http_aiohttp.py 82.81% <100.00%> (+3.12%) ⬆️

... and 3 files with indirect coverage changes

harshavamsi
harshavamsi previously approved these changes May 17, 2023
Copy link
Member

@dblock dblock left a comment

Choose a reason for hiding this comment

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

It's close! Let's cleanup the code, see below, and ensure we start/stop servers regardless of whether the tests pass/fail by moving setup/teardown in the right places.

test_opensearchpy/http_server.py Outdated Show resolved Hide resolved
test_opensearchpy/http_server.py Outdated Show resolved Hide resolved
test_opensearchpy/http_server.py Outdated Show resolved Hide resolved
self._server_thread = threading.Thread(target=self.serve_forever)
self._server_thread.start()

def stop(self):
Copy link
Member

Choose a reason for hiding this comment

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

This creates two shutdown() methods, which is weird. Overwrite shutdown or use an instance of HTTPServer instead of overriding it.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

while overwriting shutdown I am getting "recursionerror: maximum recursion depth exceeded while calling a python object". So I left it as stop method.

test_opensearchpy/test_async/test_connection.py Outdated Show resolved Hide resolved
test_opensearchpy/test_connection.py Outdated Show resolved Hide resolved
Copy link
Member

@dblock dblock left a comment

Choose a reason for hiding this comment

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

It's close! Let's cleanup the code, see below, and ensure we start/stop servers regardless of whether the tests pass/fail by moving setup/teardown in the right places.

Signed-off-by: saimedhi <saimedhi@amazon.com>
@saimedhi saimedhi force-pushed the replacing_httpbin_with_httpserver branch from 6554a47 to d68f37c Compare May 23, 2023 19:47
@saimedhi saimedhi requested a review from dblock May 23, 2023 20:04
@dblock dblock merged commit 0395798 into opensearch-project:main May 23, 2023
@dblock
Copy link
Member

dblock commented May 23, 2023

👏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[CCI][BUG] Httpbin.org intermittently stops responding, which fails some tests
3 participants