Added comments to load test locustfile.py #407
Conversation
| [Autopush HTTP Endpoints for Notifications](https://mozilla-services.github.io/autopush-rs/http.html#push-service-http-api) | ||
| for details). |
There was a problem hiding this comment.
Will this render in the file? This looks like a markdown style link.
There was a problem hiding this comment.
I think it depends on whatever you're using to render the docs. I believe that Sphinx has a markdown plug in.
I tend to use markdown because I find it easier to read than RST.
| # NOTE: I believe that this is cruft from an earlier system. This condition | ||
| # should just be replaced with | ||
| # ``` | ||
| # endpoint_url = res["pushEndpoint"] | ||
| # ``` |
There was a problem hiding this comment.
Do you think we need these notes here? Or would it be better off to track these externally so when we fix them, we won't have to edit this file again.
There was a problem hiding this comment.
I'm fine either way. I wanted to capture things like this in the code for now since I was going through it.
There was a problem hiding this comment.
I had a chat with Kat, I think I didn't understand the context of this PR. Thank you for going through and explaining all the steps, I learned some stuff for sure!
|
@jrbenny35 I'm thinking to merge this PR so that the context is available in the document history? When we refactor we can edit/add/remove them accordingly. |
| @@ -132,7 +227,12 @@ def test_basic_topic(self): | |||
| assert ( | |||
| endpoint_res.status_code == 201 | |||
| ), f"status code was {endpoint_res.status_code}" | |||
| # connect and check notification | |||
|
|
|||
| # NOTE: To properly test "topic" messages, we really ought to | |||
There was a problem hiding this comment.
This almost sounds like a TODO but test_connect_stored does this. Maybe reword or maybe mention it here?
There was a problem hiding this comment.
test_connect_stored kind of does that.
What it does is send in 10 topic messages, then read from the queue 10 times looking to see if the response matches. Since the response is never ACK'd, it's never removed, and all 10 work correctly, but it feels like that's a bug rather than a feature.
Added some comments per discussion with @Trinaa.
I marked things that are curious or inexplicable with "NOTE:"
Please feel free to either ignore, improve, or whatever.