Repository: getmoto/moto
Pull request: Update CONTRIBUTING.md
Fixes #4275 

@kbalk Some improvements to the Contributing document, following your suggestions.  Hopefully this makes onboarding easier for new contributors. Let me know what you think.

Problem statement:
Possible additions to CONTRIBUTING.md
Some suggestions for `CONTRIBUTING.md` to help contributors to moto:

- Promote the use of TaggingService.
- Describe how to run in test server mode and the environment variable `TEST_SERVER_MODE`.  You can work it out by looking at the github workflow file, but it might be nice to spell out the steps.  It's useful to know if your tests fail in server mode.  Better yet, add a rule to the Makefile to invoke it.   
- Within a unit test - don't test an exception to indicate something is not yet implemented.  If an exception is raised that isn't defined in an `exceptions.py` file and a unit test attempts to check that scenario, the test will fail when run in server mode.  Mention the use of `SkipTest`.
- It would be nice to encourage the use of a "NotImplementedError" exception where it makes sense, but it's not something that can be tested in server mode and that lowers your test coverage a little bit.  Maybe a generic error that can be defined with an understanding of `TEST_SERVER_MODE`?
- Provide some general guidelines on how to organize tests - I see monolithic test files and sometime test functions that handle a lot of tests.  I'm used to writing one test per function, but to be consistent, I've been combining multiple tests per API.  It all works, but it would be nice to see test files that aren't so large and test functions that are smaller.
- When importing the backend from another service, declare the import within the block of code where it's needed and not at the top of the file.  I don't know if recent changes have negated that need to improve performance.

These are some of the gotchas I've discovered the hard way or learned though code review comments.

Hints from issue discussion:
Hey @kbalk, thanks for pointing this out - user feedback is always useful.

>
>  If an exception is raised that isn't defined in an exceptions.py file and a unit test attempts to check that scenario, the test will fail when run in server mode.

To turn this into a more general rule:
Always verify exceptions by using `with raises(botocore.exceptions.ClientError)` (or any of it's subclasses), to ensure the exception is thrown in the correct format.

>
> It would be nice to encourage the use of a "NotImplementedError" exception

There is definitely value in letting the user know that something is missing. If we're talking about a specific parameters that are not yet supported, I'd favour a non-intrusive way of letting them know though - maybe just logging a warning.
With a warning, the user might be able to work around unexpected results. With an exception, the user would have to change the parameters to get it to work, which is not possible if that call is part of the business logic.

Completely agree on everything else.

Commit message: Update CONTRIBUTING.md

Patch to review:
diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
index 93b58a21d8c4..19a74608b0ef 100644
--- a/CONTRIBUTING.md
+++ b/CONTRIBUTING.md
@@ -1,3 +1,13 @@
+- [Contributing code](#contributing-code)
+  * [Running the tests locally](#running-the-tests-locally)
+  * [Linting](#linting)
+  * [General notes](#general-notes)
+- [Missing features](#missing-features)
+- [Missing services](#missing-services)
+    + [Generating template code of services.](#generating-template-code-of-services)
+    + [URL Indexing](#url-indexing)
+- [Maintainers](#maintainers)
+
 # Contributing code
 
 Moto has a [Code of Conduct](https://github.com/spulec/moto/blob/master/CODE_OF_CONDUCT.md), you can expect to be treated with respect at all times when interacting with this project.
@@ -11,14 +21,39 @@ You should be able to run `make init` to install the dependencies and then `make
 
 ## Linting
 
-Run `make lint` or `black --check moto tests` to verify whether your code confirms to the guidelines.
-
-## Getting to grips with the codebase
+Ensure that the correct version of black is installed - `black==19.10b0`. (Different versions of black will return different results.)  
+Run `make lint` to verify whether your code confirms to the guidelines.  
+Use `make format` to automatically format your code, if it does not conform to `black`'s rules.
 
-Moto maintains a list of [good first issues](https://github.com/spulec/moto/contribute) which you may want to look at before
-implementing a whole new endpoint.
+## General notes
 
-## Missing features
+Some tips that might help during development:
+ - A dedicated TaggingService exists in `moto.utilities`, to help with storing/retrieving tags for resources. Not all services use it yet, but contributors are encouraged to  use the TaggingService for all new features.  
+ - Our CI runs all tests twice - one normal run, and one run in ServerMode. In ServerMode, Moto is started as a stand-alone Flask server, and all tests are run against this Flask-instance.
+To verify whether your tests pass in ServerMode, you can run the following commands:
+```
+pip install .[all,server]
+moto_server
+TEST_SERVER_MODE=true pytest -sv tests/test_service/..
+```
+ - When writing tests, one test should only verify a single feature/method. I.e., one test for `create_resource()`, another for `update_resource()`, etc.
+ - When writing negative tests, try to use the following format:
+ ```
+ with pytest.raises(botocore.exceptions.ClientError) as ex:
+     client.failing_call(..)
+ err = ex.value.response["Error"]
+ err["Code"].should.equal(..)
+ err["Message"].should.equal(..)
+ ```
+ This is the best way to ensure that exceptions are dealt with correctly by Moto.
+  - If a service is only partially implemented, a warning can be used to inform the user. For instance:
+  ```
+  import warnings
+  warnings.warn("The Filters-parameter is not yet implemented for client.method()")
+  ```
+
+
+# Missing features
 
 Moto is easier to contribute to than you probably think. There's [a list of which endpoints have been implemented](https://github.com/spulec/moto/blob/master/IMPLEMENTATION_COVERAGE.md) and we invite you to add new endpoints to existing services or to add new services.
 
@@ -31,12 +66,7 @@ How to teach Moto to support a new AWS endpoint:
 * Implementing the feature itself can be done by creating a method called `import_certificate` in `moto/acm/responses.py`. It's considered good practice to deal with input/output formatting and validation in `responses.py`, and create a method `import_certificate` in `moto/acm/models.py` that handles the actual import logic.
 * If you can also implement the code that gets that test passing then great! If not, just ask the community for a hand and somebody will assist you.
 
-## Before pushing changes to GitHub
-
-1. Run `black moto/ tests/` over your code to ensure that it is properly formatted
-1. Run `make test` to ensure your tests are passing
-
-## Missing services
+# Missing services
 
 Implementing a new service from scratch is more work, but still quite straightforward. All the code that intercepts network requests to `*.amazonaws.com` is already handled for you in `moto/core` - all that's necessary for new services to be recognized is to create a new decorator and determine which URLs should be intercepted.
 
@@ -84,12 +114,12 @@ You will still need to add the mock into "__init__.py"
 In order to speed up the performance of MotoServer, all AWS URL's that need intercepting are indexed.  
 When adding/replacing any URLs in `{service}/urls.py`, please run `python scripts/update_backend_index.py` to update this index.
 
-## Maintainers
-
-### Releasing a new version of Moto
+# Maintainers
 
-You'll need a PyPi account and a DockerHub account to release Moto. After we release a new PyPi package we build and push the [motoserver/moto](https://hub.docker.com/r/motoserver/moto/) Docker image.
+## Releasing a new version of Moto
 
-* First, `scripts/bump_version` modifies the version and opens a PR
-* Then, merge the new pull request
-* Finally, generate and ship the new artifacts with `make publish`
+* Ensure the CHANGELOG document mentions the new release, and lists all significant changes.
+* Go to the dedicated [Release Action](https://github.com/spulec/moto/actions/workflows/release.yml) in our CI
+* Click 'Run workflow' on the top right
+* Provide the version you want to release
+* That's it - everything else is automated.


Code review comments:
diff_hunk: @@ -11,14 +21,39 @@ You should be able to run `make init` to install the dependencies and then `make
 
 ## Linting
 
-Run `make lint` or `black --check moto tests` to verify whether your code confirms to the guidelines.
-
-## Getting to grips with the codebase
+Ensure that the correct version of black is installed - `black==19.10b0`. (Different versions of black will return different results.)  
+Run `make lint` to verify whether your code confirms to the guidelines.  
+Use `make format` to automatically format your code, if it does not conform to `black`'s rules.
 
-Moto maintains a list of [good first issues](https://github.com/spulec/moto/contribute) which you may want to look at before
-implementing a whole new endpoint.
+## General notes
 
-## Missing features
+Some tips that might help during development:
+ - A dedicated TaggingService exists in `moto.utilities`, to help with storing/retrieving tags for resources. Not all services use it yet, but contributors are encouraged to  use the TaggingService for all new features.  
+ - Our CI runs all tests twice - one normal run, and one run in ServerMode. In ServerMode, Moto is started as a stand-alone Flask server, and all tests are run against this Flask-instance.
+To verify whether your tests pass in ServerMode, you can run the following commands:
+```
+pip install .[all,server]
+moto_server
+TEST_SERVER_MODE=true pytest -sv tests/test_service/..
+```
+ - When writing tests, one test should only verify a single feature/method. I.e., one test for `create_resource()`, another for `update_resource()`, etc.
+ - When writing negative tests, try to use the following format:
+ ```
+ with pytest.raises(botocore.exceptions.ClientError) as ex:
+     client.failing_call(..)
+ err = ex.value.response["Error"]
+ err["Code"].should.equal(..)
+ err["Message"].should.equal(..)
+ ```
path: CONTRIBUTING.md
text: @user1:
pylint expects variables to be at least three characters long, so could you change "ex" to "exc" in the example above?

@user1:
Would you want to allow the possibility of using assert() as well as `sure`?  

It might be worth noting that the above code is using methods from the `sure` library ... I hadn't seen or used that library before this and if I saw that code, I might be a little confused by it.

@author:
Done and done