Skip to content

Commit

Permalink
Rework how we render groovy templates for docs (#77125)
Browse files Browse the repository at this point in the history
On Windows, rendered Groovy templates contain carriage returns, which
breaks the unit tests and results in them sneaking into the output. Fix
this by rendering into a string and removing the carriage returns.
  • Loading branch information
pugnascotia committed Sep 1, 2021
1 parent e5ba189 commit aab02be
Show file tree
Hide file tree
Showing 11 changed files with 74 additions and 76 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -8,16 +8,13 @@

package org.elasticsearch.gradle.internal.release;

import groovy.text.SimpleTemplateEngine;

import com.google.common.annotations.VisibleForTesting;

import org.elasticsearch.gradle.VersionProperties;

import java.io.File;
import java.io.FileWriter;
import java.io.IOException;
import java.io.Writer;
import java.nio.file.Files;
import java.util.HashMap;
import java.util.List;
Expand All @@ -36,18 +33,14 @@ public class BreakingChangesGenerator {

static void update(File templateFile, File outputFile, List<ChangelogEntry> entries) throws IOException {
try (FileWriter output = new FileWriter(outputFile)) {
generateFile(
QualifiedVersion.of(VersionProperties.getElasticsearch()),
Files.readString(templateFile.toPath()),
output,
entries
output.write(
generateFile(QualifiedVersion.of(VersionProperties.getElasticsearch()), Files.readString(templateFile.toPath()), entries)
);
}
}

@VisibleForTesting
static void generateFile(QualifiedVersion version, String template, Writer outputWriter, List<ChangelogEntry> entries)
throws IOException {
static String generateFile(QualifiedVersion version, String template, List<ChangelogEntry> entries) throws IOException {

final Map<Boolean, Map<String, List<ChangelogEntry.Breaking>>> breakingChangesByNotabilityByArea = entries.stream()
.map(ChangelogEntry::getBreaking)
Expand Down Expand Up @@ -75,11 +68,6 @@ static void generateFile(QualifiedVersion version, String template, Writer outpu
bindings.put("nextMajor", (version.getMajor() + 1) + ".0");
bindings.put("version", version);

try {
final SimpleTemplateEngine engine = new SimpleTemplateEngine();
engine.createTemplate(template).make(bindings).writeTo(outputWriter);
} catch (ClassNotFoundException e) {
throw new RuntimeException(e);
}
return TemplateUtils.render(template, bindings);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,16 +8,13 @@

package org.elasticsearch.gradle.internal.release;

import groovy.text.SimpleTemplateEngine;

import com.google.common.annotations.VisibleForTesting;

import org.elasticsearch.gradle.VersionProperties;

import java.io.File;
import java.io.FileWriter;
import java.io.IOException;
import java.io.Writer;
import java.nio.file.Files;
import java.util.ArrayList;
import java.util.HashMap;
Expand All @@ -32,13 +29,14 @@
public class ReleaseHighlightsGenerator {
static void update(File templateFile, File outputFile, List<ChangelogEntry> entries) throws IOException {
try (FileWriter output = new FileWriter(outputFile)) {
generateFile(QualifiedVersion.of(VersionProperties.getElasticsearch()), Files.readString(templateFile.toPath()), entries, output);
output.write(
generateFile(QualifiedVersion.of(VersionProperties.getElasticsearch()), Files.readString(templateFile.toPath()), entries)
);
}
}

@VisibleForTesting
static void generateFile(QualifiedVersion version, String templateFile, List<ChangelogEntry> entries, Writer outputWriter)
throws IOException {
static String generateFile(QualifiedVersion version, String template, List<ChangelogEntry> entries) throws IOException {
final List<String> priorVersions = new ArrayList<>();

if (version.getMinor() > 0) {
Expand Down Expand Up @@ -66,11 +64,6 @@ static void generateFile(QualifiedVersion version, String templateFile, List<Cha
bindings.put("notableHighlights", notableHighlights);
bindings.put("nonNotableHighlights", nonNotableHighlights);

try {
final SimpleTemplateEngine engine = new SimpleTemplateEngine();
engine.createTemplate(templateFile).make(bindings).writeTo(outputWriter);
} catch (ClassNotFoundException e) {
throw new RuntimeException(e);
}
return TemplateUtils.render(template, bindings);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,24 +8,18 @@

package org.elasticsearch.gradle.internal.release;

import groovy.text.SimpleTemplateEngine;

import com.google.common.annotations.VisibleForTesting;

import org.gradle.api.GradleException;

import java.io.File;
import java.io.FileWriter;
import java.io.IOException;
import java.io.Writer;
import java.nio.file.Files;
import java.util.Comparator;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.TreeMap;
import java.util.stream.Collectors;

import static java.util.Comparator.comparing;
import static java.util.stream.Collectors.groupingBy;
Expand Down Expand Up @@ -57,25 +51,19 @@ static void update(File templateFile, File outputFile, Map<QualifiedVersion, Set
final String templateString = Files.readString(templateFile.toPath());

try (FileWriter output = new FileWriter(outputFile)) {
generateFile(templateString, changelogs, output);
output.write(generateFile(templateString, changelogs));
}
}

@VisibleForTesting
static void generateFile(String template, Map<QualifiedVersion, Set<ChangelogEntry>> changelogs, Writer outputWriter)
throws IOException {
static String generateFile(String template, Map<QualifiedVersion, Set<ChangelogEntry>> changelogs) throws IOException {
final var changelogsByVersionByTypeByArea = buildChangelogBreakdown(changelogs);

final Map<String, Object> bindings = new HashMap<>();
bindings.put("changelogsByVersionByTypeByArea", changelogsByVersionByTypeByArea);
bindings.put("TYPE_LABELS", TYPE_LABELS);

try {
final SimpleTemplateEngine engine = new SimpleTemplateEngine();
engine.createTemplate(template).make(bindings).writeTo(outputWriter);
} catch (ClassNotFoundException e) {
throw new GradleException("Failed to generate file from template", e);
}
return TemplateUtils.render(template, bindings);
}

private static Map<QualifiedVersion, Map<String, Map<String, List<ChangelogEntry>>>> buildChangelogBreakdown(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,11 @@

package org.elasticsearch.gradle.internal.release;

import groovy.text.SimpleTemplateEngine;

import com.google.common.annotations.VisibleForTesting;

import java.io.File;
import java.io.FileWriter;
import java.io.IOException;
import java.io.Writer;
import java.nio.file.Files;
import java.util.HashMap;
import java.util.List;
Expand All @@ -34,12 +31,12 @@ public class ReleaseNotesIndexGenerator {

static void update(Set<QualifiedVersion> versions, File indexTemplate, File indexFile) throws IOException {
try (FileWriter indexFileWriter = new FileWriter(indexFile)) {
generateFile(versions, Files.readString(indexTemplate.toPath()), indexFileWriter);
indexFileWriter.write(generateFile(versions, Files.readString(indexTemplate.toPath())));
}
}

@VisibleForTesting
static void generateFile(Set<QualifiedVersion> versionsSet, String indexTemplate, Writer outputWriter) throws IOException {
static String generateFile(Set<QualifiedVersion> versionsSet, String template) throws IOException {
final Set<QualifiedVersion> versions = new TreeSet<>(reverseOrder());

// For the purpose of generating the index, snapshot versions are the same as released versions. Prerelease versions are not.
Expand All @@ -54,11 +51,6 @@ static void generateFile(Set<QualifiedVersion> versionsSet, String indexTemplate
bindings.put("versions", versions);
bindings.put("includeVersions", includeVersions);

try {
final SimpleTemplateEngine engine = new SimpleTemplateEngine();
engine.createTemplate(indexTemplate).make(bindings).writeTo(outputWriter);
} catch (ClassNotFoundException e) {
throw new RuntimeException(e);
}
return TemplateUtils.render(template, bindings);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/

package org.elasticsearch.gradle.internal.release;

import groovy.text.SimpleTemplateEngine;

import java.io.IOException;
import java.io.StringWriter;
import java.util.Map;

/**
* Methods for working with Groovy templates.
*/
public class TemplateUtils {

/**
* Applies {@code bindings} to {@code template}, then removes all carriage returns from
* the result.
*
* @param template a Groovy template
* @param bindings parameters for the template
* @return the rendered template
*/
public static String render(String template, Map<String, Object> bindings) throws IOException {
final StringWriter writer = new StringWriter();

try {
final SimpleTemplateEngine engine = new SimpleTemplateEngine();
engine.createTemplate(template).make(bindings).writeTo(writer);
} catch (ClassNotFoundException e) {
throw new RuntimeException(e);
}

return writer.toString().replace("\\r", "");
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,10 @@
import org.gradle.api.tasks.InputFiles;
import org.gradle.api.tasks.TaskAction;

import javax.inject.Inject;
import java.net.URI;
import java.util.Map;
import java.util.stream.Collectors;
import javax.inject.Inject;

/**
* Performs additional checks on changelog files, beyond whether they conform to the schema.
Expand Down Expand Up @@ -49,13 +49,15 @@ public void executeTask() {

if (type.equals("known-issue") == false && type.equals("security") == false) {
if (entry.getPr() == null) {
throw new GradleException("[" + path + "] must provide a [pr] number (only 'known-issue' and " +
"'security' entries can omit this");
throw new GradleException(
"[" + path + "] must provide a [pr] number (only 'known-issue' and " + "'security' entries can omit this"
);
}

if (entry.getArea() == null) {
throw new GradleException("[" + path + "] must provide an [area] (only 'known-issue' and " +
"'security' entries can omit this");
throw new GradleException(
"[" + path + "] must provide an [area] (only 'known-issue' and " + "'security' entries can omit this"
);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@

import org.junit.Test;

import java.io.StringWriter;
import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import java.nio.file.Paths;
Expand All @@ -32,12 +31,10 @@ public void generateFile_rendersCorrectMarkup() throws Exception {
final String expectedOutput = getResource(
"/org/elasticsearch/gradle/internal/release/BreakingChangesGeneratorTest.generateFile.asciidoc"
);
final StringWriter writer = new StringWriter();
final List<ChangelogEntry> entries = getEntries();

// when:
BreakingChangesGenerator.generateFile(QualifiedVersion.of("8.4.0-SNAPSHOT"), template, writer, entries);
final String actualOutput = writer.toString();
final String actualOutput = BreakingChangesGenerator.generateFile(QualifiedVersion.of("8.4.0-SNAPSHOT"), template, entries);

// then:
assertThat(actualOutput, equalTo(expectedOutput));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,11 @@ public void partitionFiles_withPrerelease_correctlyGroupsByPrereleaseVersion() {
);

// when:
Map<QualifiedVersion, Set<File>> partitionedFiles = GenerateReleaseNotesTask.partitionFilesByVersion(gitWrapper, "8.0.0-beta1", allFiles);
Map<QualifiedVersion, Set<File>> partitionedFiles = GenerateReleaseNotesTask.partitionFilesByVersion(
gitWrapper,
"8.0.0-beta1",
allFiles
);

// then:
verify(gitWrapper).listVersions("v8.0*");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@

import org.junit.Test;

import java.io.StringWriter;
import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import java.nio.file.Paths;
Expand All @@ -32,12 +31,10 @@ public void generateFile_rendersCorrectMarkup() throws Exception {
final String expectedOutput = getResource(
"/org/elasticsearch/gradle/internal/release/ReleaseHighlightsGeneratorTest.generateFile.asciidoc"
);
final StringWriter writer = new StringWriter();
final List<ChangelogEntry> entries = getEntries();

// when:
ReleaseHighlightsGenerator.generateFile(QualifiedVersion.of("8.4.0-SNAPSHOT"), template, entries, writer);
final String actualOutput = writer.toString();
final String actualOutput = ReleaseHighlightsGenerator.generateFile(QualifiedVersion.of("8.4.0-SNAPSHOT"), template, entries);

// then:
assertThat(actualOutput, equalTo(expectedOutput));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@

import org.junit.Test;

import java.io.StringWriter;
import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import java.nio.file.Paths;
Expand All @@ -37,12 +36,10 @@ public void generateFile_rendersCorrectMarkup() throws Exception {
final String expectedOutput = getResource(
"/org/elasticsearch/gradle/internal/release/ReleaseNotesGeneratorTest.generateFile.asciidoc"
);
final StringWriter writer = new StringWriter();
final Map<QualifiedVersion, Set<ChangelogEntry>> entries = getEntries();

// when:
ReleaseNotesGenerator.generateFile(template, entries, writer);
final String actualOutput = writer.toString();
final String actualOutput = ReleaseNotesGenerator.generateFile(template, entries);

// then:
assertThat(actualOutput, equalTo(expectedOutput));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@

import org.junit.Test;

import java.io.StringWriter;
import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import java.nio.file.Paths;
Expand Down Expand Up @@ -46,11 +45,9 @@ public void generateFile_rendersCorrectMarkup() throws Exception {
final String expectedOutput = getResource(
"/org/elasticsearch/gradle/internal/release/ReleaseNotesIndexGeneratorTest.generateFile.asciidoc"
);
final StringWriter writer = new StringWriter();

// when:
ReleaseNotesIndexGenerator.generateFile(versions, template, writer);
final String actualOutput = writer.toString();
final String actualOutput = ReleaseNotesIndexGenerator.generateFile(versions, template);

// then:
assertThat(actualOutput, equalTo(expectedOutput));
Expand Down

0 comments on commit aab02be

Please sign in to comment.