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.

Sign up for GitHub

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

Jump to bottom

SQL: skip uppercasing/lowercasing function tests for AZ locales as well #32910

Merged
merged 6 commits into from
Aug 22, 2018
Merged

SQL: skip uppercasing/lowercasing function tests for AZ locales as well #32910

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
1bfeec2
Added the rest of the Locales that have different behavior for upperc…
astefan Aug 16, 2018
d75b274
Merge remote-tracking branch 'remotes/origin/master' into 32589_fix_a…
astefan Aug 16, 2018
e0a8e06
Restored the initial Locale in tests
astefan Aug 17, 2018
d1f886d
Correct character encoding for utf8
astefan Aug 17, 2018
c29b990
Merge remote-tracking branch 'remotes/origin/master' into 32589_fix_a…
astefan Aug 20, 2018
03c4343
Wrapped the Locale changing tests in try {} finally {} block
astefan Aug 21, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions ...asticsearch/xpack/sql/expression/function/scalar/string/StringFunctionProcessorTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ public void testLCase() {
stringCharInputValidation(proc);
}

public void testLCaseWithTRLocale() {
public void testLCaseWithAZTRLocale() {
Locale.setDefault(Locale.forLanguageTag("tr"));
StringProcessor proc = new StringProcessor(StringOperation.LCASE);

Expand All @@ -86,6 +86,10 @@ public void testLCaseWithTRLocale() {
// unicode 0049 = I (regular capital letter i)
// in Turkish locale this would be lowercased to a "i" without dot (unicode 0131)
assertEquals("\u0069", proc.process("\u0049"));

Locale.setDefault(Locale.forLanguageTag("az"));
assertEquals("\u0069\u0307", proc.process("\u0130"));
assertEquals("\u0069", proc.process("\u0049"));
}

public void testUCase() {
Expand All @@ -102,13 +106,16 @@ public void testUCase() {
stringCharInputValidation(proc);
}

public void testUCaseWithTRLocale() {
public void testUCaseWithAZTRLocale() {
Locale.setDefault(Locale.forLanguageTag("tr"));
StringProcessor proc = new StringProcessor(StringOperation.UCASE);

// ES-SQL is not Locale sensitive (so far).
// in Turkish locale, small letter "i" is uppercased to "I" with a dot above (unicode 130), otherwise in "i" (unicode 49)
assertEquals("\u0049", proc.process("\u0069"));

Locale.setDefault(Locale.forLanguageTag("az"));
assertEquals("\u0049", proc.process("\u0069"));
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it might makes sense to restore the original locale after the test so other tests in this class don't run in this local most of the time.

Copy link
Member

Choose a reason for hiding this comment

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

Same deal here with restoring in a finally

}

public void testLength() {
Expand Down
9 changes: 5 additions & 4 deletions x-pack/qa/sql/src/main/java/org/elasticsearch/xpack/qa/sql/jdbc/SqlSpecTestCase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import java.sql.Connection;
import java.sql.ResultSet;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.Locale;

Expand All @@ -37,8 +38,7 @@ public static List<Object[]> readScriptSpec() throws Exception {
tests.addAll(readScriptSpec("/agg.sql-spec", parser));
tests.addAll(readScriptSpec("/arithmetic.sql-spec", parser));
tests.addAll(readScriptSpec("/string-functions.sql-spec", parser));
// AwaitsFix: https://github.com/elastic/elasticsearch/issues/32589
// tests.addAll(readScriptSpec("/case-functions.sql-spec", parser));
tests.addAll(readScriptSpec("/case-functions.sql-spec", parser));
return tests;
}

Expand All @@ -60,8 +60,9 @@ public SqlSpecTestCase(String fileName, String groupName, String testName, Integ

@Override
protected final void doTest() throws Throwable {
boolean goodLocale = !(Locale.getDefault().equals(new Locale.Builder().setLanguageTag("tr").build())
|| Locale.getDefault().equals(new Locale.Builder().setLanguageTag("tr-TR").build()));
String[] badLocales = new String[] {"tr", "az", "tr-TR", "tr-CY", "az-Latn", "az-Cyrl", "az-Latn-AZ", "az-Cyrl-AZ"};
Copy link
Contributor

Choose a reason for hiding this comment

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

These locales are not bad :) it's just some application are buggy and cannot work with them correctly. So, maybe we should call them h2IncompatibleLocales or dottedILocales or something like this.

boolean goodLocale = !Arrays.stream(badLocales)
.anyMatch((l) -> Locale.getDefault().equals(new Locale.Builder().setLanguageTag(l).build()));
if (fileName.startsWith("case-functions")) {
Assume.assumeTrue(goodLocale);
}
Expand Down