Skip to content

Commit

Permalink
Cleanup TESTING and DEVELOPER_GUIDE markdowns (#946)
Browse files Browse the repository at this point in the history
  • Loading branch information
dblock authored Jul 12, 2021
1 parent 6252370 commit 88f17b1
Show file tree
Hide file tree
Showing 6 changed files with 13 additions and 26 deletions.
2 changes: 1 addition & 1 deletion CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ If you've thought of a way that OpenSearch could be better, we want to hear abou

### Documentation Changes

If you would like to contribute to the documentation, please see [OpenSearch Docs repo](https://github.com/opensearch-project/documentation-website/blob/main/README.md). To contribute javadocs, please see [this open issue on github](https://github.com/opensearch-project/OpenSearch/issues/221).
If you would like to contribute to the documentation, please do so in the [documentation-website](https://github.com/opensearch-project/documentation-website) repo. To contribute javadocs, please first check [OpenSearch#221](https://github.com/opensearch-project/OpenSearch/issues/221).

### Contributing Code

Expand Down
7 changes: 1 addition & 6 deletions DEVELOPER_GUIDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
- [Visual Studio Code](#visual-studio-code)
- [Eclipse](#eclipse)
- [Project Layout](#project-layout)
- [`docs`](#docs)
- [`distribution`](#distribution)
- [`libs`](#libs)
- [`modules`](#modules)
Expand Down Expand Up @@ -154,10 +153,6 @@ We would like to support Eclipse, but few of us use it and has fallen into disre

This repository is split into many top level directories. The most important ones are:

### `docs`

Documentation for the project.

### `distribution`

Builds our tar and zip archives and our rpm and deb packages.
Expand All @@ -175,7 +170,7 @@ For example, reindex requires the `connect` permission so it can perform reindex

### `plugins`

Officially supported plugins to OpenSearch. We decide that a feature should be a plugin rather than shipped as a module because we feel that it is only important to a subset of users, especially if it requires extra dependencies.
OpenSearch plugins. We decide that a feature should be a plugin rather than shipped as a module because we feel that it is only important to a subset of users, especially if it requires extra dependencies.

The canonical example of this is the ICU analysis plugin. It is important for folks who want the fairly language neutral ICU analyzer but the library to implement the analyzer is 11MB so we don't ship it with OpenSearch by default.

Expand Down
25 changes: 9 additions & 16 deletions TESTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -34,15 +34,14 @@ OpenSearch uses [jUnit](https://junit.org/junit5/) for testing, it also uses ran
- [Test coverage analysis](#test-coverage-analysis)
- [Building with extra plugins](#building-with-extra-plugins)
- [Environment misc](#environment-misc)
- [Benchmarking](#benchmarking)

# Requirements

You will need the following pieces of software to run these tests:

- Docker & Docker Compose
- Vagrant
- JDK 14
- JDK 11
- Gradle

# Creating packages
Expand Down Expand Up @@ -372,13 +371,13 @@ A specific version can be tested as well. For example, to test bwc with version

./gradlew v5.3.2#bwcTest

Use -Dtest.class and -Dtests.method to run a specific bwcTest test. For example to run a specific tests from the x-pack rolling upgrade from 7.7.0:
Use -Dtest.class and -Dtests.method to run a specific bwcTest test. For example to test a rolling upgrade from 7.7.0:

./gradlew :x-pack:qa:rolling-upgrade:v7.7.0#bwcTest \
-Dtests.class=org.opensearch.upgrades.UpgradeClusterClientYamlTestSuiteIT \
-Dtests.method="test {p0=*/40_ml_datafeed_crud/*}"
./gradlew :qa:rolling-upgrade:v7.7.0#bwcTest \
-Dtests.class=org.opensearch.upgrades.RecoveryIT \
-Dtests.method=testHistoryUUIDIsGenerated

Tests are run for versions that are not yet released but with which the current version will be compatible with. These are automatically checked out and built from source. See [VersionCollection](./buildSrc/src/main/java/org/opensearch/gradle/VersionCollection.java) and [distribution/bwc/build.gradle](./distribution/bwc/build.gradle) for more information.
Tests are run for versions that are not yet released but with which the current version will be compatible with. These are automatically checked out and built from source. See [BwcVersions](./buildSrc/src/main/java/org/opensearch/gradle/BwcVersions.java) and [distribution/bwc/build.gradle](./distribution/bwc/build.gradle) for more information.

When running `./gradlew check`, minimal bwc checks are also run against compatible versions that are not yet released.

Expand Down Expand Up @@ -424,7 +423,7 @@ In short, most new functionality should come with unit tests, and optionally RES

### Refactor code to make it easier to test

Unfortunately, a large part of our code base is still hard to unit test. Sometimes because some classes have lots of dependencies that make them hard to instantiate. Sometimes because API contracts make tests hard to write. Code refactors that make functionality easier to unit test are encouraged. If this sounds very abstract to you, you can have a look at [this pull request](https://github.com/elastic/elasticsearch/pull/16610) for instance, which is a good example. It refactors `IndicesRequestCache` in such a way that: - it no longer depends on objects that are hard to instantiate such as `IndexShard` or `SearchContext`, - time-based eviction is applied on top of the cache rather than internally, which makes it easier to assert on what the cache is expected to contain at a given time.
Unfortunately, a large part of our code base is still hard to unit test. Sometimes because some classes have lots of dependencies that make them hard to instantiate. Sometimes because API contracts make tests hard to write. Code refactors that make functionality easier to unit test are encouraged.

## Bad practices

Expand All @@ -442,13 +441,11 @@ Multi-threaded tests are often not reproducible due to the fact that there is no

Generating test coverage reports for OpenSearch is currently not possible through Gradle. However, it *is* possible to gain insight in code coverage using IntelliJ’s built-in coverage analysis tool that can measure coverage upon executing specific tests. Eclipse may also be able to do the same using the EclEmma plugin.

Test coverage reporting used to be possible with JaCoCo when OpenSearch was using Maven as its build system. Since the switch to Gradle though, this is no longer possible, seeing as the code currently used to build OpenSearch does not allow JaCoCo to recognize its tests. For more information on this, see the discussion in [issue #28867](https://github.com/elastic/elasticsearch/issues/28867).

Read your IDE documentation for how to attach a debugger to a JVM process.
Please read your IDE documentation for how to attach a debugger to a JVM process.

# Building with extra plugins

Additional plugins may be built alongside OpenSearch, where their dependency on OpenSearch will be substituted with the local OpenSearch build. To add your plugin, create a directory called opensearch-extra as a sibling of OpenSearch. Checkout your plugin underneath opensearch-extra and the build will automatically pick it up. You can verify the plugin is included as part of the build by checking the projects of the build.
Additional plugins may be built alongside OpenSearch, where their dependency on OpenSearch will be substituted with the local OpenSearch build. To add your plugin, create a directory called `opensearch-extra` as a sibling of OpenSearch. Checkout your plugin underneath `opensearch-extra` and the build will automatically pick it up. You can verify the plugin is included as part of the build by checking the projects of the build.

./gradlew projects

Expand All @@ -459,7 +456,3 @@ There is a known issue with macOS localhost resolve strategy that can cause some
127.0.0.1 localhost OpenSearchMBP.local
255.255.255.255 broadcasthost
::1 localhost OpenSearchMBP.local`

# Benchmarking

For changes that might affect the performance characteristics of OpenSearch you should also run macrobenchmarks. There is also a macrobenchmarking tool called [Rally](https://github.com/elastic/rally) which you can use to measure the performance impact. To get started, please see [Rally’s documentation](https://esrally.readthedocs.io/en/stable/).
1 change: 0 additions & 1 deletion buildSrc/version.properties
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ joda = 2.10.4
# when updating this version, you need to ensure compatibility with:
# - plugins/ingest-attachment (transitive dependency, check the upstream POM)
# - distribution/tools/plugin-cli
# - x-pack/plugin/security
bouncycastle=1.64
# test dependencies
randomizedrunner = 2.7.1
Expand Down
2 changes: 1 addition & 1 deletion qa/translog-policy/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ apply from : "$rootDir/gradle/bwc-test.gradle"
for (Version bwcVersion : BuildParams.bwcVersions.indexCompatible) {
if (bwcVersion.before('6.3.0')) {
// explicitly running restart on the current node does not work in step 2
// below when plugins are installed, wihch is the case for x-pack as a plugin
// below when plugins are installed, which is the case for some plugins
// prior to 6.3.0
continue
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -362,7 +362,7 @@ private static boolean addIndex(IndexMetadata metadata, Context context) {
// trappy to hide throttled indices by default. In order to avoid breaking backward compatibility,
// we changed it to look at the `index.frozen` setting instead, since frozen indices were the only
// type of index to use the `search_throttled` threadpool at that time.
// NOTE: We can't reference the Setting object, which is only defined and registered in x-pack.
// NOTE: The Setting object was defined in an external plugin prior to OpenSearch fork.
return (context.options.ignoreThrottled() && metadata.getSettings().getAsBoolean("index.frozen", false)) == false;
}

Expand Down

0 comments on commit 88f17b1

Please sign in to comment.