Skip to content
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

Adds support for setup, teardown, and on_stop methods #658

Merged
merged 13 commits into from
Mar 20, 2018

Conversation

DeepHorizons
Copy link
Contributor

If a TaskSet has an on_stop method, it will be run before the process exits. It will also look for a property on the class named always_run_on_stop, which if True will run regardless if the on_start method finished.

Looking to solve #59

Any comments on this functionality?

if a TaskSet has an `on_stop` method, it will be run before the proccess
exits. It will also look for a property on the class named
`always_run_on_stop`, which if True will run regardless if the
`on_start` method finished.
@heyman
Copy link
Member

heyman commented Sep 21, 2017

Hi! I've looked through the changes and here are my initial thoughts:

The on_start method of a TaskSet is run every time a Locust "enters" that TaskSet which might be many times during a test run (since it's common to have nested TaskSets). I do think it could be a good idea to add the ability to have an on_stop method on TaskSets, but then I think it should run within the Locust greenlet whenever a TaskSet is interrupted (so that on_stop is symmetrically opposite to on_start).

If we want to provide some hook that is executed for each Locust user when a test is stopped, I think it should be a method under the Locust class, and I also think it should be executed within the Locust greenlet (by catching GreenletExit, and we need to make sure that the greenlets are actually killed properly when running locust locally with --no-web and pressing CTRL-C).

So that's actually two different issues. And then we also have a third issue of having some kind of teardown hook that is called a single time when the test has stopped, which I believe is what the OP of #59 was seeking.

@DeepHorizons
Copy link
Contributor Author

That makes way more sense than what I was thinking/doing. on_stop should be a one to one with on_start, while the setup and teardown should be just once.

I re-worked the Locust run method to catch that exception, then run on_stop if it exists. I also added the locust_runner.quit() during shutdown which should clean up those greenlets. I'm not sure if you use them anywhere else though.

Here is my idea for the setup and teardown. The Locust class and the TaskSet class can have a setup and teardown method. They will be class methods, not tied to any particular instance. There will also be some class variable to hold the state to see if they have been run already. Order of running would be setup on Locust, setup on TaskSet, teardown on TaskSet, teardown on Locust. The __init__ methods will be modified to check to see if the setup method have been run. If not, run them and update the state. As for the teardown, I'll probably do something similar to what I was doing adding a handler to the quitting event.

@j616
Copy link

j616 commented Oct 17, 2017

I just thought I'd feed back that I've been using this branch for a few weeks for the on_stop functionality in task sets. It works exactly as I'd expect it to and feels almost essential for allowing my locustfiles to clean up after themselves. Hoping this get's accepted into the main upstream. It's too useful to just sit in someone's fork. Thanks for putting in the dev work, @DeepHorizons

@DeepHorizons
Copy link
Contributor Author

@j616 Thanks for letting me know! Let me add some documentation, then I'll remove the [WIP]

@DeepHorizons DeepHorizons changed the title [WIP] Adds support for on_stop method Adds support for setup, teardown, and on_stop methods Oct 17, 2017
@swordmaster2k
Copy link

swordmaster2k commented Nov 23, 2017

I'm also actively using the functionality provided by this branch, the on_stop functions exactly how I'd expect it to. I have a websocket associated with each locust running own their on greenlet so I needed some way to close the websockets when the controlling locust was stopped. This branch provides that ability, would be nice to see it merged into the official branch.

@aldenpeterson-wf
Copy link
Contributor

this LGTM.

@heyman @cgbystrom @mbeacom any of you have thoughts on this? It'd be nice to get this into the 0.9.0 release.

@mbeacom
Copy link
Member

mbeacom commented Mar 19, 2018

@aldenpeterson-wf Unless there are any objections, lgtm. We'll wait a little and if no one speaks up, maybe merge it this week?

Copy link
Member

@cgoldberg cgoldberg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: in the documentation, can you change the wording so it refers to on_start/on_stop as "methods" rather than "functions"?

@codecov
Copy link

codecov bot commented Mar 20, 2018

Codecov Report

❗ No coverage uploaded for pull request base (master@2a826de). Click here to learn what that means.
The diff coverage is 44.44%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #658   +/-   ##
=========================================
  Coverage          ?   65.13%           
=========================================
  Files             ?       14           
  Lines             ?     1417           
  Branches          ?      222           
=========================================
  Hits              ?      923           
  Misses            ?      443           
  Partials          ?       51
Impacted Files Coverage Δ
locust/main.py 26.16% <0%> (ø)
locust/core.py 78.97% <46.34%> (ø)
locust/runners.py 51.31% <50%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2a826de...1b4a49a. Read the comment docs.

@DeepHorizons
Copy link
Contributor Author

@cgoldberg Changed, while I'm at it should I change the language in the rest of the docs?

@cgoldberg
Copy link
Member

cgoldberg commented Mar 20, 2018 via email

@mbeacom mbeacom added LGTM and removed needs review labels Mar 20, 2018
Copy link
Member

@cgoldberg cgoldberg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@mbeacom mbeacom merged commit 524ab52 into locustio:master Mar 20, 2018
@cgoldberg
Copy link
Member

@DeepHorizons thanks for the contribution!

@aldenpeterson-wf
Copy link
Contributor

@heyman you want to cut another release? This is pretty great functionality and combined with the time based run worthy of releasing imo.

@cgoldberg
Copy link
Member

+1 for release. We also need to add a nice description of this to /docs/changelog.rst.

@migueleliasweb
Copy link

Waiting anxiously

@migueleliasweb
Copy link

@cgoldberg How's things ? Do you think that it will take long before releasing this ?

@cgoldberg
Copy link
Member

@migueleliasweb
we are going to cut a release candidate (hopefully soon) ... but not sure the timeline yet for the official 0.9 release.

@cgoldberg cgoldberg added this to the 0.9.0 milestone Apr 24, 2018
@heyman
Copy link
Member

heyman commented Aug 29, 2018

Hey! I realize that this comment is way late, but as far as I can tell from the code, the setup/teardown will run multiple times when running Locust distributed. I think we either need to make sure this is not the case (perhaps by only running the setup & teardown in the master node), or at least update the documentation which currently states that the setup/teardown is run only once. Though I think it's much preferable if Locust works the same wether it's being run distributed or not.

@andrerivas
Copy link

Hello! Is there any way for a Locust script to know if it is running as a Master or a Slave? Knowing that would help when writing the setup() function to ensure a database setup command only runs one time.

@stefanbie
Copy link

As far as I can tell Locust teardown is not working in 0.9.0.

@stefanbie
Copy link

Sorry, I was wrong, it actually works, great work guys!!! :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants