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 test case for OptionalDependenciesPlugin (com.netflix.nebula:gradle-extra-configurations-plugin plugin migration) #7995

Merged
merged 1 commit into from
Jun 9, 2023

Conversation

reta
Copy link
Collaborator

@reta reta commented Jun 9, 2023

Description

Add test case for OptionalDependenciesPlugin (com.netflix.nebula:gradle-extra-configurations-plugin plugin migration)

Related Issues

N/A

Check List

  • New functionality includes testing.
    • All tests pass
  • New functionality has been documented.
    • New functionality has javadoc added
  • Commits are signed per the DCO using --signoff
  • Commit changes are listed out in CHANGELOG.md file (See: Changelog)

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.

…le-extra-configurations-plugin plugin migration)

Signed-off-by: Andriy Redko <[email protected]>
@reta reta added the backport 2.x Backport to 2.x branch label Jun 9, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Jun 9, 2023

Gradle Check (Jenkins) Run Completed with:

  • RESULT: UNSTABLE ❕
  • TEST FAILURES:
      1 org.opensearch.search.backpressure.SearchBackpressureIT.testSearchTaskCancellationWithHighCpu
      1 org.opensearch.action.admin.cluster.node.tasks.ResourceAwareTasksTests.testTaskResourceTrackingDuringTaskCancellation

@codecov
Copy link

codecov bot commented Jun 9, 2023

Codecov Report

Merging #7995 (64b9344) into main (28d5948) will decrease coverage by 0.52%.
The diff coverage is n/a.

@@             Coverage Diff              @@
##               main    #7995      +/-   ##
============================================
- Coverage     71.44%   70.92%   -0.52%     
+ Complexity    56913    56562     -351     
============================================
  Files          4715     4715              
  Lines        267246   267246              
  Branches      39186    39186              
============================================
- Hits         190930   189549    -1381     
- Misses        60492    61678    +1186     
- Partials      15824    16019     +195     

see 430 files with indirect coverage changes

@github-actions
Copy link
Contributor

github-actions bot commented Jun 9, 2023

Gradle Check (Jenkins) Run Completed with:

  • RESULT: UNSTABLE ❕
  • TEST FAILURES:
      1 org.opensearch.remotestore.SegmentReplicationUsingRemoteStoreIT.testDropPrimaryDuringReplication
      1 org.opensearch.remotestore.RemoteStoreRefreshListenerIT.testRemoteRefreshRetryOnFailure

@kotwanikunal
Copy link
Member

Thanks @reta!

@reta reta merged commit 5ebfbcd into opensearch-project:main Jun 9, 2023
opensearch-trigger-bot bot pushed a commit that referenced this pull request Jun 9, 2023
…le-extra-configurations-plugin plugin migration) (#7995)

Signed-off-by: Andriy Redko <[email protected]>
(cherry picked from commit 5ebfbcd)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
reta pushed a commit that referenced this pull request Jun 9, 2023
…le-extra-configurations-plugin plugin migration) (#7995) (#7996)

(cherry picked from commit 5ebfbcd)

Signed-off-by: Andriy Redko <[email protected]>
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
@gaiksaya
Copy link
Member

gaiksaya commented Jun 15, 2023

Hi @reta ,

Just curious, Any particular reason why we added the test cases for
buildSrc/src/main/groovy/org/opensearch/gradle/plugin/OptionalDependenciesPlugin.groovy under
buildSrc/src/test/java/org/opensearch/gradle/plugin/OptionalDependenciesPluginTests.java?
Gradle has some examples here https://docs.gradle.org/current/userguide/test_kit.html that uses spockframework (bot fancy and easy to use). Wondering if can explore that in future?

Also for my understanding does the codecoverage tool recognize what we are testing? (from this comment looks like it showed codecov as decreased)
Thanks!

@reta
Copy link
Collaborator Author

reta commented Jun 15, 2023

Thanks @gaiksaya !

Just curious, Any particular reason why we added the test cases for
buildSrc/src/main/groovy/org/opensearch/gradle/plugin/OptionalDependenciesPlugin.groovy under
buildSrc/src/test/java/org/opensearch/gradle/plugin/OptionalDependenciesPluginTests.java?

Yes, so the plugin is written in Groovy (OptionalDependenciesPlugin.groovy) so it is under src/main/groovy by convention, the OptionalDependenciesPluginTests is written in Java, so it is under src/test/java by convention, does it help or you were asking about different thing?

Gradle has some examples here https://docs.gradle.org/current/userguide/test_kit.html that uses spockframework (bot fancy and easy to use). Wondering if can explore that in future?

I think in general there is no constraints on test framework to use, as far as it brings tangible benefits. Our tests for this module are currently written in Java, as all other tests for OpenSearch core.

Also for my understanding does the codecoverage tool recognize what we are testing? (from #7995 (comment) comment looks like it showed codecov as decreased)

That is a good question, it believe it does not recognize what we are testing in this particular case.

@gaiksaya
Copy link
Member

Thanks!

does it help or you were asking about different thing?

It helps. Was just curious why we didn't add groovy tests under buildSrc/src/test/groovy for those components. Looks like anything works and there are no constraints?

@reta
Copy link
Collaborator Author

reta commented Jun 15, 2023

It helps. Was just curious why we didn't add groovy tests under buildSrc/src/test/groovy for those components. Looks like anything works and there are no constraints?

What are the benefits? And what is the cost to maintain the new framework? Who in the community is familiar with it? Groovy is on severe decline, the only reason I've added the Groovy based plugin is to preserve the original license and source code.

@gaiksaya
Copy link
Member

@reta
Copy link
Collaborator Author

reta commented Jun 15, 2023

Indeed, thanks! Wanna give it a try by refactoring the OptionalDependenciesPluginTests to use Spock? No objections from my side, if you are curious to do that :)

@prudhvigodithi
Copy link
Member

Here is the gradle functional unit tests example from opensearch.pluginzip plugin.

Also for CodeCov coverage the buildSrc folder is included, assume its coming from code-coverage.gradle, @reta am I right here?
@bbarani @peterzhuamazon

@gaiksaya
Copy link
Member

Indeed, thanks! Wanna give it a try by refactoring the OptionalDependenciesPluginTests to use Spock? No objections from my side, if you are curious to do that :)

Will add to my queue for sure. Thanks!

@reta
Copy link
Collaborator Author

reta commented Jun 16, 2023

Also for CodeCov coverage the buildSrc folder is included, assume its coming from code-coverage.gradle, @reta am I right here?

The pluginzip/Publish is Java, not Groovy, right?

gaiksaya pushed a commit to gaiksaya/OpenSearch that referenced this pull request Jun 26, 2023
…le-extra-configurations-plugin plugin migration) (opensearch-project#7995) (opensearch-project#7996)

(cherry picked from commit 5ebfbcd)

Signed-off-by: Andriy Redko <[email protected]>
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
imRishN pushed a commit to imRishN/OpenSearch that referenced this pull request Jun 27, 2023
…le-extra-configurations-plugin plugin migration) (opensearch-project#7995)

Signed-off-by: Andriy Redko <[email protected]>
Signed-off-by: Rishab Nahata <[email protected]>
shiv0408 pushed a commit to Gaurav614/OpenSearch that referenced this pull request Apr 25, 2024
…le-extra-configurations-plugin plugin migration) (opensearch-project#7995)

Signed-off-by: Andriy Redko <[email protected]>
Signed-off-by: Shivansh Arora <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 2.x Backport to 2.x branch skip-changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants