-
Notifications
You must be signed in to change notification settings - Fork 3k
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
stats summary refactoring #2193
Conversation
Did you end up just duplicating the code and not using the new functions from anywhere else? This doesnt make sense. And Also, you need to fix flake8 & black formatting errors. |
print_stats_summary is directly printing to console_logger. So I refactored that function into get_stats_summary and called it from print_stats but the unit test is checking the length of string. Since it is single string the length is always 1. So it is failed. Due to that I duplicated the function. May be I will try to create it as list of string then call it from print_stats so that it won't be a duplicate. For black/flake8 formatting, as of now I am doing this from vscode web editor which doesn't have formatting option. I am trying to setup local dev environment and try to format it. |
Not sure what you mean, but duplication is bad :)
Yea... tbh, that kind of sounds like you'll have a lot of work ahead of you to make this reach the quality required for merge. You might consider having this as a fork, or trying to implement it inside your own locustfile or support lib. |
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.
Duplication is removed
@cyberw I removed the duplication. |
Looks a lot better now. Just fix the formatting and typing issues and it might be ready. |
I have fixed the type hint issue. |
formatting also done. type hint is fixed as well |
👍 👍 |
Added get_stats_summary and get_percentile_stats_summary . This will return the stats as string , So that the string can be used in other places. I am using this to record it in Reportportal.
currently print_stats and print_percentile_stats functions are directly record the result into console_logger. But these functions return the same results into string so that it can be used by other log handler