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

Add End-to-End tests to CI job #1043

Merged
merged 2 commits into from
Feb 14, 2025
Merged

Conversation

normanj-bitquill
Copy link
Contributor

Description

Adds the End-to-End tests to the CI job GitHub action. They will be run with the integration tests. The end-to-end test results are included in the uploaded artifacts.

Related Issues

Related to #647

Check List

  • Updated documentation (docs/ppl-lang/README.md)
  • Implemented unit tests
  • Implemented tests for combination with other commands
  • New added source code should include a copyright header
  • 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: Norman Jordan <[email protected]>
}
}
}.start()
val completed = dockerProcess.waitFor(30, TimeUnit.MINUTES)
Copy link
Member

Choose a reason for hiding this comment

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

@normanj-bitquill why is the timeout so long ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tested this in a different PR. It took about 13 minutes to startup the cluster. This includes downloading Docker images that are needed. 30 minutes is just to give a safe buffer around that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@YANG-DB This can be decreased but it we will want it to be 13 minutes plus a buffer. Could use 20 minutes instead.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@normanj-bitquill is it possible to have a seperate workflow for e2e test? my concern is add e2e test will add 30mins for every PR.

Copy link
Member

@YANG-DB YANG-DB left a comment

Choose a reason for hiding this comment

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

can/should we set timeout wait period shorter ?

@YANG-DB
Copy link
Member

YANG-DB commented Feb 12, 2025

@LantaoJin @penghuo
Any comments?

@YANG-DB YANG-DB merged commit 3b4866a into opensearch-project:main Feb 14, 2025
4 checks passed
@penghuo
Copy link
Collaborator

penghuo commented Feb 14, 2025

CI broken after this merge this PR. https://github.com/opensearch-project/opensearch-spark/commits/main/

@YANG-DB @normanj-bitquill should we revert this PR?

@YANG-DB
Copy link
Member

YANG-DB commented Feb 14, 2025

@normanj-bitquill actually did find a real regression so IMO the feature is good to have...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants