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

[Spotless] Applying Google Code Format for integ-tests #8 #327

Merged
merged 1 commit into from
Aug 14, 2023

Conversation

MitchellGale
Copy link

Description

Spotless apply on integ-test (part 1 of 4 integ-test integration).

Issues Resolved

opensearch-project#1101

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.

@codecov
Copy link

codecov bot commented Aug 4, 2023

Codecov Report

Merging #327 (0521d7c) into integ/sl_GoogleJavaFormat8 (245c4f8) will not change coverage.
The diff coverage is 100.00%.

❗ Current head 0521d7c differs from pull request most recent head 3e16d96. Consider uploading reports for the commit 3e16d96 to get more accurate results

@@                      Coverage Diff                      @@
##             integ/sl_GoogleJavaFormat8     #327   +/-   ##
=============================================================
  Coverage                         97.51%   97.51%           
  Complexity                         4657     4657           
=============================================================
  Files                               408      408           
  Lines                             11933    11933           
  Branches                            829      829           
=============================================================
  Hits                              11637    11637           
  Misses                              289      289           
  Partials                              7        7           
Flag Coverage Δ
sql-engine 97.51% <100.00%> (ø)

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

Files Changed Coverage Δ
...a/org/opensearch/sql/analysis/AnalysisContext.java 100.00% <ø> (ø)
...org/opensearch/sql/analysis/HighlightAnalyzer.java 100.00% <ø> (ø)
...ensearch/sql/analysis/NamedExpressionAnalyzer.java 100.00% <ø> (ø)
.../org/opensearch/sql/analysis/symbol/Namespace.java 100.00% <ø> (ø)
...search/sql/data/model/AbstractExprNumberValue.java 100.00% <ø> (ø)
...g/opensearch/sql/data/model/AbstractExprValue.java 100.00% <ø> (ø)
...rg/opensearch/sql/data/model/ExprBooleanValue.java 100.00% <ø> (ø)
...a/org/opensearch/sql/data/model/ExprByteValue.java 100.00% <ø> (ø)
...org/opensearch/sql/data/model/ExprDoubleValue.java 100.00% <ø> (ø)
.../org/opensearch/sql/data/model/ExprFloatValue.java 100.00% <ø> (ø)
... and 192 more

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

integ-test/build.gradle Outdated Show resolved Hide resolved
integ-test/build.gradle Outdated Show resolved Hide resolved
integ-test/build.gradle Outdated Show resolved Hide resolved
Comment on lines 226 to 230
"{ \"fetch_size\": 200, \"query\": \" SELECT age, state FROM %s WHERE age > ? OR"
+ " state IN (?, ?)\", \"parameters\": [ { \"type\": \"integer\", "
+ " \"value\": 25 }, { \"type\": \"string\", \"value\":"
+ " \"WA\" }, { \"type\": \"string\", \"value\": \"UT\""
+ " } ]}",

Choose a reason for hiding this comment

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

Is is possible to ask spotless to keep some string literals unmodified aka exception? Please, check.

Copy link
Author

Choose a reason for hiding this comment

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

To my best research, it is not possible to have an exception for that in spotless. Spotless seems to be a very strict application of the Google Java Format.

Copy link

@Yury-Fridlyand Yury-Fridlyand Aug 5, 2023

Choose a reason for hiding this comment

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

I see SL keeps format for other tests. See newSettingNewEndpoint on this PR or https://github.com/Bit-Quill/opensearch-project-sql/pull/328/files#diff-2f3432d682c1cca41cfda328adbdf15149e610abb1dc522082218c1d6126669fR98-R111 (integ-test/src/test/java/org/opensearch/sql/legacy/PluginIT.java)

Copy link
Author

Choose a reason for hiding this comment

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

Fixed.

JSONObject result =
executeQuery(
String.format(
"SELECT gender " + "FROM %s " + "GROUP BY gender " + "HAVING COUNT(*) > 0",

Choose a reason for hiding this comment

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

Since this has been moved to be on the same row rather than split onto multiple rows I believe that we can have this whole row be in one set of quotations rather than split into multiple with + signs between them

Copy link
Author

Choose a reason for hiding this comment

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

Good catch, thanks, fixed.

JSONObject result =
executeQuery(
String.format(
"SELECT a.gender " + "FROM %s a " + "GROUP BY a.gender " + "HAVING COUNT(*) > 0",

Choose a reason for hiding this comment

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

Same as above, this can be combined into a single quote

Copy link
Author

Choose a reason for hiding this comment

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

Good catch, thanks, fixed.

Comment on lines +629 to +644
/**
* <a>
* http://www.elasticsearch.org/guide/en/elasticsearch/reference/current/search-aggregations-bucket-daterange-aggregation.html</a>
*/
@Test
@Ignore
public void countDateRangeTest() throws IOException {
String result =
explainQuery(
String.format(
"select online from %s group by date_range("
+ "field='insert_time', 'format'='yyyy-MM-dd' ,'2014-08-18','2014-08-17', "
+ "'now-8d','now-7d','now-6d','now')",
TEST_INDEX_ONLINE));
// TODO: fix the query or fix the code for the query to work
}

Choose a reason for hiding this comment

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

Should this have been uncommented out?

Copy link
Author

Choose a reason for hiding this comment

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

Was requested here: #327 (comment). The @ignore was added to not run the test.

"| where match_phrase_prefix(Tags, 'gas ta', slop=2) " +
"| fields Tags";
String query =
"source = %s" + "| where match_phrase_prefix(Tags, 'gas ta', slop=2) " + "| fields Tags";

Choose a reason for hiding this comment

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

unneeded split up quotations

Copy link
Author

Choose a reason for hiding this comment

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

thanks, fixed.

"| where match_phrase_prefix(Tags, 'gas ta', slop=3)" +
"| fields Tags";
String query =
"source = %s" + "| where match_phrase_prefix(Tags, 'gas ta', slop=3)" + "| fields Tags";

Choose a reason for hiding this comment

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

split quotations

Copy link
Author

Choose a reason for hiding this comment

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

thanks, fixed.

@MitchellGale MitchellGale force-pushed the dev/sl_GoogleJavaFormat8 branch 2 times, most recently from 4baa02d to 0521d7c Compare August 14, 2023 21:28
@MitchellGale MitchellGale changed the base branch from integ/sl_GoogleJavaFormat8 to Integ/Fix-StopPrometheus August 14, 2023 21:32
@MitchellGale MitchellGale changed the base branch from Integ/Fix-StopPrometheus to integ/sl_GoogleJavaFormat8 August 14, 2023 21:32
@MitchellGale MitchellGale changed the base branch from integ/sl_GoogleJavaFormat8 to Integ/Fix-StopPrometheus August 14, 2023 21:48
@MitchellGale MitchellGale changed the base branch from Integ/Fix-StopPrometheus to integ/sl_GoogleJavaFormat8 August 14, 2023 21:48
@MitchellGale MitchellGale changed the base branch from integ/sl_GoogleJavaFormat8 to integ/sl_GJF8 August 14, 2023 21:49
@MitchellGale MitchellGale changed the base branch from integ/sl_GJF8 to integ/sl_GoogleJavaFormat8 August 14, 2023 21:53
Signed-off-by: Mitchell Gale <[email protected]>

add ignore failures for build.gradle.

Signed-off-by: Mitchell Gale <[email protected]>

Reverting ignore for checkstyle in integ-test

Signed-off-by: Mitchell Gale <[email protected]>

Addressed PR comments.

Signed-off-by: Mitchell Gale <[email protected]>

Addressed PR comments to expand jav doc.

Signed-off-by: Mitchell Gale <[email protected]>

fixed string formatting

Signed-off-by: Mitchell Gale <[email protected]>

Fixed string formatting.

Signed-off-by: Mitchell Gale <[email protected]>

Fixed string formatting in MatchPhrasePrefixIT

Signed-off-by: Mitchell Gale <[email protected]>
@MitchellGale MitchellGale force-pushed the dev/sl_GoogleJavaFormat8 branch from 0521d7c to 3e16d96 Compare August 14, 2023 22:10
@MitchellGale MitchellGale merged commit 550105b into integ/sl_GoogleJavaFormat8 Aug 14, 2023
MitchellGale added a commit that referenced this pull request Aug 16, 2023
…-project#1962)

* spotless apply for 81 integ-test files (#327)

add ignore failures for build.gradle.



Reverting ignore for checkstyle in integ-test



Addressed PR comments.



Addressed PR comments to expand jav doc.



fixed string formatting



Fixed string formatting.



Fixed string formatting in MatchPhrasePrefixIT

Signed-off-by: Mitchell Gale <[email protected]>

* Apply suggestions from code review

Co-authored-by: Yury-Fridlyand <[email protected]>
Signed-off-by: Mitchell Gale <[email protected]>

* address PR comments

Signed-off-by: Mitchell Gale <[email protected]>

---------

Signed-off-by: Mitchell Gale <[email protected]>
Signed-off-by: Mitchell Gale <[email protected]>
Co-authored-by: Yury-Fridlyand <[email protected]>
MitchellGale added a commit that referenced this pull request Aug 22, 2023
…-project#1962)

* spotless apply for 81 integ-test files (#327)

add ignore failures for build.gradle.

Reverting ignore for checkstyle in integ-test

Addressed PR comments.

Addressed PR comments to expand jav doc.

fixed string formatting

Fixed string formatting.

Fixed string formatting in MatchPhrasePrefixIT

Signed-off-by: Mitchell Gale <[email protected]>

* Apply suggestions from code review

Co-authored-by: Yury-Fridlyand <[email protected]>
Signed-off-by: Mitchell Gale <[email protected]>

* address PR comments

Signed-off-by: Mitchell Gale <[email protected]>

---------

Signed-off-by: Mitchell Gale <[email protected]>
Signed-off-by: Mitchell Gale <[email protected]>
Co-authored-by: Yury-Fridlyand <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants