Skip to content
This repository has been archived by the owner on Aug 2, 2022. It is now read-only.

fix failed integration cases #385

Merged
merged 3 commits into from
Feb 10, 2021

Conversation

ylwu-amzn
Copy link
Contributor

@ylwu-amzn ylwu-amzn commented Feb 10, 2021

Issue #, if available:

Description of changes:

Fix flaky integration test cases

Test

  1. Start local cluster with security plugin and run ./gradlew integTest -Dtests.rest.cluster=localhost:9200 -Dtests.cluster=localhost:9200 -Dtests.clustername="docker-cluster" -Dhttps=true -Dsecurity=true -Duser=admin -Dpassword=admin
  2. Start local cluster without security plugin and run ./gradlew integTest -Dtests.rest.cluster=localhost:9200 -Dtests.cluster=localhost:9200 -Dtests.clustername="docker-cluster" -Dhttps=false and ./gradlew integTest -Dtests.rest.cluster=localhost:9200 -Dtests.cluster=localhost:9200 -Dtests.clustername="docker-cluster"
  3. ./gradlew build
  4. ./gradlew integTest -PnumNodes=3
    By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@codecov
Copy link

codecov bot commented Feb 10, 2021

Codecov Report

Merging #385 (c465e6a) into main (c0962ea) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##               main     #385   +/-   ##
=========================================
  Coverage     79.07%   79.07%           
  Complexity     2658     2658           
=========================================
  Files           247      247           
  Lines         11717    11717           
  Branches       1008     1008           
=========================================
  Hits           9265     9265           
  Misses         1971     1971           
  Partials        481      481           
Flag Coverage Δ Complexity Δ
cli 79.27% <ø> (ø) 0.00 <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

createRoleMapping("anomaly_read_access", new ArrayList<>(Arrays.asList(bobUser)));
createRoleMapping("anomaly_full_access", new ArrayList<>(Arrays.asList(aliceUser, catUser, dogUser)));
createRoleMapping("index_all_access", new ArrayList<>(Arrays.asList(aliceUser, bobUser, catUser, dogUser)));
if (isHttps()) {
Copy link
Contributor

@saratvemulapalli saratvemulapalli Feb 10, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Had an offline chat with @ylwu-amzn .
We intentionally wanted to run these tests if the https flag is set from the arguments. (Refer to comments on original PR: #331 (comment))
We set this in build.gradle: https://github.com/opendistro-for-elasticsearch/anomaly-detection/blob/main/build.gradle#L168-L172
Probably as we discussed the cleaner way is to add another case of false to the build.gradle script instead of adding isHttps() check for all of tests.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exclude SecureADRestIT if https is false in case someone test with "-Dhttps=false"

@ylwu-amzn ylwu-amzn merged commit 72b7f4a into opendistro-for-elasticsearch:main Feb 10, 2021
ylwu-amzn added a commit to ylwu-amzn/anomaly-detection that referenced this pull request Feb 10, 2021
ylwu-amzn added a commit that referenced this pull request Feb 10, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants