Modified TESTING instructions to clarify use of testing classes #1930
Conversation
|
Can one of the admins verify this patch? |
|
The stuff in |
|
✅ Gradle Check success 2343307b061a3ef0ede99bcb2889207eb4d532c9 |
|
✅ Gradle Check success 42280bc2b878845d84b237d31bb3d37bdb8f029c |
Signed-off-by: Dave Lago <davelago@amazon.com>
davidlago
force-pushed
the
change-testing-recommendations
branch
from
42280bc to
6bf9713
Compare
|
Fixed! Completely missed those when creating the commit. |
| @@ -421,9 +421,11 @@ Unit tests are the preferred way to test some functionality: most of the time th | |||
|
|
|||
| The reason why `OpenSearchSingleNodeTestCase` exists is that all our components used to be very hard to set up in isolation, which had led us to having a number of integration tests but close to no unit tests. `OpenSearchSingleNodeTestCase` is a workaround for this issue which provides an easy way to spin up a node and get access to components that are hard to instantiate like `IndicesService`. Whenever practical, you should prefer unit tests. | |||
|
|
|||
| Many tests extend `OpenSearchIntegTestCase`, mostly because this is how most tests used to work in the early days of Elasticsearch. However, the complexity of these tests tends to make them hard to debug. Whenever the functionality that is being tested isn’t intimately dependent on how OpenSearch behaves as a cluster, it is recommended to write unit tests or REST tests instead. | |||
| Finally, if the the functionality under test needs to be run in a cluster, there are two test classes to consider: | |||
There was a problem hiding this comment.
Can we retain language that communicates how integ tests are very costly and should only be used when cluster testing is unavoidable?
We really want to keep the number of integ tests minimal so check can complete in a "reasonable" time. Long running tests should be marked for nightly only.
There was a problem hiding this comment.
the language is still there in some way in the next paragraph (In short, most new functionality should come with unit tests, and optionally integration tests using either an external cluster or a local one if there's a need for more specific cluster configurations) but perhaps we can make a mention of cost there to emphasize the point... let me update.
Signed-off-by: Dave Lago <davelago@amazon.com>
|
@dreamer-89 here is the pull request I mentioned about test guidance updates. If you have further questions or considerations for your scenarios these are passionate folks and this part of the documentation would be good places to start. |
Signed-off-by: Dave Lago davelago@amazon.com
Description
Updated testing guide to clarify guidance on when to use the different testing classes. This PR is intended to be a vehicle for discussion as this will become the guidance for new tests.
Issues Resolved
N/A
Check List
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.