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

Changed the test for RegexValidator class #608

Merged
merged 13 commits into from
Apr 7, 2023

Conversation

Kuanysh-kst
Copy link
Contributor

Description

In this PR, I am refactoring the test so that it passes, this PR is related to this PR: opensearch-project/OpenSearch#6823

Issues Resolved

#591

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.

@dbwiddis dbwiddis added the CCI Part of the College Contributor Initiative label Mar 25, 2023
Copy link
Member

@dbwiddis dbwiddis left a comment

Choose a reason for hiding this comment

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

Let's make this even simpler!

@Kuanysh-kst Kuanysh-kst requested a review from dbwiddis April 3, 2023 09:52
dbwiddis
dbwiddis previously approved these changes Apr 5, 2023
ryanbogan
ryanbogan previously approved these changes Apr 6, 2023
@joshpalis
Copy link
Member

@Kuanysh-kst Looks like gradle check is failing. Please rebase your PR with SDK main

@dbwiddis
Copy link
Member

dbwiddis commented Apr 7, 2023

@Kuanysh-kst Looks like gradle check is failing. Please rebase your PR with SDK main

The failure is the signature of the class just merged in companion PR. May need to wait for snapshot publishing.

@owaiskazi19
Copy link
Member

owaiskazi19 commented Apr 7, 2023

The failure is the signature of the class just merged in companion PR. May need to wait for snapshot publishing.

Snapshot has been published: https://github.com/opensearch-project/OpenSearch/actions/runs/4639883181. Updated the main branch

@owaiskazi19
Copy link
Member

Build is failing because of:

> Task :compileTestJava
/home/runner/work/opensearch-sdk-java/opensearch-sdk-java/src/test/java/org/opensearch/sdk/TestExtensionsRunner.java:145: error: no suitable constructor found for ExtensionRestRequest(Method,String,Map<Object,Object>,<null>,BytesArray,String)
        ExtensionRestRequest request = new ExtensionRestRequest(
                                       ^
    constructor ExtensionRestRequest.ExtensionRestRequest(Method,String,String,Map<String,String>,Map<String,List<String>>,XContentType,BytesReference,String,HttpVersion) is not applicable
      (actual and formal argument lists differ in length)
    constructor ExtensionRestRequest.ExtensionRestRequest(StreamInput) is not applicable
      (actual and formal argument lists differ in length)
1 error

> Task :compileTestJava FAILED

@dbwiddis
Copy link
Member

dbwiddis commented Apr 7, 2023

Build is failing because of ...

I got confused on which PR this comment was on.

That method signature is the just-merged OS 6826, companion PR of #643. We need to merge #643 and then rebase this PR and it should pass.

Signed-off-by: Kuanysh <[email protected]>
Signed-off-by: Kuanysh <[email protected]>
Signed-off-by: Kuanysh <[email protected]>
Signed-off-by: Kuanysh <[email protected]>
Signed-off-by: Kuanysh <[email protected]>
Signed-off-by: Kuanysh <[email protected]>
@dbwiddis dbwiddis dismissed stale reviews from ryanbogan and themself via 54272a7 April 7, 2023 19:47
@codecov-commenter
Copy link

codecov-commenter commented Apr 7, 2023

Codecov Report

Merging #608 (cd8367a) into main (ec6a8e6) will increase coverage by 0.08%.
The diff coverage is n/a.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@             Coverage Diff              @@
##               main     #608      +/-   ##
============================================
+ Coverage     64.08%   64.17%   +0.08%     
- Complexity      268      269       +1     
============================================
  Files            54       54              
  Lines          1150     1150              
  Branches         35       35              
============================================
+ Hits            737      738       +1     
+ Misses          402      401       -1     
  Partials         11       11              
Impacted Files Coverage Δ
.../sample/helloworld/ExampleCustomSettingConfig.java 50.00% <ø> (+50.00%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@ryanbogan ryanbogan merged commit a7a8164 into opensearch-project:main Apr 7, 2023
opensearch-trigger-bot bot pushed a commit that referenced this pull request Apr 7, 2023
* Created the MapperExtension interface and added the MapperPlugin content

Signed-off-by: Kuanysh <[email protected]>

* deleted comments

Signed-off-by: Kuanysh <[email protected]>

* add javadoc

Signed-off-by: Kuanysh <[email protected]>

* add unit test

Signed-off-by: Kuanysh <[email protected]>

* refactor for unit test

Signed-off-by: Kuanysh <[email protected]>

* fix bugs

Signed-off-by: Kuanysh <[email protected]>

* code refactoring

Signed-off-by: Kuanysh <[email protected]>

* change test

Signed-off-by: Kuanysh <[email protected]>

* code refactoring

Signed-off-by: Kuanysh <[email protected]>

* shortened the code

Signed-off-by: Kuanysh <[email protected]>

* add false parametr

Signed-off-by: Kuanysh <[email protected]>

* code rafactoring

Signed-off-by: Kuanysh <[email protected]>

* Remove stray file from old commit that survived rebase

Signed-off-by: Daniel Widdis <[email protected]>

---------

Signed-off-by: Kuanysh <[email protected]>
Signed-off-by: Daniel Widdis <[email protected]>
Co-authored-by: Daniel Widdis <[email protected]>
(cherry picked from commit a7a8164)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
dbwiddis added a commit that referenced this pull request Apr 11, 2023
* Created the MapperExtension interface and added the MapperPlugin content



* deleted comments



* add javadoc



* add unit test



* refactor for unit test



* fix bugs



* code refactoring



* change test



* code refactoring



* shortened the code



* add false parametr



* code rafactoring



* Remove stray file from old commit that survived rebase



---------




(cherry picked from commit a7a8164)

Signed-off-by: Kuanysh <[email protected]>
Signed-off-by: Daniel Widdis <[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>
Co-authored-by: Daniel Widdis <[email protected]>
Co-authored-by: Owais Kazi <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 1.x CCI Part of the College Contributor Initiative repeat-contributor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants