Skip to content

Commit

Permalink
[FLINK-15458][conf][tests] Separate options discovery and well-define…
Browse files Browse the repository at this point in the history 
…dness check
  • Loading branch information
@zentol
zentol committed Jan 18, 2020
1 parent ab6b359 commit 7be292c
Showing 1 changed file with 85 additions and 83 deletions.
168 changes: 85 additions & 83 deletions ...rc/test/java/org/apache/flink/docs/configuration/ConfigOptionsDocsCompletenessITCase.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
import java.nio.file.Paths;
import java.util.ArrayList;
import java.util.Collection;
import java.util.HashMap;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.Objects;
Expand Down Expand Up @@ -63,49 +63,94 @@ public class ConfigOptionsDocsCompletenessITCase {

@Test
public void testCommonSectionCompleteness() throws IOException, ClassNotFoundException {
Map<String, DocumentedOption> documentedOptions = parseDocumentedCommonOptions();
Map<String, ExistingOption> existingOptions = findExistingOptions(
Map<String, List<DocumentedOption>> documentedOptions = parseDocumentedCommonOptions();
Map<String, List<ExistingOption>> existingOptions = findExistingOptions(
optionWithMetaInfo -> optionWithMetaInfo.field.getAnnotation(Documentation.CommonOption.class) != null);

assertExistingOptionsAreWellDefined(existingOptions);

compareDocumentedAndExistingOptions(documentedOptions, existingOptions);
}

@Test
public void testFullReferenceCompleteness() throws IOException, ClassNotFoundException {
Map<String, DocumentedOption> documentedOptions = parseDocumentedOptions();
Map<String, ExistingOption> existingOptions = findExistingOptions(ignored -> true);
Map<String, List<DocumentedOption>> documentedOptions = parseDocumentedOptions();
Map<String, List<ExistingOption>> existingOptions = findExistingOptions(ignored -> true);

assertExistingOptionsAreWellDefined(existingOptions);

compareDocumentedAndExistingOptions(documentedOptions, existingOptions);
}

private static void compareDocumentedAndExistingOptions(Map<String, DocumentedOption> documentedOptions, Map<String, ExistingOption> existingOptions) {
private static void assertExistingOptionsAreWellDefined(Map<String, List<ExistingOption>> allOptions) {
allOptions.values().stream()
.forEach(options -> options.stream()
.reduce((option1, option2) -> {
if (option1.equals(option2)) {
// we allow multiple instances of ConfigOptions with the same key if they are identical
return option1;
} else {
// found a ConfigOption pair with the same key that aren't equal
// we fail here outright as this is not a documentation-completeness problem
if (!option1.defaultValue.equals(option2.defaultValue)) {
String errorMessage = String.format(
"Ambiguous option %s due to distinct default values (%s (in %s) vs %s (in %s)).",
option1.key,
option1.defaultValue,
option1.containingClass.getSimpleName(),
option2.defaultValue,
option2.containingClass.getSimpleName());
throw new AssertionError(errorMessage);
} else {
String errorMessage = String.format(
"Ambiguous option %s due to distinct descriptions (%s vs %s).",
option1.key,
option1.containingClass.getSimpleName(),
option2.containingClass.getSimpleName());
throw new AssertionError(errorMessage);
}
}
}));
}

private static void compareDocumentedAndExistingOptions(Map<String, List<DocumentedOption>> documentedOptions, Map<String, List<ExistingOption>> existingOptions) {
final Collection<String> problems = new ArrayList<>(0);

// first check that all existing options are properly documented
existingOptions.forEach((key, supposedState) -> {
DocumentedOption documentedState = documentedOptions.remove(key);

// if nothing matches the docs for this option are up-to-date
if (documentedState == null) {
// option is not documented at all
problems.add("Option " + supposedState.key + " in " + supposedState.containingClass + " is not documented.");
} else if (!supposedState.defaultValue.equals(documentedState.defaultValue)) {
// default is outdated
problems.add("Documented default of " + supposedState.key + " in " + supposedState.containingClass +
" is outdated. Expected: " + supposedState.defaultValue + " Actual: " + documentedState.defaultValue);
} else if (!supposedState.description.equals(documentedState.description)) {
// description is outdated
problems.add("Documented description of " + supposedState.key + " in " + supposedState.containingClass +
" is outdated.");
existingOptions.forEach((key, supposedStates) -> {
List<DocumentedOption> documentedState = documentedOptions.get(key);

for (ExistingOption supposedState : supposedStates) {
if (documentedState == null || documentedState.isEmpty()) {
// option is not documented at all
problems.add("Option " + supposedState.key + " in " + supposedState.containingClass + " is not documented.");
} else {
final Iterator<DocumentedOption> candidates = documentedState.iterator();

boolean matchFound = false;
while (candidates.hasNext() && !matchFound) {
DocumentedOption candidate = candidates.next();
if (supposedState.defaultValue.equals(candidate.defaultValue) && supposedState.description.equals(candidate.description)) {
matchFound = true;
candidates.remove();
}
}

if (!matchFound) {
problems.add(String.format(
"Documentation of %s in %s is outdated. Expected: default=(%s) description=(%s).",
supposedState.key,
supposedState.containingClass.getSimpleName(),
supposedState.defaultValue,
supposedState.description));
}
}
}
});

// documentation contains an option that no longer exists
if (!documentedOptions.isEmpty()) {
for (DocumentedOption documentedOption : documentedOptions.values()) {
problems.add("Documented option " + documentedOption.key + " does not exist.");
}
}
documentedOptions.values().stream().flatMap(Collection::stream)
.forEach(documentedOption -> problems.add("Documented option " + documentedOption.key + " does not exist."));

if (!problems.isEmpty()) {
StringBuilder sb = new StringBuilder("Documentation is outdated, please regenerate it according to the" +
Expand All @@ -121,28 +166,13 @@ private static void compareDocumentedAndExistingOptions(Map<String, DocumentedOp
}
}

private static Map<String, DocumentedOption> parseDocumentedCommonOptions() throws IOException {
private static Map<String, List<DocumentedOption>> parseDocumentedCommonOptions() throws IOException {
Path commonSection = Paths.get(System.getProperty("rootDir"), "docs", "_includes", "generated", COMMON_SECTION_FILE_NAME);
return parseDocumentedOptionsFromFile(commonSection).stream()
.collect(Collectors.toMap(option -> option.key, option -> option, (option1, option2) -> {
if (option1.equals(option2)) {
// we allow multiple instances of ConfigOptions with the same key if they are identical
return option1;
} else {
// found a ConfigOption pair with the same key that aren't equal
// we fail here outright as this is not a documentation-completeness problem
if (!option1.defaultValue.equals(option2.defaultValue)) {
throw new AssertionError("Documentation contains distinct defaults for " +
option1.key + " in " + option1.containingFile + " and " + option2.containingFile + '.');
} else {
throw new AssertionError("Documentation contains distinct descriptions for " +
option1.key + " in " + option1.containingFile + " and " + option2.containingFile + '.');
}
}
}));
.collect(Collectors.groupingBy(option -> option.key, Collectors.toList()));
}

private static Map<String, DocumentedOption> parseDocumentedOptions() throws IOException {
private static Map<String, List<DocumentedOption>> parseDocumentedOptions() throws IOException {
Path includeFolder = Paths.get(System.getProperty("rootDir"), "docs", "_includes", "generated").toAbsolutePath();
return Files.list(includeFolder)
.filter(path -> path.getFileName().toString().contains("configuration"))
Expand All @@ -153,22 +183,7 @@ private static Map<String, DocumentedOption> parseDocumentedOptions() throws IOE
return Stream.empty();
}
})
.collect(Collectors.toMap(option -> option.key, option -> option, (option1, option2) -> {
if (option1.equals(option2)) {
// we allow multiple instances of ConfigOptions with the same key if they are identical
return option1;
} else {
// found a ConfigOption pair with the same key that aren't equal
// we fail here outright as this is not a documentation-completeness problem
if (!option1.defaultValue.equals(option2.defaultValue)) {
throw new AssertionError("Documentation contains distinct defaults for " +
option1.key + " in " + option1.containingFile + " and " + option2.containingFile + '.');
} else {
throw new AssertionError("Documentation contains distinct descriptions for " +
option1.key + " in " + option1.containingFile + " and " + option2.containingFile + '.');
}
}
}));
.collect(Collectors.groupingBy(option -> option.key, Collectors.toList()));
}

private static Collection<DocumentedOption> parseDocumentedOptionsFromFile(Path file) throws IOException {
Expand Down Expand Up @@ -198,33 +213,20 @@ private static Collection<DocumentedOption> parseDocumentedOptionsFromFile(Path
.collect(Collectors.toList());
}

private static Map<String, ExistingOption> findExistingOptions(Predicate<ConfigOptionsDocGenerator.OptionWithMetaInfo> predicate) throws IOException, ClassNotFoundException {
Map<String, ExistingOption> existingOptions = new HashMap<>(32);
private static Map<String, List<ExistingOption>> findExistingOptions(Predicate<ConfigOptionsDocGenerator.OptionWithMetaInfo> predicate) throws IOException, ClassNotFoundException {
final Collection<ExistingOption> existingOptions = new ArrayList<>();

for (OptionsClassLocation location : LOCATIONS) {
processConfigOptions(System.getProperty("rootDir"), location.getModule(), location.getPackage(), DEFAULT_PATH_PREFIX, optionsClass -> {
List<ConfigOptionsDocGenerator.OptionWithMetaInfo> configOptions = extractConfigOptions(optionsClass);
for (ConfigOptionsDocGenerator.OptionWithMetaInfo option : configOptions) {
if (predicate.test(option)) {
ExistingOption newOption = toExistingOption(option, optionsClass)
ExistingOption duplicate = existingOptions.put(
newOption.key,
newOption);
if (duplicate != null) {
// multiple documented options have the same key
// we fail here outright as this is not a documentation-completeness problem
if (!(duplicate.description.equals(description))) {
throw new AssertionError("Ambiguous option " + key + " due to distinct descriptions.");
} else if (!duplicate.defaultValue.equals(defaultValue)) {
throw new AssertionError("Ambiguous option " + key + " due to distinct default values (" + defaultValue + " vs " + duplicate.defaultValue + ").");
}
}
}
}
});
processConfigOptions(System.getProperty("rootDir"), location.getModule(), location.getPackage(), DEFAULT_PATH_PREFIX, optionsClass ->
extractConfigOptions(optionsClass)
.stream()
.filter(predicate)
.map(optionWithMetaInfo -> toExistingOption(optionWithMetaInfo, optionsClass))
.forEach(existingOptions::add));
}

return existingOptions;
return existingOptions.stream().collect(Collectors.groupingBy(option -> option.key, Collectors.toList()));
}

private static ExistingOption toExistingOption(ConfigOptionsDocGenerator.OptionWithMetaInfo optionWithMetaInfo, Class<?> optionsClass) {
String key = optionWithMetaInfo.option.key();
Expand Down

0 comments on commit 7be292c

Please sign in to comment.