-
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
Docs: update test statistics page with example responses #747
Docs: update test statistics page with example responses #747
Conversation
Codecov Report
@@ Coverage Diff @@
## master #747 +/- ##
==========================================
- Coverage 65.93% 65.13% -0.81%
==========================================
Files 14 14
Lines 1374 1417 +43
Branches 214 222 +8
==========================================
+ Hits 906 923 +17
- Misses 421 443 +22
- Partials 47 51 +4
Continue to review full report at Codecov.
|
docs/retrieving-stats.rst
Outdated
|
||
$cat example_distribution.csv | ||
"Name","# requests","50%","66%","75%","80%","90%","95%","98%","99%","100%" | ||
"GET /",31,4,4,4,4,4,4,4,4,4 |
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.
isn't the "Method" field header missing here?
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'm not sure, it's identical to what is generated by locust for the stats pages currently.
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.
oh I see. Method and Name are merged into a "Name" column. This seems like a bug to me (though unrelated to your PR)
docs/retrieving-stats.rst
Outdated
@@ -9,13 +9,31 @@ First, when running Locust with the web UI, you can retrieve CSV files under the | |||
Secondly, you can run Locust with a flag which will periodically save the CSV file. This is particularly useful | |||
if you plan on running Locust in an automated way with the ``--no-web`` flag:: | |||
|
|||
locust -f locust_files/my_locust_file.py --csv=foobar --no-web -n10 -t10m | |||
locust -f examples/basic.py --csv=example --no-web -n10 -t10m | |||
|
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.
is example
the full name of the csv file? Might want to name it example.csv
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.
the type of file gets added, so providing example
results in example_distribution.csv
and example_requests.csv
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.
gotcha. that's confusing. maybe note that in the docs?
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 added clarification there
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.
LGTM 👍
docs/retrieving-stats.rst
Outdated
|
||
or for v0.8 (where the ``-t`` option isn't available):: | ||
|
||
locust -f locust_files/my_locust_file.py --csv=foobar --no-web -n10 -c10 | ||
locust -f examples/basic.py --csv=example --no-web -n10 -c10 |
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.
is -n
switch working? If not (which I suspect) maybe we should either remove it from the example or reintroduce it to the code.
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.
good catch, will update - it worked when I originally wrote this functionality :)
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.
@aldenpeterson-wf can you update ^^ ? then let's get this merged.
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.
👍
Provide an example in the docs based on this Stack Overflow question.