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

[Star tree] Refactoring builder tests #16036

Merged

Conversation

bharath-techie
Copy link
Contributor

Description

The AbstractBuilderTests has grown to be around 4500 lines long, and its very difficult to write new tests for future builder changes in the same file. So this PR just splits the tests into different categories such as flush, merge , sort and aggregate tests and so on.

Ideally, we also need to remove the hardcoded configurations in StarTreeBuilderTestCase. Will further check how to optimize that.

Related Issues

Resolves #[Issue number to be closed when this PR is merged]

Check List

  • Functionality includes testing.
  • API changes companion pull request created, if applicable.
  • Public documentation issue/PR created, if applicable.

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.

Copy link
Contributor

✅ Gradle check result for 77112f4: SUCCESS

Copy link

codecov bot commented Sep 23, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 71.88%. Comparing base (a81b868) to head (02f2b83).
Report is 47 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff              @@
##               main   #16036      +/-   ##
============================================
- Coverage     71.94%   71.88%   -0.07%     
- Complexity    64612    64681      +69     
============================================
  Files          5298     5303       +5     
  Lines        301952   302476     +524     
  Branches      43627    43699      +72     
============================================
+ Hits         217247   217430     +183     
- Misses        66884    67185     +301     
- Partials      17821    17861      +40     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@sarthakaggarwal97 sarthakaggarwal97 left a comment

Choose a reason for hiding this comment

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

Thanks @bharath-techie for taking this refactor. Definitely helps with segregation of tests.
Could not verify all the tests, spot checked few. Approving since we have not changed any internal workings of all the tests.

Copy link
Contributor

github-actions bot commented Oct 7, 2024

❌ Gradle check result for 7c001ee: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

Signed-off-by: Bharathwaj G <bharath78910@gmail.com>
Copy link
Contributor

github-actions bot commented Oct 7, 2024

❌ Gradle check result for 11c3838: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

Signed-off-by: Bharathwaj G <bharath78910@gmail.com>
Copy link
Contributor

github-actions bot commented Oct 8, 2024

✅ Gradle check result for 02f2b83: SUCCESS

@sachinpkale sachinpkale merged commit 3696c29 into opensearch-project:main Oct 8, 2024
34 checks passed
@bharath-techie bharath-techie self-assigned this Oct 8, 2024
@bharath-techie bharath-techie added the backport 2.x Backport to 2.x branch label Oct 15, 2024
opensearch-trigger-bot bot pushed a commit that referenced this pull request Oct 15, 2024
* Refactoring builder tests

Signed-off-by: Bharathwaj G <bharath78910@gmail.com>

* adding date tests

Signed-off-by: Bharathwaj G <bharath78910@gmail.com>

---------

Signed-off-by: Bharathwaj G <bharath78910@gmail.com>
(cherry picked from commit 3696c29)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
gbbafna pushed a commit that referenced this pull request Oct 16, 2024
(cherry picked from commit 3696c29)

Signed-off-by: Bharathwaj G <bharath78910@gmail.com>
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>
dk2k pushed a commit to dk2k/OpenSearch that referenced this pull request Oct 16, 2024
* Refactoring builder tests

Signed-off-by: Bharathwaj G <bharath78910@gmail.com>

* adding date tests

Signed-off-by: Bharathwaj G <bharath78910@gmail.com>

---------

Signed-off-by: Bharathwaj G <bharath78910@gmail.com>
dk2k pushed a commit to dk2k/OpenSearch that referenced this pull request Oct 17, 2024
* Refactoring builder tests

Signed-off-by: Bharathwaj G <bharath78910@gmail.com>

* adding date tests

Signed-off-by: Bharathwaj G <bharath78910@gmail.com>

---------

Signed-off-by: Bharathwaj G <bharath78910@gmail.com>
dk2k pushed a commit to dk2k/OpenSearch that referenced this pull request Oct 21, 2024
* Refactoring builder tests

Signed-off-by: Bharathwaj G <bharath78910@gmail.com>

* adding date tests

Signed-off-by: Bharathwaj G <bharath78910@gmail.com>

---------

Signed-off-by: Bharathwaj G <bharath78910@gmail.com>
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.

3 participants