Add JDK 8 to CI Matrix #483

davidcui1225 · 2022-03-11T00:44:48Z

Description

Add JDK 8 to CI matrix
Replace Map.of() method since it has no support in JDK 8
Replace String.stripTrailing() and stripLeading() with regex expressions
Use different number for Degree conversion to Radian due to JDK difference: https://stackoverflow.com/questions/64586096/math-pow-in-java-8-and-java-13-returns-different-result
Filter out DateTime unit test on Java 8 that fails due to JDK bug https://bugs.openjdk.java.net/browse/JDK-8114833
Skip DateTime IT tests that fail due to JDK bug

Issues Resolved

[List any issues this PR will resolve]

Check List

New functionality includes testing.
- All tests pass, including unit test, integration test and doctest
New functionality has been documented.
- New functionality has javadoc added
- New functionality has user manual doc added
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.

Signed-off-by: David Cui <[email protected]>

codecov-commenter · 2022-03-11T00:55:30Z

Codecov Report

Merging #483 (4b664a9) into main (646ce3c) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##               main     #483   +/-   ##
=========================================
  Coverage     95.25%   95.25%           
  Complexity     2746     2746           
=========================================
  Files           276      276           
  Lines          7414     7416    +2     
  Branches        538      545    +7     
=========================================
+ Hits           7062     7064    +2     
  Misses          298      298           
  Partials         54       54

Flag	Coverage Δ
query-workbench	`62.91% <ø> (ø)`
sql-engine	`98.40% <100.00%> (+<0.01%)`	⬆️

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

Impacted Files	Coverage Δ
...c/main/java/org/opensearch/sql/expression/DSL.java	`100.00% <ø> (ø)`
...g/opensearch/sql/expression/text/TextFunction.java	`100.00% <100.00%> (ø)`
.../opensearch/sql/planner/physical/SortOperator.java	`100.00% <0.00%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 646ce3c...4b664a9. Read the comment docs.

core/src/test/java/org/opensearch/sql/expression/datetime/DateTimeFunctionTest.java

Signed-off-by: David Cui <[email protected]>

penghuo · 2022-03-11T01:16:30Z

core/build.gradle

- 'org.opensearch.sql.utils.MLCommonsConstants'
+ 'org.opensearch.sql.utils.MLCommonsConstants',
+ 'org.opensearch.sql.expression.datetime.DateTimeFormatterUtil',
+ 'org.opensearch.sql.expression.DSL',


why disable DSL? I didn't see code change.

DSL was also failing jacoco test coverage with lines covered ratio is 0.9, but expected minimum is 1.0. Wasn't sure how to resolve so I temp added it to get a working build

This was after the date_format test case was disabled

Signed-off-by: David Cui <[email protected]>

joshuali925

might need a tracking issue to fix disabled tests

joshuali925 · 2022-03-11T02:47:29Z

docs/user/dql/functions.rst

+ | DEGREES(0) |
+ |--------------|
+ | 0.0  |
+ +--------------+


could you describe a bit what the issue is in PR description?

davidcui1225 · 2022-03-11T03:00:03Z

might need a tracking issue to fix disabled tests

I'll cut one, good call

* fixed references in java 8 and added to ci matrix * filter test that fails due to jdk 8 bug * use different degree to radian conversion for difference in jdk * skip date time format tests that cause error due to jdk bug * only disable DateTime unit test on Java 8 Signed-off-by: David Cui <[email protected]> (cherry picked from commit b6fccbd)

* fixed references in java 8 and added to ci matrix * filter test that fails due to jdk 8 bug * use different degree to radian conversion for difference in jdk * skip date time format tests that cause error due to jdk bug * only disable DateTime unit test on Java 8 Bypassing failed DCO check Signed-off-by: David Cui <[email protected]> (cherry picked from commit b6fccbd) Co-authored-by: David Cui <[email protected]>

* fixed references in java 8 and added to ci matrix * filter test that fails due to jdk 8 bug * use different degree to radian conversion for difference in jdk * skip date time format tests that cause error due to jdk bug * only disable DateTime unit test on Java 8 Signed-off-by: David Cui <[email protected]> (cherry picked from commit b6fccbd)

* Disable flaky test in JdbcTestIT. (#475) Signed-off-by: penghuo <[email protected]> * Update dependency opensearch-ml-client group name (#477) Signed-off-by: jackieyanghan <[email protected]> * Collected fixes related to bugs #280, #302, #783, #696 and 317 all together. (#479) Signed-off-by: Yury Fridlyand <[email protected]> * Add JDK 8 to CI Matrix (#483) (#484) * fixed references in java 8 and added to ci matrix * filter test that fails due to jdk 8 bug * use different degree to radian conversion for difference in jdk * skip date time format tests that cause error due to jdk bug * only disable DateTime unit test on Java 8 Bypassing failed DCO check Signed-off-by: David Cui <[email protected]> (cherry picked from commit b6fccbd) Co-authored-by: David Cui <[email protected]> * Add parse docs to PPL commands index (#486) (#487) Signed-off-by: Joshua Li <[email protected]> * Add 1.3.0 release notes (#492) Signed-off-by: Eugene Lee <[email protected]> * Add Kmeans and AD command documentation (#493) (#497) Signed-off-by: jackieyanghan <[email protected]> (cherry picked from commit ee4bce0) * Bump version to 1.3.1 Signed-off-by: Sayali Gaikawad <[email protected]> * Bump ml Signed-off-by: Sayali Gaikawad <[email protected]> * Add new zip Signed-off-by: Sayali Gaikawad <[email protected]> * Updated release drafter Signed-off-by: Sayali Gaikawad <[email protected]> * Remove checked-in ml-commons dependency (#529) Signed-off-by: jackieyanghan <[email protected]> * Bump workbench to 1.3.1 (#540) Signed-off-by: Joshua Li <[email protected]> * Bug Fix, return default ID when log4j ThreadContext is empty (#538) (#539) (#543) Signed-off-by: penghuo <[email protected]> (cherry picked from commit 54b7257) * Incremented version to 1.3.2. (#593) Signed-off-by: Zelin Hao <[email protected]> Signed-off-by: Joshua Li <[email protected]> Co-authored-by: Joshua Li <[email protected]> * Version bump 1.3.3 Signed-off-by: pgodithi <[email protected]> * [1.3] Bump spring-beans to 5.2.20 (#632) Signed-off-by: Joshua Li <[email protected]> * Version bump 1.3.3 Signed-off-by: pgodithi <[email protected]> * Incremented version to 1.3.4. (#681) * Incremented version to 1.3.4. Signed-off-by: Zelin Hao <[email protected]> * bump jackson to 2.13.2 Signed-off-by: Joshua Li <[email protected]> Co-authored-by: Joshua Li <[email protected]> Co-authored-by: Peng Huo <[email protected]> Co-authored-by: Jackie Han <[email protected]> Co-authored-by: Yury-Fridlyand <[email protected]> Co-authored-by: opensearch-trigger-bot[bot] <98922864+opensearch-trigger-bot[bot]@users.noreply.github.com> Co-authored-by: David Cui <[email protected]> Co-authored-by: Eugene Lee <[email protected]> Co-authored-by: Sayali Gaikawad <[email protected]> Co-authored-by: Sayali Gaikawad <[email protected]> Co-authored-by: Joshua Li <[email protected]> Co-authored-by: Zelin Hao <[email protected]> Co-authored-by: pgodithi <[email protected]> Co-authored-by: Anirudha (Ani) Jadhav <[email protected]>

davidcui1225 added 15 commits May 5, 2021 18:06

fixed no-error-output bug in workbench (opensearch-project#32)

69a658e

Signed-off-by: David Cui <[email protected]>

Merge remote-tracking branch 'origin/main' into main

2a61342

Merge remote-tracking branch 'origin/main' into main

96f8e98

Merge remote-tracking branch 'origin/main' into main

67e5a4d

Merge remote-tracking branch 'origin/main' into main

a56fdaf

Merge remote-tracking branch 'origin/main' into main

73c0c1d

Merge remote-tracking branch 'origin/main' into main

7551985

Merge remote-tracking branch 'origin/main' into main

7a9e626

Merge remote-tracking branch 'origin/main' into main

bcdf019

Merge remote-tracking branch 'origin/main' into main

f817737

Merge remote-tracking branch 'origin/main' into main

413f8d4

Merge remote-tracking branch 'origin/main' into main

9dc17d9

Merge remote-tracking branch 'origin/main' into main

b96a9ee

fixed references in java 8 and added to ci matrix

73658d7

Signed-off-by: David Cui <[email protected]>

filter test that fails due to jdk 8 bug

d6a6475

Signed-off-by: David Cui <[email protected]>

davidcui1225 requested a review from a team as a code owner March 11, 2022 00:44

penghuo reviewed Mar 11, 2022

View reviewed changes

core/src/test/java/org/opensearch/sql/expression/datetime/DateTimeFunctionTest.java Outdated Show resolved Hide resolved

use different degree to radian conversion for difference in jdk

774c7ab

Signed-off-by: David Cui <[email protected]>

penghuo reviewed Mar 11, 2022

View reviewed changes

davidcui1225 added 2 commits March 10, 2022 17:41

skip date time format tests that cause error due to jdk bug

8b2957d

Signed-off-by: David Cui <[email protected]>

only disable DateTime unit test on Java 8

4b664a9

Signed-off-by: David Cui <[email protected]>

davidcui1225 requested review from anirudha and joshuali925 March 11, 2022 02:46

joshuali925 approved these changes Mar 11, 2022

View reviewed changes

anirudha approved these changes Mar 11, 2022

View reviewed changes

davidcui1225 merged commit b6fccbd into opensearch-project:main Mar 11, 2022

davidcui1225 deleted the add-jdk-8 branch March 11, 2022 03:14

davidcui1225 added the backport 1.3 label Mar 11, 2022

opensearch-trigger-bot bot mentioned this pull request Mar 11, 2022

[Backport 1.3] Add JDK 8 to CI Matrix #484

Merged

penghuo added the backport 1.x label Mar 15, 2022

opensearch-trigger-bot bot mentioned this pull request Mar 15, 2022

[Backport 1.x] Add JDK 8 to CI Matrix #499

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add JDK 8 to CI Matrix #483

Add JDK 8 to CI Matrix #483

davidcui1225 commented Mar 11, 2022 •

edited

Loading

codecov-commenter commented Mar 11, 2022 •

edited

Loading

penghuo Mar 11, 2022

davidcui1225 Mar 11, 2022

davidcui1225 Mar 11, 2022

joshuali925 left a comment

joshuali925 Mar 11, 2022

davidcui1225 Mar 11, 2022

davidcui1225 commented Mar 11, 2022

Add JDK 8 to CI Matrix #483

Add JDK 8 to CI Matrix #483

Conversation

davidcui1225 commented Mar 11, 2022 • edited Loading

Description

Issues Resolved

Check List

codecov-commenter commented Mar 11, 2022 • edited Loading

Codecov Report

penghuo Mar 11, 2022

Choose a reason for hiding this comment

davidcui1225 Mar 11, 2022

Choose a reason for hiding this comment

davidcui1225 Mar 11, 2022

Choose a reason for hiding this comment

joshuali925 left a comment

Choose a reason for hiding this comment

joshuali925 Mar 11, 2022

Choose a reason for hiding this comment

davidcui1225 Mar 11, 2022

Choose a reason for hiding this comment

davidcui1225 commented Mar 11, 2022

davidcui1225 commented Mar 11, 2022 •

edited

Loading

codecov-commenter commented Mar 11, 2022 •

edited

Loading