Skip to content

Added missing integration tests for the pow and power functions#247

Merged
matthewryanwells merged 8 commits intointeg-pow-itfrom
dev-pow-it
Mar 21, 2023
Merged

Added missing integration tests for the pow and power functions#247
matthewryanwells merged 8 commits intointeg-pow-itfrom
dev-pow-it

Conversation

@matthewryanwells
Copy link
Copy Markdown

@matthewryanwells matthewryanwells commented Mar 16, 2023

Description

Add integration tests for the pow and power functions which were previously missing it

Issues Resolved

opensearch-project#1190

Check List

  • All tests pass, including unit test, integration test and doctest
  • 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.

@matthewryanwells
added missing integration tests for the pow and power functions
16661a0 
Signed-off-by: Matthew Wells <matthew.wells@improving.com>
@GumpacG
Copy link
Copy Markdown

GumpacG commented Mar 16, 2023

Can you also add a test where the result would be null

@codecov
Copy link
Copy Markdown

codecov bot commented Mar 16, 2023 •
edited
Loading

Codecov Report

❗ No coverage uploaded for pull request base (integ-pow-it@4f9f5ca). Click here to learn what that means.
The diff coverage is n/a.

❗ Current head f97e964 differs from pull request most recent head 6217a96. Consider uploading reports for the commit 6217a96 to get more accurate results

@@               Coverage Diff               @@
##             integ-pow-it     #247   +/-   ##
===============================================
  Coverage                ?   98.39%           
  Complexity              ?     3717           
===============================================
  Files                   ?      343           
  Lines                   ?     9159           
  Branches                ?      584           
===============================================
  Hits                    ?     9012           
  Misses                  ?      142           
  Partials                ?        5
Flag Coverage Δ
sql-engine 98.39% <0.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@matthewryanwells
Copy link
Copy Markdown
Author

matthewryanwells commented Mar 16, 2023

Can you also add a test where the result would be null

Is there a mathematical equation that would cause pow/power to output a null or do you want me to force a null value by giving it invalid data?

@GumpacG
Copy link
Copy Markdown

GumpacG commented Mar 16, 2023

Can you also add a test where the result would be null

Is there a mathematical equation that would cause pow/power to output a null or do you want me to force a null value by giving it invalid data?

Some examples here https://stackoverflow.com/questions/20250453/why-math-powa-b-is-nan-when-a-is-a-negative-number-and-b-is-a-non-integer

@matthewryanwells
Copy link
Copy Markdown
Author

matthewryanwells commented Mar 17, 2023

Can you also add a test where the result would be null

Is there a mathematical equation that would cause pow/power to output a null or do you want me to force a null value by giving it invalid data?

Some examples here https://stackoverflow.com/questions/20250453/why-math-powa-b-is-nan-when-a-is-a-negative-number-and-b-is-a-non-integer

From what I have seen with the other tests IT don't actually ever test for null or invalid values, those are testing in the unit tests. Also if you try to input data that would cause an invalid outcome the tests returns an IllegalArgumentException

@matthewryanwells
Updated pow/power function to return null in case of invalid input, u…
883bb13 
…pdated IT to test for invalid output

Signed-off-by: Matthew Wells <matthew.wells@improving.com>
@matthewryanwells
Removed accidentally added file
1c14417 
Signed-off-by: Matthew Wells <matthew.wells@improving.com>
@matthewryanwells
Fixed checkstyle failure
a6058fd 
Signed-off-by: Matthew Wells <matthew.wells@improving.com>
@matthewryanwells
Add tests to improve test coverage
9cdc72b 
Signed-off-by: Matthew Wells <matthew.wells@improving.com>
GumpacG
GumpacG approved these changes Mar 18, 2023
View reviewed changes
Copy link
Copy Markdown

@GumpacG GumpacG left a comment

Choose a reason for hiding this comment

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

LGTM! Although, some GH actions are failing.

@matthewryanwells
Fixed failing Jacoco test coverage
abe65a0 
Signed-off-by: Matthew Wells <matthew.wells@improving.com>
@matthewryanwells
seperated pow tests where results should be null from edge cases
f97e964 
Signed-off-by: Matthew Wells <matthew.wells@improving.com>
@matthewryanwells
added missing period on javadoc
6217a96 
Signed-off-by: Matthew Wells <matthew.wells@improving.com>
@matthewryanwells matthewryanwells merged commit 54aa6e5 into integ-pow-it Mar 21, 2023
@matthewryanwells matthewryanwells deleted the dev-pow-it branch March 21, 2023 18:21
forestmvey pushed a commit that referenced this pull request Mar 22, 2023
@matthewryanwells
Added missing integration tests for the pow and power functions (#247) (
a35f963 
opensearch-project#1457)

* Changed pow and power to return null when an invalid response would be returned and added missing integration tests for the pow and power function.

Signed-off-by: Matthew Wells <matthew.wells@improving.com>
MaxKsyunz pushed a commit that referenced this pull request Mar 29, 2023
@opensearch-trigger-bot @matthewryanwells
Added missing integration tests for the pow and power functions (#247) (
49a82ae 
opensearch-project#1457) (opensearch-project#1459)

* Changed pow and power to return null when an invalid response would be returned and added missing integration tests for the pow and power function.

Signed-off-by: Matthew Wells <matthew.wells@improving.com>
(cherry picked from commit a35f963)

Co-authored-by: Matthew Wells <matthew.wells@improving.com>
andy-k-improving pushed a commit that referenced this pull request Nov 16, 2024
@naveentatikonda
Replace Map, List, and Set in org.opensearch.common.collect with java…
e594bf3 
….util references (#247)

Signed-off-by: Naveen Tatikonda <navtat@amazon.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers

@GumpacG GumpacG GumpacG approved these changes

@Yury-Fridlyand Yury-Fridlyand Yury-Fridlyand approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@matthewryanwells @GumpacG @Yury-Fridlyand