-
Notifications
You must be signed in to change notification settings - Fork 11
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
[CI] Add binary run tests #22
base: master
Are you sure you want to change the base?
Conversation
717751d
to
b3feada
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
@pytest.fixture | ||
def start_binary(): | ||
clear_octobot_previous_folders() | ||
with TemporaryFile() as output, TemporaryFile() as err: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
tests/test_binary.py
Outdated
terminate_binary(binary_process, output, err) | ||
|
||
|
||
def create_binary(binary_options, output_file, err_file): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should this function be named 'start_binary' ?
preexec_fn=os.setsid if not is_on_windows() else None) | ||
|
||
|
||
def terminate_binary(binary_process, output_file, err_file): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
tests/test_binary.py
Outdated
with TemporaryFile() as output, TemporaryFile() as err: | ||
binary_process = create_binary("", output, err) | ||
yield | ||
terminate_binary(binary_process, output, err) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should put this call in a finally
tests/test_binary.py
Outdated
time.sleep(LOG_CONTENT_TEST_WAITING_TIME) | ||
log_content = get_log_file_content() | ||
logger.debug(log_content) | ||
assert "BALANCE PROFITABILITY :" in log_content |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
great tests !
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we could add while loop for the test_version_endpoint
, test_evaluation_state_created
and test_balance_profitability_updated
instead of time.sleep() to speed up the test when possible ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think that there is a lot of time to save, but it would be a great improvement.
b89d823
to
05c6a5c
Compare
217e782
to
4ea7304
Compare
45fbefb
to
560bfb6
Compare
finally: | ||
logger.debug("Killing binary process...") | ||
if is_on_windows(): | ||
subprocess.call(["taskkill", "/F", "/IM", "OctoBot_windows.exe"]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
|
||
BINARY_DISABLE_WEB_OPTION = "-nw" | ||
LOG_CHECKS_MAX_ATTEMPTS = 300 | ||
DEFAULT_TIMEOUT_WINDOW = -95 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
a4f075f
to
da2b75b
Compare
No description provided.