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

CLDR-16393 assume cldr-archive is present for tests #3215

Merged
merged 10 commits into from
Nov 30, 2023
Merged

CLDR-16393 assume cldr-archive is present for tests #3215

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
bdd4931
CLDR-16393 assume cldr-archive is present for tests
srl295 Aug 23, 2023
1ed1b38
CLDR-16393 reinstate some tests that depend on cldr-archive
srl295 Aug 23, 2023
cde86d9
CLDR-16393 add a test for loading SDI
srl295 Aug 30, 2023
a5d79eb
CLDR-16393 fix breakage with old DTDs
srl295 Aug 31, 2023
50d8877
CLDR-16393 spotless
srl295 Aug 31, 2023
4bd5711
CLDR-16393 merge
srl295 Nov 29, 2023
bb85440
CLDR-16393 code review comments
srl295 Nov 30, 2023
50bf437
CLDR-16393 change the property to -DHAS_CLDR_ARCHIVE
srl295 Nov 30, 2023
84a677c
CLDR-16393 update failure message
srl295 Nov 30, 2023
d29b102
CLDR-16393 spotless
srl295 Nov 30, 2023
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
1 change: 1 addition & 0 deletions tools/cldr-code/src/main/java/org/unicode/cldr/tool/CheckHtmlFiles.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,7 @@ private static void checkForDtd(Data target) {
M4<String, String, DtdType, Boolean> typeToElements =
ChainedMap.of(new TreeMap(), new TreeMap(), new TreeMap(), Boolean.class);
for (DtdType type : DtdType.values()) {
if (type.getStatus() != DtdType.DtdStatus.active) continue;
if (type == DtdType.ldmlICU) continue;
DtdData dtdData = DtdData.getInstance(type);
Set<Element> elements = dtdData.getElements();
Expand Down
9 changes: 9 additions & 0 deletions tools/cldr-code/src/main/java/org/unicode/cldr/tool/CldrVersion.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import java.util.Map;
import java.util.Set;
import java.util.TreeSet;
import org.unicode.cldr.util.CLDRFile;
import org.unicode.cldr.util.CLDRPaths;
import org.unicode.cldr.util.SupplementalDataInfo;

Expand Down Expand Up @@ -67,6 +68,9 @@ public enum CldrVersion {
v42_0,
v43_0,
v44_0,
/**
* @see CLDRFile#GEN_VERSION
*/
baseline;

private final String baseDirectory;
Expand Down Expand Up @@ -98,6 +102,11 @@ public static CldrVersion from(VersionInfo versionInfo) {
}

public static CldrVersion from(String versionString) {
// treat 'current' as baseline
if (versionString.equals(CLDRFile.GEN_VERSION)
|| versionString.equals(CLDRFile.GEN_VERSION + ".0")) {
return CldrVersion.baseline;
}
return valueOf(
versionString.charAt(0) < 'A'
? "v" + versionString.replace('.', '_')
Expand Down
1 change: 1 addition & 0 deletions tools/cldr-code/src/main/java/org/unicode/cldr/tool/GenerateDtd.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ public static void main(String[] args) throws IOException {
final DTD2Markdown dtd2md = new DTD2Markdown();
// System.setProperty("show_all", "true");
for (DtdType type : DtdType.values()) {
if (type.getStatus() != DtdType.DtdStatus.active) continue;
Copy link
Member

Choose a reason for hiding this comment

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

Missing {...}

Copy link
Member

Choose a reason for hiding this comment

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

This happens multiple times below, so check for those also.

if (type == DtdType.ldmlICU) {
continue;
}
Expand Down
1 change: 1 addition & 0 deletions tools/cldr-code/src/main/java/org/unicode/cldr/tool/GenerateItemCounts.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,7 @@ private static void checkBadAttributes(Relation<String, String> path2value2, Str

SupplementalDataInfo supp = SUPPLEMENTAL_DATA_INFO;
for (DtdType dtdType : DtdType.values()) {
if (dtdType.getStatus() != DtdType.DtdStatus.active) continue;
Copy link
Member

Choose a reason for hiding this comment

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

I think there's an Eclipse tool that will add {...} after every if statement. If you want to fix these (and others) that way, a separate PR would work.

Copy link
Member Author

Choose a reason for hiding this comment

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

Since were using spotless it would be better to set spotless to do this.

Copy link
Member Author

Choose a reason for hiding this comment

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

@macchiati there are 2,000+ cases of single-line if statements in the code. I couldn't find a spotless setting to fix this, though eclipse has one.

Copy link
Member

Choose a reason for hiding this comment

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

@macchiati @srl295 here is a PR with comments implying that Google Java Style does call for doing what Mark wants here, however google-java-format has a "constraint of only modifying whitespace": google/google-java-format#938

The implication seems to be that if we inserted brackets as a one-time operation using a different tool, then spotless configured for google-java-format would preserve the brackets, but exceptions could still arise in new code

if (dtdType == DtdType.ldmlICU) {
continue;
}
Expand Down
1 change: 1 addition & 0 deletions tools/cldr-code/src/main/java/org/unicode/cldr/tool/GenerateReformattedXml.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
public class GenerateReformattedXml {
public static void main(String[] args) {
for (DtdType dtdType : DtdType.values()) {
if (dtdType.getStatus() != DtdType.DtdStatus.active) continue;
if (args.length > 0 && !dtdType.toString().matches(args[0])) {
continue;
}
Expand Down
1 change: 1 addition & 0 deletions tools/cldr-code/src/main/java/org/unicode/cldr/tool/ShowStarredCoverage.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,7 @@ public static void main(String[] args) {
System.out.println("№\tLevel\tStarredPath\tPH Status\t?\tAttributes");
}
for (DtdType dtdType : DtdType.values()) {
if (dtdType.getStatus() != DtdType.DtdStatus.active) continue;
if (dtdTypes != null && !dtdTypes.contains(dtdType)) {
continue;
}
Expand Down
25 changes: 25 additions & 0 deletions tools/cldr-code/src/main/java/org/unicode/cldr/util/DtdType.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import com.google.common.collect.ImmutableSet;
import java.lang.annotation.Annotation;
import java.util.Collections;
import java.util.Set;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
Expand Down Expand Up @@ -29,6 +30,7 @@ public enum DtdType {
"transforms",
"validity"),
ldmlBCP47("common/dtd/ldmlBCP47.dtd", "1.7.2", null, "bcp47"),
// keyboard 3.0
keyboard3("keyboards/dtd/ldmlKeyboard3.dtd", "44.0", null, "../keyboards/3.0"),
keyboardTest3("keyboards/dtd/ldmlKeyboardTest3.dtd", "44.0", null, "../keyboards/test");

Expand All @@ -48,10 +50,32 @@ public enum DtdType {
public final String firstVersion;
public final Set<String> directories;

private final DtdStatus status;

/** Get the ststus, whether this is an active DTD or not */
public DtdStatus getStatus() {
return status;
}

public enum DtdStatus {
/** DTD active (default) */
active,
/** DTD no longer used */
removed,
};

private DtdType(String dtdPath) {
this(dtdPath, null, null);
}

private DtdType(DtdStatus status, String sinceVersion) {
this.directories = Collections.emptySet();
this.dtdPath = null;
this.rootType = this;
this.firstVersion = sinceVersion;
this.status = status;
}

private DtdType(String dtdPath, DtdType realType) {
this(dtdPath, null, realType);
}
Expand All @@ -61,6 +85,7 @@ private DtdType(String dtdPath, String firstVersion, DtdType realType, String...
this.rootType = realType == null ? this : realType;
this.firstVersion = firstVersion;
this.directories = ImmutableSet.copyOf(directories);
this.status = DtdStatus.active;
}

public static DtdType fromPath(String elementOrPath) {
Expand Down
68 changes: 39 additions & 29 deletions tools/cldr-code/src/main/java/org/unicode/cldr/util/ElementAttributeInfo.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,35 +55,7 @@ public static final ElementAttributeInfo getInstance(String commonDirectory, Dtd
}
}
if (result == null) {
result = new HashMap<>();
// pick short files that are in repository
// Add to this when a DTD is added
result.put(
DtdType.ldml,
new ElementAttributeInfo(
canonicalCommonDirectory + "/main/root.xml", DtdType.ldml));
result.put(
DtdType.supplementalData,
new ElementAttributeInfo(
canonicalCommonDirectory + "/supplemental/plurals.xml",
DtdType.supplementalData));
result.put(
DtdType.ldmlBCP47,
new ElementAttributeInfo(
canonicalCommonDirectory + "/bcp47/calendar.xml",
DtdType.ldmlBCP47));
result.put(
DtdType.keyboard3,
new ElementAttributeInfo(
canonicalCommonDirectory
+ "/../keyboards/3.0/fr-t-k0-azerty.xml",
DtdType.keyboard3));
result.put(
DtdType.keyboardTest3,
new ElementAttributeInfo(
canonicalCommonDirectory
+ "/../keyboards/test/fr-t-k0-azerty-test.xml",
DtdType.keyboardTest3));
result = makeElementAttributeInfoMap(canonicalCommonDirectory);
cache.put(commonDirectory, result);
cache.put(canonicalCommonDirectory, result);
}
Expand All @@ -101,6 +73,44 @@ public static final ElementAttributeInfo getInstance(String commonDirectory, Dtd
return eai;
}

private static void addElementAttributeInfo(
Map<DtdType, ElementAttributeInfo> result, DtdType type, String path)
throws IOException {
if (!new File(path).canRead()) {
System.err.println(
"ElementAttributeInfo: Warning: Sample file did not exist: "
+ path
+ " for DtdType "
+ type.name());
return; // file doesn't exist.
}
result.put(type, new ElementAttributeInfo(path, type));
}

private static Map<DtdType, ElementAttributeInfo> makeElementAttributeInfoMap(
String canonicalCommonDirectory) throws IOException {
Map<DtdType, ElementAttributeInfo> result;
result = new HashMap<>();
// pick short files that are in repository
// Add to this when a DTD is added
addElementAttributeInfo(result, DtdType.ldml, canonicalCommonDirectory + "/main/root.xml");
addElementAttributeInfo(
result,
DtdType.supplementalData,
canonicalCommonDirectory + "/supplemental/plurals.xml");
addElementAttributeInfo(
result, DtdType.ldmlBCP47, canonicalCommonDirectory + "/bcp47/calendar.xml");
addElementAttributeInfo(
result,
DtdType.keyboard3,
canonicalCommonDirectory + "/../keyboards/3.0/fr-t-k0-azerty.xml");
addElementAttributeInfo(
result,
DtdType.keyboardTest3,
canonicalCommonDirectory + "/../keyboards/test/fr-t-k0-azerty-test.xml");
return result;
}

// static {
// try {
// addFromDTD(CldrUtility.COMMON_DIRECTORY + "main/en.xml", DtdType.ldml);
Expand Down
5 changes: 5 additions & 0 deletions tools/cldr-code/src/main/java/org/unicode/cldr/util/SupplementalDataInfo.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@
import org.unicode.cldr.util.Builder.CBuilder;
import org.unicode.cldr.util.CldrUtility.VariableReplacer;
import org.unicode.cldr.util.DayPeriodInfo.DayPeriod;
import org.unicode.cldr.util.DtdType.DtdStatus;
import org.unicode.cldr.util.GrammarInfo.GrammaticalFeature;
import org.unicode.cldr.util.GrammarInfo.GrammaticalScope;
import org.unicode.cldr.util.GrammarInfo.GrammaticalTarget;
Expand Down Expand Up @@ -4572,6 +4573,10 @@ public MetaZoneRange getMetaZoneRange(String zone, long date) {
}

public boolean isDeprecated(DtdType type, String element, String attribute, String value) {
if (type.getStatus() == DtdStatus.removed) {
// if the DTD was removed, skip
return true;
}
return DtdData.getInstance(type).isDeprecated(element, attribute, value);
}

Expand Down
56 changes: 47 additions & 9 deletions tools/cldr-code/src/test/java/org/unicode/cldr/unittest/TestBasic.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import java.util.Collection;
import java.util.Collections;
import java.util.Comparator;
import java.util.EnumSet;
import java.util.HashSet;
import java.util.Iterator;
import java.util.LinkedHashSet;
Expand Down Expand Up @@ -62,6 +63,7 @@
import org.unicode.cldr.util.DtdData.Element;
import org.unicode.cldr.util.DtdData.ElementType;
import org.unicode.cldr.util.DtdType;
import org.unicode.cldr.util.DtdType.DtdStatus;
import org.unicode.cldr.util.ElementAttributeInfo;
import org.unicode.cldr.util.Factory;
import org.unicode.cldr.util.InputStreamFactory;
Expand All @@ -75,6 +77,7 @@
import org.unicode.cldr.util.SupplementalDataInfo;
import org.unicode.cldr.util.SupplementalDataInfo.PluralInfo;
import org.unicode.cldr.util.SupplementalDataInfo.PluralType;
import org.unicode.cldr.util.TestCLDRPaths;
import org.unicode.cldr.util.XMLFileReader;
import org.unicode.cldr.util.XPathParts;
import org.xml.sax.ErrorHandler;
Expand Down Expand Up @@ -224,6 +227,7 @@ public void showFoundElements(Relation<Row.R2<DtdType, String>, String> foundAtt
Relation<Row.R2<DtdType, String>, String> theoryAttributes =
Relation.of(new TreeMap<Row.R2<DtdType, String>, Set<String>>(), TreeSet.class);
for (DtdType type : DtdType.values()) {
if (type.getStatus() != DtdType.DtdStatus.active) continue;
DtdData dtdData = DtdData.getInstance(type);
for (Element element : dtdData.getElementFromName().values()) {
String name = element.getName();
Expand Down Expand Up @@ -1003,6 +1007,10 @@ static boolean isLikelyEquivalent(String locale1, String locale2) {

static final Map<String, String> likelyData = SUPPLEMENTAL_DATA_INFO.getLikelySubtags();

private static final EnumSet<CldrVersion> badLdmlICUVersions =
EnumSet.of(
CldrVersion.v1_1_1, CldrVersion.v1_2, CldrVersion.v1_4_1, CldrVersion.v1_5_1);

public void TestLikelySubtagsComplete() {
LanguageTagParser ltp = new LanguageTagParser();
for (String locale : testInfo.getCldrFactory().getAvailable()) {
Expand Down Expand Up @@ -1165,6 +1173,7 @@ private boolean isTopLevel(String localeID) {
/** Tests that every dtd item is connected from root */
public void TestDtdCompleteness() {
for (DtdType type : DtdType.values()) {
if (type.getStatus() != DtdType.DtdStatus.active) continue;
DtdData dtdData = DtdData.getInstance(type);
Set<Element> descendents = new LinkedHashSet<>();
dtdData.getDescendents(dtdData.ROOT, descendents);
Expand Down Expand Up @@ -1196,9 +1205,7 @@ public void TestDtdCompleteness() {

public void TestBasicDTDCompatibility() {

if (logKnownIssue(
"cldrbug:16393",
"Comment out until we detect whether to enable cldr-archive for unit tests")) {
if (!TestCLDRPaths.canUseArchiveDirectory()) {
return;
}

Expand Down Expand Up @@ -1226,6 +1233,13 @@ public void TestBasicDTDCompatibility() {

// test all DTDs
for (DtdType dtd : DtdType.values()) {
if (dtd.getStatus() != DtdType.DtdStatus.active) continue;
if (dtd.firstVersion != null
&& CldrVersion.LAST_RELEASE_VERSION.isOlderThan(
CldrVersion.from(dtd.firstVersion))) {
continue; // DTD didn't exist in last release
}
if (dtd == DtdType.ldmlICU) continue;
try {
ElementAttributeInfo oldDtd = ElementAttributeInfo.getInstance(oldCommon, dtd);
ElementAttributeInfo newDtd = ElementAttributeInfo.getInstance(dtd);
Expand Down Expand Up @@ -1334,6 +1348,7 @@ private <T> Set<T> containsInOrder(Set<T> superset, Set<T> subset) {
public void TestDtdCompatibility() {

for (DtdType type : DtdType.values()) {
if (type.getStatus() != DtdType.DtdStatus.active) continue;
DtdData dtdData = DtdData.getInstance(type);
Map<String, Element> currentElementFromName = dtdData.getElementFromName();

Expand Down Expand Up @@ -1390,26 +1405,35 @@ public void TestDtdCompatibility() {
Collections.EMPTY_SET,
elementsWithoutSpecial);

if (logKnownIssue(
"cldrbug:16393",
"Comment out test until cldr-archive is signaled for unit tests on CI")) {
if (!TestCLDRPaths.canUseArchiveDirectory()) {
return;
}

for (CldrVersion version : CldrVersion.CLDR_VERSIONS_DESCENDING) {
if (version == CldrVersion.unknown || version == CldrVersion.baseline) {
continue;
}
if (type.getStatus() != DtdStatus.active) {
continue; // not active
}
if (type.firstVersion != null
&& version.isOlderThan(CldrVersion.from(type.firstVersion))) {
continue; // didn't exist at that point
}
DtdData dtdDataOld;
try {
dtdDataOld = DtdData.getInstance(type, version.toString());
} catch (IllegalArgumentException e) {
boolean tooOld = false;
switch (type) {
case ldmlBCP47:
case ldmlICU:
tooOld = badLdmlICUVersions.contains(version);
break;
case ldmlBCP47:
case keyboard3:
tooOld = version.isOlderThan(CldrVersion.from(type.firstVersion));
if (type.firstVersion != null) {
tooOld = version.isOlderThan(CldrVersion.from(type.firstVersion));
}
break;
default:
break;
Expand All @@ -1418,7 +1442,8 @@ public void TestDtdCompatibility() {
continue;
} else {
errln(
version
"v"
+ version
+ ": "
+ e.getClass().getSimpleName()
+ ", "
Expand Down Expand Up @@ -1449,6 +1474,19 @@ public void TestDtdCompatibility() {
continue;
}
Element newChild = newElement.getChildNamed(oldChild.getName());
// skip certain items
if (version.isOlderThan(CldrVersion.v1_6_1)
&& newElement.getName().equals("zone")
&& oldChild.getName().equals("usesMetazone")) {
if (logKnownIssue(
"CLDR-17054",
"Breakage with items older than 1.6.1: "
+ newElement.getName()
+ " / "
+ oldChild.getName())) {
continue;
}
}

if (knownChildExceptions.contains(
Pair.of(newElement.getName(), oldChild.getName()))) {
Expand Down
Loading
Loading