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 to 2.x] Update BWC Test Version and Enhance Code Coverage (#1253) #1255

Merged
merged 1 commit into from
Jun 26, 2024
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
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"
}
16 changes: 9 additions & 7 deletions build.gradle
Original file line number Diff line number Diff line change
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
Loading