Conversation
eb5a384
to
9a840e9
Compare
@@ -10,8 +10,8 @@ cache: | |||
directories: | |||
- $HOME/.cache/pip | |||
install: | |||
- pip install pip==6.1.0 | |||
- pip install -r dev-requirements.txt | |||
- pip install -U pip |
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.
You also want to install wheel.
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.
Thanks @clintonb.
9a840e9
to
e050faa
Compare
@@ -0,0 +1,7 @@ | |||
# Packages required to run load tests | |||
-e git+https://github.com/edx/locust.git@edx#egg=locustio[scipy] | |||
-e . |
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.
@cpennington why is edx-load-tests installed as a requirement? Why does this repo even have a setup.py?
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.
It's installed as a requirement so that tests can reference anything in the package as importable modules. Locust does some funny things with sys.path
when it starts a test.
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.
@cpennington it does look like Locust does some sys.path
manipulation, but it appears to revert any changes it makes afterwards.
Is there a specific import in a test that requires this? The new test I've added doesn't require installing edx-load-tests to import modules from other packages in the repo.
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.
It's possible that the need for this was resolved when I restructured the package. However, I think it was added so that tests could use code in the helpers
package without needing to do their own sys.path
manipulation.
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.
@cpennington I've verified that the tests can use code from helpers
without any sys.path
manipulation, without installing edx-load-tests
as a dependency. In an effort to clean up this repo, I'm going to remove this and setup.py
in a separate commit. Let me know if you object.
4c0b1c8
to
c104da8
Compare
@@ -10,8 +10,7 @@ cache: | |||
directories: | |||
- $HOME/.cache/pip | |||
install: | |||
- pip install pip==6.1.0 | |||
- pip install -r dev-requirements.txt | |||
- pip install -U pip wheel pep8 |
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.
can you use the long form of the flag, to make it clearer what it's doing?
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.
Updated.
c104da8
to
6cd5929
Compare
@rlucioni please note my comment #28 (comment) which is hidden under an outdated diff. |
from locust import TaskSet, task, HttpLocust | ||
|
||
from helpers.api import LocustEdxRestApiClient | ||
from programs.config import PROGRAMS_API_URL, JWT_AUDIENCE, JWT_ISSUER, JWT_SECRET_KEY |
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.
If you pull in helpers.raw_logs
, then you can get csv files with all of the requests in them, and use the ipython notebook that's also checked into the repo to get better stats than locust gives on its own.
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.
@cpennington it would be great to document what these raw logs provide and how to use them in this repo's README. Will the raw logs take the place of Locust's statistics reporting, or are they in addition?
Given that I'm presently only after a 95% percentile response time for a single endpoint, and that our fork of Locust already includes error bands in the compiled report post-test, I'm leaning towards not pulling in the raw logging as part of this PR. If we need it later, my impression is that we can pull it in easily.
6cd5929
to
5e50df5
Compare
Includes a task exercising the Programs API's program list endpoint. Also cleans up some aspects of this repo. ECOM-2627.
5e50df5
to
a668c44
Compare
Tests are capable of importing code from other modules without doing this.
@jimabramson @cpennington I've addressed all of your comments. Could you please give this another pass? |
👍 |
1 similar comment
👍 |
Includes a task exercising the Programs API's program list endpoint. Also cleans up some aspects of this repo. ECOM-2627.
@jimabramson @cpennington @edx/ecommerce