-
-
Notifications
You must be signed in to change notification settings - Fork 746
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
Add pants-plugins/uses_services
to check before running tests for required services (like mongo)
#5864
Conversation
pants-plugins/uses_services
to check before running tests for required services (like mongo)
ea26379
to
7671c08
Compare
@cognifloyd Is this ready for review? One test seems to be still failing |
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.
This needs some more work. There are a few bits I could use some help on (see the comment about getting platform samples).
This plugin is not necessary to release wheels. It is strictly to improve Developer Experience. So, I'm working on a pack_metadata
plugin which is needed for that. I'll come back to this plugin later.
Also, I had a lot going on this past week so I wasn't able to work on the pants integration much. Hopefully I can wrap up the branches for the next 2 plugins this week.
833df33
to
451a4e6
Compare
To assert services (mongo) are running before running tests that require a database or similar.
Reintroduce windows platofrm tests if someone submits a sample.
070cf24
to
ffe161f
Compare
rebased |
Background
This is another part of introducing pants, as discussed in various TSC meetings.
Related PRs can be found in:
Overview of this PR
This PR improves the DX (developer experience) by failing as early as possible if the development (or CI) environment does not have the required services.
This PR only checks for
mongo
, but I have draft branches to also addrabbitmq
andredis
.Developer Experience
I do local development on my linux and mac laptops. Others use vagrant. Wherever you have your st2 sources, you must have mongo running in order to run some of our tests. I don't normally run mongo, so I often forget to start it before running tests. I would rather the tests bail out if the service is not running instead of giving me tons of logs to sift through to realize that mongo isn't running.
So, this PR focuses on failing as early as possible if the required services aren't running. On failure, the error message should be as actionable as possible to help guide new contributors to do the right thing without having to search our docs or ask for help in slack.
This does not run over and over. Pants has a daemon,
pantsd
, that runs in the background to handle results caching of the various test runs. Theis_mongo_running
check will re-run whenever pantsd restarts.Developing pants plugins
Pants has extensive documentation on the plugin API, including how to create targets, how to write rules, and there's even a mini tutorial about how to add a formatter (adding formatters is a very common thing for plugins to do).
Architecture of the
uses_services
plugincontents in this section:
uses
fieldServiceMissingError
when the service is not runningPlatform
dataclass and the rule andinspect_platform.py
script that creates itis_mongo_running.py
script and the rule that runs ittest
goal so it runsis_mongo_running
whenpytest
runs on a target with theuses
field.uses
fieldIn BUILD files, we can add the
uses
field like this to say that these tests "use" mongo (so mongo should be running):st2/st2common/tests/unit/services/BUILD
Lines 1 to 4 in 89b6960
This field is defined here:
st2/pants-plugins/uses_services/target_types.py
Lines 20 to 23 in 89b6960
And gets added to the
python_test
andpython_tests
targets here:st2/pants-plugins/uses_services/register.py
Lines 25 to 26 in 89b6960
raise
ServiceMissingError
when the service is not runningWhen a target has the
uses
field, some rules (described below) handle checking for running services like mongo. When the service is not running, the rule raises aServiceMissingError
which is defined here:st2/pants-plugins/uses_services/exceptions.py
Lines 38 to 51 in 89b6960
Wherever possible, the exception should include general instructions to help guide the developer towards getting the service running. Those instructions are platform-specific (eg development on Linux vs on Mac, or on Ubuntu vs on Rocky). So, the error needs to now the
service
name (egmongo
), aPlatform
descriptor dataclass (described below), and the instructions to show.As I wrote the instructions for
mongo
,rabbitmq
, andredis
, I found that I was basically copy/pasting the same message, which meant that if I wanted to reword something, I had to change 3 copies of it, which is not pleasant. So, I wrote a little util,ServiceMissingError.generate()
that generates an error with standardized instructions. Theplatform
argument is the same as what theServiceMissingError
requires (described below) and themessages
argument takes a dataclass with all of the strings that need to be interpolated in those standard instructions. That class is defined here:st2/pants-plugins/uses_services/exceptions.py
Lines 22 to 35 in 89b6960
Here is what raising the error looks like for
mongo
:st2/pants-plugins/uses_services/mongo_rules.py
Lines 161 to 196 in 89b6960
Please review the messages that these strings get interpolated into in
exceptions.py
. These are effectively documentation to help developers have a better experience.Platform
dataclass and the rule script that creates itHere is the platform dataclass:
st2/pants-plugins/uses_services/scripts/inspect_platform.py
Lines 21 to 32 in 89b6960
inspect_platform.py
is a simple script that collects the data about the running arch, os, and distro.The rule that runs this script to collect the
Platform
details is defined inplatform_rules.py
Normally, direct file access is a bad idea in pants rules. However, after experimentation, that turned out to be the cleanest/quickest way to handle loading a python file that is part of the plugin. In most rules, pants keeps track of which files that rule needs so it can watch for changes and restart the rule if that file changes. Because the file in question is part of the plugin that needs to use it, pants is already watching the file, so using a pants
Get
request is superfluous. So the rule grabs the contents of the script like this:st2/pants-plugins/uses_services/platform_rules.py
Lines 44 to 47 in 89b6960
Then it runs that script in a pex, which gets cached once per session (which is as long as
pantsd
is still running) according to this:st2/pants-plugins/uses_services/platform_rules.py
Lines 71 to 72 in 89b6960
And then creates the
Platform
here:st2/pants-plugins/uses_services/platform_rules.py
Lines 76 to 77 in 89b6960
is_mongo_running.py
script and the rule that runs itFinally,
is_mongo_running.py
is the part that checks to see if mongo is running.The pants plugin cannot import anything from our other
st2*
code, sois_mongo_running.py
is a minimal self-contained script that mirrors how st2 connects to mongo. It should be as minimal as possible so that keeping it up-to-date with the core st2 code is not onerous.The
is_mongo_running.py
rule gets opened and run in a pex with the same rule logic asinspect_platform.py
(see above).I avoided handling any kind of mongo auth; instead I had it use a mongo command that does not require auth, but still guarantees that mongo is actually running and accessible. For now, all of the tests hard-code the credentials and mongo connection settings like ssl. I do not expect this script to support auth in the future, but we might need to add TLS and other connection settings so we can support whatever mongo instance people want to use to run tests when developing locally. For now, it only supports the database connection settings hard-coded in the tests. Here is where the rule defines the default connection settings:
st2/pants-plugins/uses_services/mongo_rules.py
Lines 46 to 68 in 89b6960
Here is the definition of the rule that runs
is_mongo_running.py
:st2/pants-plugins/uses_services/mongo_rules.py
Lines 116 to 118 in 89b6960
So, when another rule Gets
MongoIsRunning
with aUsesMongoRequest
, pants will also run the rule that generatesPlatform
(described above), and then it will run thisis_mongo_running
rule.The
is_mongo_running
rule either returnsMongoIsRunning()
if it is running, or raisesServiceMissingError
if it is not. By raising an error, this actually breaks a convention for pants rules. Exceptions stop everything and get shown to the user right away, and for most goals, pants wants to see some dataclass returned that has something like asucceeded
boolean instead. But, we want to error as early as possible, so this breaks that convention on purpose.wiring up the
test
goal so it runsis_mongo_running
whenpytest
runs on a target with theuses
field.The last piece that ties this all together is a rule that makes sure the
is_mongo_running
rule runs beforepytest
runs (if pants is running it on a target with theuses
field). Here is the definition of themongo_is_running_for_pytest
rule:st2/pants-plugins/uses_services/mongo_rules.py
Lines 89 to 91 in 89b6960
This rule is very simple. It just triggers running the
is_mongo_running
rule:st2/pants-plugins/uses_services/mongo_rules.py
Lines 106 to 109 in 89b6960
This rule needs the
PytestUsesMongoRequest
which selects targets that have theuses
field.st2/pants-plugins/uses_services/mongo_rules.py
Lines 76 to 82 in 89b6960
This request will be made by the pants-internal pytest rules thanks to this
UnionRule
, which is a way to declare that our class "implements" thePytestPluginSetupRequest
:st2/pants-plugins/uses_services/mongo_rules.py
Line 202 in 89b6960
If we need to add
uses
support to other targets, we will need to find similar UnionRules where we can inject our rule logic. For now, I've only wired this up so ouruses
rules will run for pytest.testing the rules and scripts
I collected a set of
Platform
samples from a variety of machines, including the 4 OSes we officially support, for use in tests. We will need them for several tests, so I put them indata_fixtures.py
Then there are some tests that test running the scripts as part of testing the rules that run them.
There is at least one test for every new pants rule.