Skip to content

Commit

Permalink
Update BWC Test Version and Enhance Code Coverage (#1253)
Browse files Browse the repository at this point in the history
* Update BWC Test Version and Enhance Code Coverage

This PR updates the BWC test version to 2.16 in the build.gradle file and includes additional integration and unit tests to improve code coverage.

Testing Performed:
* Executed gradle build to ensure successful build and integration.

Signed-off-by: Kaituo Li <kaituo@amazon.com>

* address comments

Signed-off-by: Kaituo Li <kaituo@amazon.com>

---------

Signed-off-by: Kaituo Li <kaituo@amazon.com>
  • Loading branch information
kaituo committed Jun 25, 2024
1 parent 3f0fc8c commit 08f23fa
Show file tree
Hide file tree
Showing 26 changed files with 1,564 additions and 483 deletions.
4 changes: 2 additions & 2 deletions .github/workflows/benchmark.yml
Original file line number Diff line number Diff line change
Expand Up @@ -58,12 +58,12 @@ jobs:
su `id -un 1000` -c "./gradlew ':test' --tests 'org.opensearch.ad.ml.HCADModelPerfTests' \
-Dtests.seed=2AEBDBBAE75AC5E0 -Dtests.security.manager=false \
-Dtests.locale=es-CU -Dtests.timezone=Chile/EasterIsland -Dtest.logs=true \
-Dmodel-benchmark=true"
-Dtests.timeoutSuite=3600000! -Dmodel-benchmark=true"
;;
single_stream)
su `id -un 1000` -c "./gradlew integTest --tests 'org.opensearch.ad.e2e.SingleStreamModelPerfIT' \
-Dtests.seed=60CDDB34427ACD0C -Dtests.security.manager=false \
-Dtests.locale=kab-DZ -Dtests.timezone=Asia/Hebron -Dtest.logs=true \
-Dmodel-benchmark=true"
-Dtests.timeoutSuite=3600000! -Dmodel-benchmark=true"
;;
esac
6 changes: 3 additions & 3 deletions .github/workflows/test_security.yml
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,9 @@ jobs:
- name: Pull and Run Docker
run: |
plugin=`basename $(ls build/distributions/*.zip)`
version=`echo $plugin|awk -F- '{print $5}'| cut -d. -f 1-3`
plugin_version=`echo $plugin|awk -F- '{print $5}'| cut -d. -f 1-4`
qualifier=`echo $plugin|awk -F- '{print $6}'| cut -d. -f 1-1`
version=`echo $plugin|awk -F- '{print $4}'| cut -d. -f 1-3`
plugin_version=`echo $plugin|awk -F- '{print $4}'| cut -d. -f 1-4`
qualifier=`echo $plugin|awk -F- '{print $5}'| cut -d. -f 1-1`
if $qualifier!=SNAPSHOT
then
Expand Down
49 changes: 49 additions & 0 deletions build-tools/coverage.gradle
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
/*
* Copyright OpenSearch Contributors
* SPDX-License-Identifier: Apache-2.0
*/

apply plugin: 'jacoco'

jacoco {
toolVersion = "0.8.10"
}

/**
* This code sets up coverage reporting manually for the AD plugin tests. This is complicated because:
* 1. The OS integTest Task doesn't implement Gradle's JavaForkOptions so we have to manually start the jacoco agent with the test JVM
* 2. The cluster nodes are stopped using 'kill -9' which means jacoco can't dump it's execution output to a file on VM shutdown
* 3. The Java Security Manager prevents JMX from writing execution output to the file.
*
* To workaround these we start the cluster with jmx enabled and then use Jacoco's JMX MBean to get the execution data before the
* cluster is stopped and dump it to a file. Luckily our current security policy seems to allow this. This will also probably
* break if there are multiple nodes in the integTestCluster. But for now... it sorta works.
*/
integTest {
jacoco {
jmx = true
}

systemProperty 'jacoco.dir', project.layout.buildDirectory.get().file("jacoco").asFile.absolutePath
systemProperty 'jmx.serviceUrl', "service:jmx:rmi:///jndi/rmi://127.0.0.1:7777/jmxrmi"
}

jacocoTestReport {
dependsOn integTest, test
executionData.from = [integTest.jacoco.destinationFile, test.jacoco.destinationFile]
reports {
html.getRequired().set(true) // human readable
csv.getRequired().set(true)
xml.getRequired().set(true) // for coverlay
}
}

testClusters.integTest {
jvmArgs " ${integTest.jacoco.getAsJvmArg()}"

systemProperty 'com.sun.management.jmxremote', "true"
systemProperty 'com.sun.management.jmxremote.authenticate', "false"
systemProperty 'com.sun.management.jmxremote.port', "7777"
systemProperty 'com.sun.management.jmxremote.ssl', "false"
systemProperty 'java.rmi.server.hostname', "127.0.0.1"
}
18 changes: 10 additions & 8 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ buildscript {
js_resource_folder = "src/test/resources/job-scheduler"
common_utils_version = System.getProperty("common_utils.version", opensearch_build)
job_scheduler_version = System.getProperty("job_scheduler.version", opensearch_build)
bwcVersionShort = "2.15.0"
bwcVersionShort = "2.16.0"
bwcVersion = bwcVersionShort + ".0"
bwcOpenSearchADDownload = 'https://ci.opensearch.org/ci/dbc/distribution-build-opensearch/' + bwcVersionShort + '/latest/linux/x64/tar/builds/' +
'opensearch/plugins/opensearch-anomaly-detection-' + bwcVersion + '.zip'
Expand Down Expand Up @@ -662,6 +662,13 @@ task release(type: Copy, group: 'build') {
eachFile { it.path = it.path - "opensearch/" }
}

def usingRemoteCluster = System.properties.containsKey('tests.rest.cluster') || System.properties.containsKey('tests.cluster')
def usingMultiNode = project.properties.containsKey('numNodes')
// Only apply jacoco test coverage if we are running a local single node cluster
if (!usingRemoteCluster && !usingMultiNode) {
apply from: 'build-tools/coverage.gradle'
}

List<String> jacocoExclusions = [
// code for configuration, settings, etc is excluded from coverage
'org.opensearch.timeseries.TimeSeriesAnalyticsPlugin',
Expand Down Expand Up @@ -701,6 +708,8 @@ List<String> jacocoExclusions = [


jacocoTestCoverageVerification {
dependsOn(jacocoTestReport)
executionData.from = [integTest.jacoco.destinationFile, test.jacoco.destinationFile]
violationRules {
rule {
element = 'CLASS'
Expand All @@ -722,13 +731,6 @@ jacocoTestCoverageVerification {
}
}

jacocoTestReport {
reports {
xml.required = true // for coverlay
html.required = true // human readable
}
}

check.dependsOn jacocoTestCoverageVerification
jacocoTestCoverageVerification.dependsOn jacocoTestReport

Expand Down
2 changes: 1 addition & 1 deletion dataGeneration/requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,4 @@ numpy==1.23.0
opensearch_py==2.0.0
retry==0.9.2
scipy==1.10.0
urllib3==1.26.18
urllib3==1.26.19
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,6 @@ public class ForecastEnabledSetting extends DynamicNumericSetting {
*/
public static final String FORECAST_ENABLED = "plugins.forecast.enabled";

public static final boolean enabled = false;

public static final Map<String, Setting<?>> settings = unmodifiableMap(new HashMap<String, Setting<?>>() {
{
/**
Expand All @@ -55,8 +53,6 @@ public static synchronized ForecastEnabledSetting getInstance() {
* @return whether forecasting is enabled.
*/
public static boolean isForecastEnabled() {
// return ForecastEnabledSetting.getInstance().getSettingValue(ForecastEnabledSetting.FORECAST_ENABLED);
// TODO: enable forecasting before released
return enabled;
return ForecastEnabledSetting.getInstance().getSettingValue(ForecastEnabledSetting.FORECAST_ENABLED);
}
}
4 changes: 4 additions & 0 deletions src/main/java/org/opensearch/timeseries/JobProcessor.java
Original file line number Diff line number Diff line change
Expand Up @@ -579,5 +579,9 @@ private void releaseLock(Job jobParameter, LockService lockService, LockModel lo
);
}

public Integer getEndRunExceptionCount(String configId) {
return endRunExceptionCount.getOrDefault(configId, 0);
}

protected abstract ResultRequest createResultRequest(String configID, long start, long end);
}
41 changes: 0 additions & 41 deletions src/main/java/org/opensearch/timeseries/util/TaskUtil.java

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -122,8 +122,6 @@ public void setUp() throws Exception {
clientUtil = new SecurityClientUtil(nodeStateManager, settings);
transportService = mock(TransportService.class);

// channel = mock(ActionListener.class);

anomalyDetectionIndices = mock(ADIndexManagement.class);
when(anomalyDetectionIndices.doesConfigIndexExist()).thenReturn(true);

Expand Down
Loading

0 comments on commit 08f23fa

Please sign in to comment.