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

Localization tests #2582

Merged
merged 4 commits into from
Feb 22, 2017
Merged

Localization tests #2582

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6d7f9be
Test for #1985
stefan-kolb Feb 22, 2017
d99115e
Remove closing brackets
stefan-kolb Feb 22, 2017
09576fb
Add tests
stefan-kolb Feb 22, 2017
0ab0cb5
Renaming
stefan-kolb Feb 22, 2017
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
20 changes: 20 additions & 0 deletions src/test/java/org/jabref/logic/l10n/LocalizationConsistencyTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import org.junit.Test;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;

public class LocalizationConsistencyTest {

Expand Down Expand Up @@ -168,4 +169,23 @@ public void findObsoleteMenuLocalizationKeys() throws IOException {
Collections.<String>emptySet(), obsoleteKeys);
}

@Test
public void localizationParameterMustIncludeAString() throws IOException {
// Must start or end with "
// Localization.lang("test"), Localization.lang("test" + var), Localization.lang(var + "test")
// TODO: Localization.lang(var1 + "test" + var2) not covered
// Localization.lang("Problem downloading from %1", address)
Set<LocalizationEntry> keys = LocalizationParser.findLocalizationParametersStringsInJavaFiles(LocalizationBundleForTest.LANG);
for(LocalizationEntry e : keys) {
assertTrue("Illegal localization parameter found. Must include a String with potential concatenation or replacement parameters. Illegal parameter: Localization.lang(" + e.getKey(),
e.getKey().startsWith("\"") || e.getKey().endsWith("\""));
}

keys = LocalizationParser.findLocalizationParametersStringsInJavaFiles(LocalizationBundleForTest.MENU);
for(LocalizationEntry e : keys) {
assertTrue("Illegal localization parameter found. Must include a String with potential concatenation or replacement parameters. Illegal parameter: Localization.lang(" + e.getKey(),
e.getKey().startsWith("\"") || e.getKey().endsWith("\""));
}
}

}
87 changes: 65 additions & 22 deletions src/test/java/org/jabref/logic/l10n/LocalizationParser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,14 @@ private static Set<LocalizationEntry> findLocalizationEntriesInFiles(Localizatio
}
}

public static Set<LocalizationEntry> findLocalizationParametersStringsInJavaFiles(LocalizationBundleForTest type)
throws IOException {
return Files.walk(Paths.get("src/main"))
Copy link
Member

@Siedlerchr Siedlerchr Feb 22, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

You could directly use the Files.find method and pass the filter predicate as argument. maxDepth would then be Integer.MaxValue

Copy link
Member Author

Choose a reason for hiding this comment

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

Just looked at it. Doesn't seem like a real change to me except that its a parameter then?!

Copy link
Member

Choose a reason for hiding this comment

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

A bit different. Instead of collecting all files and then filtering on the list(the stream), the Files.find directly invokes the predicate on each file and the file is only included in the stream if it matches:

This method walks the file tree in exactly the manner specified by the walk method. For each file encountered, the given BiPredicate is invoked with its Path and BasicFileAttributes. The Path object is obtained as if by resolving the relative path against start and is only included in the returned Stream if the BiPredicate returns true. Compare to calling filter on the Stream returned by walk method, this method may be more efficient by avoiding redundant retrieval of the BasicFileAttributes.

https://docs.oracle.com/javase/8/docs/api/java/nio/file/Files.html#find-java.nio.file.Path-int-java.util.function.BiPredicate-java.nio.file.FileVisitOption...-

look in FileUtil.findFiles

.filter(LocalizationParser::isJavaFile)
.flatMap(path -> getLocalizationParametersInJavaFile(path, type).stream())
.collect(Collectors.toSet());
}

private static Set<LocalizationEntry> findLocalizationEntriesInJavaFiles(LocalizationBundleForTest type)
throws IOException {
return Files.walk(Paths.get("src/main"))
Expand Down Expand Up @@ -141,6 +149,26 @@ private static List<LocalizationEntry> getLanguageKeysInJavaFile(Path path, Loca
return result;
}

private static List<LocalizationEntry> getLocalizationParametersInJavaFile(Path path, LocalizationBundleForTest type) {
List<LocalizationEntry> result = new LinkedList<>();

try {
List<String> lines = Files.readAllLines(path, StandardCharsets.UTF_8);
String content = String.join("\n", lines);

List<String> keys = JavaLocalizationEntryParser.getLocalizationParameter(content, type);

for (String key : keys) {
result.add(new LocalizationEntry(path, key, type));
}

} catch (IOException ignore) {
ignore.printStackTrace();
}

return result;
}

/**
* Loads the fxml file and returns all used language resources.
*/
Expand Down Expand Up @@ -192,31 +220,13 @@ static class JavaLocalizationEntryParser {
private static final Pattern QUOTATION_SYMBOL = Pattern.compile("QUOTATIONPLACEHOLDER");

public static List<String> getLanguageKeysInString(String content, LocalizationBundleForTest type) {
List<String> parameters = getLocalizationParameter(content, type);

List<String> result = new LinkedList<>();

Matcher matcher;
if (type == LocalizationBundleForTest.LANG) {
matcher = LOCALIZATION_START_PATTERN.matcher(content);
} else {
matcher = LOCALIZATION_MENU_START_PATTERN.matcher(content);
}
while (matcher.find()) {
// find contents between the brackets, covering multi-line strings as well
int index = matcher.end();
int brackets = 1;
StringBuilder buffer = new StringBuilder();
while (brackets != 0) {
char c = content.charAt(index);
if (c == '(') {
brackets++;
} else if (c == ')') {
brackets--;
}
buffer.append(c);
index++;
}
for (String param : parameters) {

String parsedContentsOfLangMethod = ESCAPED_QUOTATION_SYMBOL.matcher(buffer.toString()).replaceAll("QUOTATIONPLACEHOLDER");
String parsedContentsOfLangMethod = ESCAPED_QUOTATION_SYMBOL.matcher(param).replaceAll("QUOTATIONPLACEHOLDER");

// only retain what is within quotation
StringBuilder b = new StringBuilder();
Expand Down Expand Up @@ -259,6 +269,39 @@ public static List<String> getLanguageKeysInString(String content, LocalizationB
return result;
}

public static List<String> getLocalizationParameter(String content, LocalizationBundleForTest type) {
List<String> result = new LinkedList<>();

Matcher matcher;
if (type == LocalizationBundleForTest.LANG) {
matcher = LOCALIZATION_START_PATTERN.matcher(content);
} else {
matcher = LOCALIZATION_MENU_START_PATTERN.matcher(content);
}
while (matcher.find()) {
// find contents between the brackets, covering multi-line strings as well
int index = matcher.end();
int brackets = 1;
StringBuilder buffer = new StringBuilder();
while (brackets != 0) {
Copy link
Member

Choose a reason for hiding this comment

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

Look in our StringUtil class, there is already a method for finding braces

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for the hints, but this is old code which I didnt write and don't wanna change if not really necessary.

Copy link
Member

Choose a reason for hiding this comment

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

But you created a test for it right? So you could check it....

char c = content.charAt(index);
if (c == '(') {
brackets++;
} else if (c == ')') {
brackets--;
}
// skip closing brackets
if (brackets != 0) {
buffer.append(c);
}
index++;
}
// trim newlines and whitespace
result.add(buffer.toString().trim());
}

return result;
}
}

}
43 changes: 30 additions & 13 deletions src/test/java/org/jabref/logic/l10n/LocalizationParserTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,27 +11,44 @@
public class LocalizationParserTest {

@Test
public void testParsingCode() {
assertLocalizationParsing("Localization.lang(\"one per line\")", "one_per_line");
assertLocalizationParsing("Localization.lang(\n \"Copy \\\\cite{BibTeX key}\")", "Copy_\\cite{BibTeX_key}");
assertLocalizationParsing("Localization.lang(\"two per line\") Localization.lang(\"two per line\")", Arrays.asList("two_per_line", "two_per_line"));
assertLocalizationParsing("Localization.lang(\"multi \" + \n\"line\")", "multi_line");
assertLocalizationParsing("Localization.lang(\"one per line with var\", var)", "one_per_line_with_var");
assertLocalizationParsing("Localization.lang(\"Search %0\", \"Springer\")", "Search_%0");
assertLocalizationParsing("Localization.lang(\"Reset preferences (key1,key2,... or 'all')\")", "Reset_preferences_(key1,key2,..._or_'all')");
assertLocalizationParsing("Localization.lang(\"Multiple entries selected. Do you want to change the type of all these to '%0'?\")",
public void testKeyParsingCode() {
assertLocalizationKeyParsing("Localization.lang(\"one per line\")", "one_per_line");
assertLocalizationKeyParsing("Localization.lang(\n \"Copy \\\\cite{BibTeX key}\")", "Copy_\\cite{BibTeX_key}");
assertLocalizationKeyParsing("Localization.lang(\"two per line\") Localization.lang(\"two per line\")", Arrays.asList("two_per_line", "two_per_line"));
assertLocalizationKeyParsing("Localization.lang(\"multi \" + \n\"line\")", "multi_line");
assertLocalizationKeyParsing("Localization.lang(\"one per line with var\", var)", "one_per_line_with_var");
assertLocalizationKeyParsing("Localization.lang(\"Search %0\", \"Springer\")", "Search_%0");
assertLocalizationKeyParsing("Localization.lang(\"Reset preferences (key1,key2,... or 'all')\")", "Reset_preferences_(key1,key2,..._or_'all')");
assertLocalizationKeyParsing("Localization.lang(\"Multiple entries selected. Do you want to change the type of all these to '%0'?\")",
"Multiple_entries_selected._Do_you_want_to_change_the_type_of_all_these_to_'%0'?");
assertLocalizationParsing("Localization.lang(\"Run fetcher, e.g. \\\"--fetch=Medline:cancer\\\"\");",
assertLocalizationKeyParsing("Localization.lang(\"Run fetcher, e.g. \\\"--fetch=Medline:cancer\\\"\");",
"Run_fetcher,_e.g._\"--fetch\\=Medline\\:cancer\"");
}

private void assertLocalizationParsing(String code, String expectedLanguageKeys) {
assertLocalizationParsing(code, Collections.singletonList(expectedLanguageKeys));
@Test
public void testParameterParsingCode() {
assertLocalizationParameterParsing("Localization.lang(\"one per line\")", "\"one per line\"");
assertLocalizationParameterParsing("Localization.lang(\"one per line\" + var)", "\"one per line\" + var");
assertLocalizationParameterParsing("Localization.lang(var + \"one per line\")", "var + \"one per line\"");
assertLocalizationParameterParsing("Localization.lang(\"Search %0\", \"Springer\")", "\"Search %0\", \"Springer\"");
}

private void assertLocalizationKeyParsing(String code, String expectedLanguageKeys) {
assertLocalizationKeyParsing(code, Collections.singletonList(expectedLanguageKeys));
}

private void assertLocalizationParsing(String code, List<String> expectedLanguageKeys) {
private void assertLocalizationKeyParsing(String code, List<String> expectedLanguageKeys) {
List<String> languageKeysInString = LocalizationParser.JavaLocalizationEntryParser.getLanguageKeysInString(code, LocalizationBundleForTest.LANG);
assertEquals(expectedLanguageKeys, languageKeysInString);
}

private void assertLocalizationParameterParsing(String code, List<String> expectedParameter) {
List<String> languageKeysInString = LocalizationParser.JavaLocalizationEntryParser.getLocalizationParameter(code, LocalizationBundleForTest.LANG);
assertEquals(expectedParameter, languageKeysInString);
}

private void assertLocalizationParameterParsing(String code, String expectedParameter) {
assertLocalizationParameterParsing(code, Collections.singletonList(expectedParameter));
}

}