-
Notifications
You must be signed in to change notification settings - Fork 37
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
Separated out tests in a different GHA workflow #84
Closed
Closed
Changes from 1 commit
Commits
Show all changes
10 commits
Select commit
Hold shift + click to select a range
632b450
Separated out tests in a different GHA workflow
owaiskazi19 72c8b98
Handled code coverage for the PR
owaiskazi19 a441fd5
Testing without MacOS
owaiskazi19 4b63fbf
Merge branch 'main' into tests-ci
owaiskazi19 d59127d
Setting up JDK 17
owaiskazi19 0c594dc
Added macOS back
owaiskazi19 adf0352
Fixed codecov
owaiskazi19 ea0028d
Addressed PR comments
owaiskazi19 f9817f5
Merge branch 'main' into tests-ci
owaiskazi19 dad496b
Addressed PR comments
owaiskazi19 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,39 @@ | ||
| name: Test Plugin | ||
|
|
||
| on: | ||
| push: | ||
| branches-ignore: | ||
| - 'whitesource-remediate/**' | ||
| pull_request: | ||
| types: [opened, synchronize, reopened] | ||
|
|
||
| jobs: | ||
| strategy: | ||
| matrix: | ||
| os: | ||
| - ubuntu-latest | ||
| - macOS-latest | ||
| - windows-latest | ||
| java: | ||
| - 17 | ||
| test: | ||
| if: github.repository == 'opensearch-project/opensearch-ai-flow-framework' | ||
| runs-on: ${{ matrix.os }} | ||
| steps: | ||
| - uses: actions/checkout@v4 | ||
| - name: Test | ||
| run: ./gradlew test | ||
| yamlRestTest: | ||
| if: github.repository == 'opensearch-project/opensearch-ai-flow-framework' | ||
| runs-on: ${{ matrix.os }} | ||
| steps: | ||
| - uses: actions/checkout@v4 | ||
| - name: Test | ||
| run: ./gradlew yamlRestTest | ||
| integTest: | ||
| if: github.repository == 'opensearch-project/opensearch-ai-flow-framework' | ||
| runs-on: ${{ matrix.os }} | ||
| steps: | ||
| - uses: actions/checkout@v4 | ||
| - name: Test | ||
| run: ./gradlew integTest |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just wondering, why not use gradlew build here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can. I don't have any strong opinion here. We had check before I kept the same.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am good with either,
./gradlew buildmeans we also run assemble which runs the:compileJava, :processResourceswhichcheckdoes also but this does it into a jar.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed it to build.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should step back and figure out all the dependencies and figure out what we are trying to optimize here. With the current PR we're shifting the tests somewhere else (still in series so not really gaining much) but duplicating compile/build stuff in parallel.
My general preference would be for CI to keep all the check stuff and
testand code coverage bits, and just spin off the integ tests and yaml tests into another job in parallel (or just put them in the CI yaml at the top level under "jobs").Lots of different ways to do it, haven't really been convinced we have a problem that needs solving yet :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with you. I will go ahead and close the PR. We can always come back and open it back when the plugin becomes heavy.