Add an optional parameter -o to integtest.sh #644

Divyaasm · 2023-04-26T18:36:36Z

Description

A new optional argument OPTION [-o ] is added to integtest.sh

Issues Resolved

Fix to the issue #3316

Call to ./integtest.sh from integ-workflow can now use additional -o default parameter when -t is passed which executes the tests individually instead of altogether. This is optional and does not change the behaviour if -o is not passed.

Check List

Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Signed-off-by: Divya Madala <[email protected]>

ashwin-pc · 2023-04-26T19:17:03Z

integtest.sh

@@ -20,11 +18,12 @@ function usage() {
    echo -e "-c CREDENTIAL\t(usename:password), no defaults, effective when SECURITY_ENABLED=true."
    echo -e "-t TEST_COMPONENTS\t(OpenSearch-Dashboards reportsDashboards etc.), optional, specify test components, separate with space, else test everything."
    echo -e "-v VERSION\t, no defaults, indicates the OpenSearch version to test."
+    echo -e "-o OPTION\t, no defaults, determine the TEST_TYPE value among(default, manifest) in test_finder.sh, optional."


why dont we want a default value here? since one of the accepted value is already default

The background of the issue is that when you the run the OSD integ test by simply checking out the FT repo and directly invoking integtest.sh script the TEST_TYPE is default and if you look at test_finder.sh script, in case the TEST_TYPE is default it acknowledges the component passed using -t flag and runs the test for that particular component.

But in case the OSD integ test is triggered using the integ-test workflow the TEST_TYPE is manifest and the test_finder.sh script doesn't acknowledge the passed component and runs test for all the components.

At this moment we don't know the logic behind the TEST_TYPE field and this PR is to force set the TEST_TYPE to default when running from integ-test workflow so that when we pass the specific component the integ tests are run for that only.
This change doesn't alter the current behavior in case the TEST_TYPE is not force set it will follow the current behavior.
Hope it makes sense.
@ashwin-pc

I think its the wording that @ashwin-pc is saying. If you do not pass -o test_finder.sh will anyway have TEST_TYPE = default. As @rishabh6788 said we don't know what the difference in the behavior is for default and manifest but if something needs to change if a manifest is provided which is true in case of distribution build/test pipelines, please guide us where to make the changes.
cc: @tianleh

@rishabh6788 thanks for the context. Yeah as @gaiksaya points out, my comment was about the wording no defaults but based on your comment the -o flag is an optional flag that can take a TEST_TYPE value, but when not present defaults to default. The echo statement here does not say that and makes it seem like the -o flag is a required flag with no defaults. As a caveat, im not sure what the TEST_TYPE flag does either and my comment is strictly about the instructions provided to a user of the script when we echo the help instructions.

As per my testing I see TEST_TYPE had null value when we directly call the integtest script inside a cloned repo, which is later getting initialized at line. But not sure if were are setting the variable TEST_TYPE when the script is called from a different source.

Since TEST_TYPE is set to $OPTION, it has null value when -o is not passed in the script, that's the reason -o has no default value.
That's the reason we didn't alter it's value if TEST_TYPE is already set.

Hi @ashwin-pc, @Divyaasm will be creating the issue for fixing the bug here:https://github.com/opensearch-project/opensearch-dashboards-functional-test/blob/main/test_finder.sh#L57-L75
If manifest is provided there is no option to run integration test for just 1 component today. So we will be using default now. If you think TEST_TYPE manifest should be removed completely that would work as well. I believe that was added for same distribution integration test which is not being used now.

Thanks!

rishabh6788

LGTM!

kavilla

Seems to fine me.

Historically there was no ability to run this repo and specific tests from the Jenkins CI for OSD. So this script was to added to prevent the default run where it scanned the directory of integration tests and just ran them based on if it's either default (so all the existing tests) or checking if the plugin existed.

So I think it's good but it is still open to user error where if they pass in the test type to be default and then pass a test component that doesn't exist in the build then the tests will fail. But that can be an improvement later if it's enough of a problem.

Signed-off-by: Divya Madala <[email protected]> (cherry picked from commit f265231)

gaiksaya · 2023-04-27T20:32:49Z

Here is the bug issue created @ashwin-pc @kavilla #648

Thanks!

Signed-off-by: Divya Madala <[email protected]> (cherry picked from commit f265231) Co-authored-by: Divya Madala <[email protected]>

opensearch-trigger-bot · 2023-05-15T16:48:13Z

The backport to 1.3 failed:

The process '/usr/bin/git' failed with exit code 1

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-1.3 1.3
# Navigate to the new working tree
cd .worktrees/backport-1.3
# Create a new branch
git switch --create backport/backport-644-to-1.3
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 f26523133ce33618b78f1557a07edd00a12c2ac5
# Push it to GitHub
git push --set-upstream origin backport/backport-644-to-1.3
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-1.3

Then, create a pull request where the base branch is 1.3 and the compare/head branch is backport/backport-644-to-1.3.

prudhvigodithi · 2023-05-15T16:51:02Z

@Divyaasm can you please backport this to 1.3 branch to move forward with 1.3.10 release.
opensearch-project/opensearch-build#3331

…pensearch-project#649) Signed-off-by: Divya Madala <[email protected]> (cherry picked from commit f265231) Co-authored-by: Divya Madala <[email protected]> Signed-off-by: [email protected] <[email protected]>

add optional parameter -o to integtest.sh

6adf9c2

Signed-off-by: Divya Madala <[email protected]>

Divyaasm requested review from tianleh, kavilla, ohltyler, CCongWang, ashwin-pc and peterzhuamazon as code owners April 26, 2023 18:36

peterzhuamazon mentioned this pull request Apr 26, 2023

[META] RPM, Deb and Windows integration testing lacks jenkins automation opensearch-project/opensearch-build#2994

Closed

18 tasks

ashwin-pc reviewed Apr 26, 2023

View reviewed changes

rishabh6788 approved these changes Apr 26, 2023

View reviewed changes

gaiksaya mentioned this pull request Apr 26, 2023

Updated integ-tests implementation to run all the OSD plugins. opensearch-project/opensearch-build#3316

Merged

rishabh6788 requested a review from ashwin-pc April 27, 2023 17:56

Divyaasm mentioned this pull request Apr 27, 2023

Integtests are executed for all components even when -t component is passed when TEST_TYPE is checked to manifest. #648

Closed

peterzhuamazon linked an issue Apr 27, 2023 that may be closed by this pull request

Integtests are executed for all components even when -t component is passed when TEST_TYPE is checked to manifest. #648

Closed

kavilla approved these changes Apr 27, 2023

View reviewed changes

kavilla added backport 2.x backport 2.7 labels Apr 27, 2023

gaiksaya merged commit f265231 into opensearch-project:main Apr 27, 2023

opensearch-trigger-bot bot mentioned this pull request Apr 27, 2023

[Backport 2.x] Add an optional parameter -o to integtest.sh #649

Merged

opensearch-trigger-bot bot pushed a commit that referenced this pull request Apr 27, 2023

add optional parameter -o to integtest.sh (#644)

05143de

Signed-off-by: Divya Madala <[email protected]> (cherry picked from commit f265231)

opensearch-trigger-bot bot pushed a commit that referenced this pull request Apr 27, 2023

add optional parameter -o to integtest.sh (#644)

91926a0

Signed-off-by: Divya Madala <[email protected]> (cherry picked from commit f265231)

opensearch-trigger-bot bot mentioned this pull request Apr 27, 2023

[Backport 2.7] Add an optional parameter -o to integtest.sh #650

Merged

kavilla pushed a commit that referenced this pull request May 2, 2023

add optional parameter -o to integtest.sh (#644) (#649)

ecfdbd3

Signed-off-by: Divya Madala <[email protected]> (cherry picked from commit f265231) Co-authored-by: Divya Madala <[email protected]>

kavilla pushed a commit that referenced this pull request May 2, 2023

add optional parameter -o to integtest.sh (#644) (#650)

fa739fd

Signed-off-by: Divya Madala <[email protected]> (cherry picked from commit f265231) Co-authored-by: Divya Madala <[email protected]>

prudhvigodithi mentioned this pull request May 15, 2023

[Release 1.3.10] Onboard test manifests and add remaining components opensearch-project/opensearch-build#3514

Merged

prudhvigodithi added the backport 1.3 label May 15, 2023

Divyaasm mentioned this pull request May 15, 2023

Backport changes from main to 1.3 branch related to OSD Integtests #662

Merged

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add an optional parameter -o to integtest.sh #644

Add an optional parameter -o to integtest.sh #644

Divyaasm commented Apr 26, 2023 •

edited

Loading

ashwin-pc Apr 26, 2023

rishabh6788 Apr 26, 2023 •

edited

Loading

gaiksaya Apr 26, 2023 •

edited

Loading

ashwin-pc Apr 26, 2023

Divyaasm Apr 27, 2023 •

edited

Loading

gaiksaya Apr 27, 2023

rishabh6788 left a comment

kavilla left a comment

gaiksaya commented Apr 27, 2023

opensearch-trigger-bot bot commented May 15, 2023

prudhvigodithi commented May 15, 2023

Add an optional parameter -o to integtest.sh #644

Add an optional parameter -o to integtest.sh #644

Conversation

Divyaasm commented Apr 26, 2023 • edited Loading

Description

Issues Resolved

Check List

ashwin-pc Apr 26, 2023

Choose a reason for hiding this comment

rishabh6788 Apr 26, 2023 • edited Loading

Choose a reason for hiding this comment

gaiksaya Apr 26, 2023 • edited Loading

Choose a reason for hiding this comment

ashwin-pc Apr 26, 2023

Choose a reason for hiding this comment

Divyaasm Apr 27, 2023 • edited Loading

Choose a reason for hiding this comment

gaiksaya Apr 27, 2023

Choose a reason for hiding this comment

rishabh6788 left a comment

Choose a reason for hiding this comment

kavilla left a comment

Choose a reason for hiding this comment

gaiksaya commented Apr 27, 2023

opensearch-trigger-bot bot commented May 15, 2023

prudhvigodithi commented May 15, 2023

Divyaasm commented Apr 26, 2023 •

edited

Loading

rishabh6788 Apr 26, 2023 •

edited

Loading

gaiksaya Apr 26, 2023 •

edited

Loading

Divyaasm Apr 27, 2023 •

edited

Loading