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

[Backport-1.x] Cleanup TESTING and DEVELOPER_GUIDE markdowns (#946) #954

Merged
merged 1 commit into from
Jul 13, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 11 additions & 5 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,16 @@ Contributing to OpenSearch
OpenSearch is a community project that is built and maintained by people just like **you**. We're glad you're interested in helping out. There are several different ways you can do it, but before we talk about that, let's talk about how to get started.

## Table of Contents:
- [First Things First](#first-things-first)
- [Ways to Contribute](#ways-to-contribute)
- [Developer Certificate of Origin](#developer-certificate-of-origin)
- [Review Process](#review-process)
- [Contributing to OpenSearch](#contributing-to-opensearch)
- [Table of Contents:](#table-of-contents)
- [First Things First](#first-things-first)
- [Ways to Contribute](#ways-to-contribute)
- [Bug Reports](#bug-reports)
- [Feature Requests](#feature-requests)
- [Documentation Changes](#documentation-changes)
- [Contributing Code](#contributing-code)
- [Developer Certificate of Origin](#developer-certificate-of-origin)
- [Review Process](#review-process)


## First Things First
Expand Down Expand Up @@ -52,7 +58,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).
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
43 changes: 23 additions & 20 deletions DEVELOPER_GUIDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,32 +2,39 @@

So you want to contribute code to OpenSearch? Excellent! We're glad you're here. Here's what you need to do.

- [Getting Started](#getting-started)
- [Developer Guide](#developer-guide)
- [Getting Started](#getting-started)
- [Git Clone OpenSearch Repo](#git-clone-opensearch-repo)
- [Install Prerequisites](#install-prerequisites)
- [JDK 14](#jdk-14)
- [Docker](#docker)
- [Run Tests](#run-tests)
- [Run OpenSearch](#run-opensearch)
- [Use an Editor](#use-an-editor)
- [Use an Editor](#use-an-editor)
- [IntelliJ IDEA](#intellij-idea)
- [Visual Studio Code](#visual-studio-code)
- [Eclipse](#eclipse)
- [Project Layout](#project-layout)
- [docs](#docs)
- [distribution](#distribution)
- [libs](#libs)
- [modules](#modules)
- [plugins](#plugins)
- [qa](#qa)
- [server](#server)
- [test](#test)
- [Java Language Formatting Guidelines](#java-language-formatting-guidelines)
- [Project Layout](#project-layout)
- [`distribution`](#distribution)
- [`libs`](#libs)
- [`modules`](#modules)
- [`plugins`](#plugins)
- [`qa`](#qa)
- [`server`](#server)
- [`test`](#test)
- [Java Language Formatting Guidelines](#java-language-formatting-guidelines)
- [Editor / IDE Support](#editor--ide-support)
- [Formatting Failures](#formatting-failures)
- [Gradle Build](#gradle-build)
- [Gradle Build](#gradle-build)
- [Configurations](#configurations)
- [Misc](#misc)
- [implementation](#implementation)
- [api](#api)
- [runtimeOnly](#runtimeonly)
- [compileOnly](#compileonly)
- [testImplementation](#testimplementation)
- [Misc](#misc)
- [git-secrets](#git-secrets)
- [Submitting Changes](#submitting-changes)
- [Submitting Changes](#submitting-changes)

## Getting Started

Expand Down Expand Up @@ -156,10 +163,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 @@ -177,7 +180,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 @@ -368,13 +367,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 @@ -420,7 +419,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 @@ -438,13 +437,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 @@ -455,7 +452,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