Skip to content

Commit f6f191e

Browse files
pugnascotiapugnascotia
authored
Updates to changelog processing after docs redesign (#89463) (#89474)
1 parent 11d1053 commit f6f191e

File tree

6 files changed

+265
-35
lines changed

6 files changed

+265
-35
lines changed

‎build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/release/ValidateChangelogEntryTask.java

Lines changed: 58 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@
88

99
package org.elasticsearch.gradle.internal.release;
1010

11+
import com.google.common.annotations.VisibleForTesting;
12+
1113
import org.gradle.api.DefaultTask;
1214
import org.gradle.api.GradleException;
1315
import org.gradle.api.file.ConfigurableFileCollection;
@@ -30,6 +32,21 @@ public class ValidateChangelogEntryTask extends DefaultTask {
3032
private final ConfigurableFileCollection changelogs;
3133
private final ProjectLayout projectLayout;
3234

35+
public static final String TRIPLE_BACKTICK = "```";
36+
private static final String CODE_BLOCK_ERROR = """
37+
[%s] uses a triple-backtick in the [%s] section, but it must be
38+
formatted as a Asciidoc code block. For example:
39+
40+
[source,yaml]
41+
----
42+
{
43+
"metrics.time" : 10,
44+
"metrics.time.min" : 1,
45+
"metrics.time.max" : 500
46+
}
47+
----
48+
""";
49+
3350
@Inject
3451
public ValidateChangelogEntryTask(ObjectFactory objectFactory, ProjectLayout projectLayout) {
3552
this.changelogs = objectFactory.fileCollection();
@@ -43,37 +60,60 @@ public void executeTask() {
4360
.stream()
4461
.collect(Collectors.toMap(file -> rootDir.relativize(file.toURI()).toString(), ChangelogEntry::parse));
4562

63+
changelogs.forEach(ValidateChangelogEntryTask::validate);
64+
}
65+
66+
@VisibleForTesting
67+
static void validate(String path, ChangelogEntry entry) {
4668
// We don't try to find all such errors, because we expect them to be rare e.g. only
4769
// when a new file is added.
48-
changelogs.forEach((path, entry) -> {
49-
final String type = entry.getType();
50-
51-
if (type.equals("known-issue") == false && type.equals("security") == false) {
52-
if (entry.getPr() == null) {
53-
throw new GradleException(
54-
"[" + path + "] must provide a [pr] number (only 'known-issue' and " + "'security' entries can omit this"
55-
);
56-
}
57-
58-
if (entry.getArea() == null) {
59-
throw new GradleException(
60-
"[" + path + "] must provide an [area] (only 'known-issue' and " + "'security' entries can omit this"
61-
);
62-
}
70+
final String type = entry.getType();
71+
72+
if (type.equals("known-issue") == false && type.equals("security") == false) {
73+
if (entry.getPr() == null) {
74+
throw new GradleException(
75+
"[" + path + "] must provide a [pr] number (only 'known-issue' and 'security' entries can omit this"
76+
);
6377
}
6478

65-
if ((type.equals("breaking") || type.equals("breaking-java")) && entry.getBreaking() == null) {
79+
if (entry.getArea() == null) {
80+
throw new GradleException("[" + path + "] must provide an [area] (only 'known-issue' and 'security' entries can omit this");
81+
}
82+
}
83+
84+
if (type.equals("breaking") || type.equals("breaking-java")) {
85+
if (entry.getBreaking() == null) {
6686
throw new GradleException(
6787
"[" + path + "] has type [" + type + "] and must supply a [breaking] section with further information"
6888
);
6989
}
7090

71-
if (type.equals("deprecation") && entry.getDeprecation() == null) {
91+
if (entry.getBreaking().getDetails().contains(TRIPLE_BACKTICK)) {
92+
throw new GradleException(CODE_BLOCK_ERROR.formatted(path, "breaking.details"));
93+
}
94+
if (entry.getBreaking().getImpact().contains(TRIPLE_BACKTICK)) {
95+
throw new GradleException(CODE_BLOCK_ERROR.formatted(path, "breaking.impact"));
96+
}
97+
}
98+
99+
if (type.equals("deprecation")) {
100+
if (entry.getDeprecation() == null) {
72101
throw new GradleException(
73102
"[" + path + "] has type [deprecation] and must supply a [deprecation] section with further information"
74103
);
75104
}
76-
});
105+
106+
if (entry.getDeprecation().getDetails().contains(TRIPLE_BACKTICK)) {
107+
throw new GradleException(CODE_BLOCK_ERROR.formatted(path, "deprecation.details"));
108+
}
109+
if (entry.getDeprecation().getImpact().contains(TRIPLE_BACKTICK)) {
110+
throw new GradleException(CODE_BLOCK_ERROR.formatted(path, "deprecation.impact"));
111+
}
112+
}
113+
114+
if (entry.getHighlight() != null && entry.getHighlight().getBody().contains(TRIPLE_BACKTICK)) {
115+
throw new GradleException(CODE_BLOCK_ERROR.formatted(path, "highlight.body"));
116+
}
77117
}
78118

79119
@InputFiles

‎build-tools-internal/src/main/resources/templates/release-highlights.asciidoc

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,14 +32,18 @@ if (notableHighlights.isEmpty()) { %>
3232
<% for (highlight in notableHighlights) { %>
3333
[discrete]
3434
[[${ highlight.anchor }]]
35-
=== {es-pull}${highlight.pr}[${highlight.title}]
35+
=== ${highlight.title}
3636
${highlight.body.trim()}
37+
38+
{es-pull}${highlight.pr}[#${highlight.pr}]
3739
<% } %>
3840
// end::notable-highlights[]
3941
<% } %>
4042
<% for (highlight in nonNotableHighlights) { %>
4143
[discrete]
4244
[[${ highlight.anchor }]]
43-
=== {es-pull}${highlight.pr}[${highlight.title}]
45+
=== ${highlight.title}
4446
${highlight.body.trim()}
47+
48+
{es-pull}${highlight.pr}[#${highlight.pr}]
4549
<% } %>

‎build-tools-internal/src/test/java/org/elasticsearch/gradle/internal/release/ReleaseHighlightsGeneratorTest.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -60,11 +60,11 @@ public void generateFile_rendersCorrectMarkup() throws Exception {
6060
}
6161

6262
private List<ChangelogEntry> getEntries() {
63-
ChangelogEntry entry1 = makeChangelogEntry(1, true);
64-
ChangelogEntry entry2 = makeChangelogEntry(2, true);
65-
ChangelogEntry entry3 = makeChangelogEntry(3, false);
63+
ChangelogEntry entry123 = makeChangelogEntry(123, true);
64+
ChangelogEntry entry456 = makeChangelogEntry(456, true);
65+
ChangelogEntry entry789 = makeChangelogEntry(789, false);
6666
// Return unordered list, to test correct re-ordering
67-
return List.of(entry2, entry1, entry3);
67+
return List.of(entry456, entry123, entry789);
6868
}
6969

7070
private ChangelogEntry makeChangelogEntry(int pr, boolean notable) {

‎build-tools-internal/src/test/java/org/elasticsearch/gradle/internal/release/ValidateChangelogEntryTaskTest.java

Lines changed: 179 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,179 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the Elastic License
4+
* 2.0 and the Server Side Public License, v 1; you may not use this file except
5+
* in compliance with, at your election, the Elastic License 2.0 or the Server
6+
* Side Public License, v 1.
7+
*/
8+
9+
package org.elasticsearch.gradle.internal.release;
10+
11+
import org.gradle.api.GradleException;
12+
import org.hamcrest.Matchers;
13+
import org.junit.jupiter.api.Test;
14+
15+
import java.util.stream.Stream;
16+
17+
import static org.hamcrest.MatcherAssert.assertThat;
18+
import static org.hamcrest.Matchers.containsString;
19+
import static org.hamcrest.Matchers.endsWith;
20+
21+
class ValidateChangelogEntryTaskTest {
22+
23+
@Test
24+
void test_prNumber_isRequired() {
25+
ChangelogEntry changelog = new ChangelogEntry();
26+
changelog.setType("enhancement");
27+
28+
final String message = doValidate(changelog);
29+
30+
assertThat(message, endsWith("must provide a [pr] number (only 'known-issue' and 'security' entries can omit this"));
31+
}
32+
33+
@Test
34+
void test_prNumber_notRequired() {
35+
Stream.of("known-issue", "security").forEach(type -> {
36+
ChangelogEntry changelog = new ChangelogEntry();
37+
changelog.setType(type);
38+
39+
// Should not throw an exception!
40+
ValidateChangelogEntryTask.validate("", changelog);
41+
});
42+
}
43+
44+
@Test
45+
void test_area_isRequired() {
46+
final ChangelogEntry changelog = new ChangelogEntry();
47+
changelog.setType("enhancement");
48+
changelog.setPr(123);
49+
50+
final String message = doValidate(changelog);
51+
52+
assertThat(message, endsWith("must provide an [area] (only 'known-issue' and 'security' entries can omit this"));
53+
}
54+
55+
@Test
56+
void test_breaking_requiresBreakingSection() {
57+
Stream.of("breaking", "breaking-java").forEach(type -> {
58+
final ChangelogEntry changelog = buildChangelog(type);
59+
60+
final String message = doValidate(changelog);
61+
62+
assertThat(message, endsWith("has type [" + type + "] and must supply a [breaking] section with further information"));
63+
});
64+
}
65+
66+
@Test
67+
void test_breaking_rejectsTripleBackticksInDetails() {
68+
Stream.of("breaking", "breaking-java").forEach(type -> {
69+
final ChangelogEntry.Breaking breaking = new ChangelogEntry.Breaking();
70+
breaking.setDetails("""
71+
Some waffle.
72+
```
73+
I AM CODE!
74+
```
75+
""");
76+
77+
final ChangelogEntry changelog = buildChangelog(type);
78+
changelog.setBreaking(breaking);
79+
80+
final String message = doValidate(changelog);
81+
82+
assertThat(message, containsString("uses a triple-backtick in the [breaking.details] section"));
83+
});
84+
}
85+
86+
@Test
87+
void test_breaking_rejectsTripleBackticksInImpact() {
88+
Stream.of("breaking", "breaking-java").forEach(type -> {
89+
final ChangelogEntry.Breaking breaking = new ChangelogEntry.Breaking();
90+
breaking.setDetails("Waffle waffle");
91+
breaking.setImpact("""
92+
More waffle.
93+
```
94+
THERE ARE WEASEL RAKING THROUGH MY GARBAGE!
95+
```
96+
""");
97+
98+
final ChangelogEntry changelog = buildChangelog(type);
99+
changelog.setBreaking(breaking);
100+
101+
final String message = doValidate(changelog);
102+
103+
assertThat(message, containsString("uses a triple-backtick in the [breaking.impact] section"));
104+
});
105+
}
106+
107+
@Test
108+
void test_deprecation_rejectsTripleBackticksInImpact() {
109+
final ChangelogEntry.Deprecation deprecation = new ChangelogEntry.Deprecation();
110+
deprecation.setDetails("Waffle waffle");
111+
deprecation.setImpact("""
112+
More waffle.
113+
```
114+
THERE ARE WEASEL RAKING THROUGH MY GARBAGE!
115+
```
116+
""");
117+
118+
final ChangelogEntry changelog = buildChangelog("deprecation");
119+
changelog.setDeprecation(deprecation);
120+
121+
final String message = doValidate(changelog);
122+
123+
assertThat(message, containsString("uses a triple-backtick in the [deprecation.impact] section"));
124+
}
125+
126+
@Test
127+
void test_deprecation_rejectsTripleBackticksInDetails() {
128+
final ChangelogEntry.Deprecation deprecation = new ChangelogEntry.Deprecation();
129+
deprecation.setDetails("""
130+
Some waffle.
131+
```
132+
I AM CODE!
133+
```
134+
""");
135+
136+
final ChangelogEntry changelog = buildChangelog("deprecation");
137+
changelog.setDeprecation(deprecation);
138+
139+
final String message = doValidate(changelog);
140+
141+
assertThat(message, containsString("uses a triple-backtick in the [deprecation.details] section"));
142+
}
143+
144+
@Test
145+
void test_highlight_rejectsTripleBackticksInBody() {
146+
final ChangelogEntry.Highlight highlight = new ChangelogEntry.Highlight();
147+
highlight.setBody("""
148+
Some waffle.
149+
```
150+
I AM CODE!
151+
```
152+
""");
153+
154+
final ChangelogEntry changelog = buildChangelog("enhancement");
155+
changelog.setHighlight(highlight);
156+
157+
final String message = doValidate(changelog);
158+
159+
assertThat(message, containsString("uses a triple-backtick in the [highlight.body] section"));
160+
}
161+
162+
private static ChangelogEntry buildChangelog(String type) {
163+
final ChangelogEntry changelog = new ChangelogEntry();
164+
changelog.setType(type);
165+
changelog.setPr(123);
166+
changelog.setArea("Infra/Core");
167+
return changelog;
168+
}
169+
170+
private String doValidate(ChangelogEntry entry) {
171+
try {
172+
ValidateChangelogEntryTask.validate("docs/123.yaml", entry);
173+
throw new AssertionError("No exception thrown!");
174+
} catch (Exception e) {
175+
assertThat(e, Matchers.instanceOf(GradleException.class));
176+
return e.getMessage();
177+
}
178+
}
179+
}

‎build-tools-internal/src/test/resources/org/elasticsearch/gradle/internal/release/ReleaseHighlightsGeneratorTest.generateFile.asciidoc

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -20,20 +20,26 @@ Other versions:
2020
// tag::notable-highlights[]
2121

2222
[discrete]
23-
[[notable_release_highlight_number_1]]
24-
=== {es-pull}1[Notable release highlight number 1]
25-
Notable release body number 1
23+
[[notable_release_highlight_number_123]]
24+
=== Notable release highlight number 123
25+
Notable release body number 123
26+
27+
{es-pull}123[#123]
2628

2729
[discrete]
28-
[[notable_release_highlight_number_2]]
29-
=== {es-pull}2[Notable release highlight number 2]
30-
Notable release body number 2
30+
[[notable_release_highlight_number_456]]
31+
=== Notable release highlight number 456
32+
Notable release body number 456
33+
34+
{es-pull}456[#456]
3135

3236
// end::notable-highlights[]
3337

3438

3539
[discrete]
36-
[[notable_release_highlight_number_3]]
37-
=== {es-pull}3[Notable release highlight number 3]
38-
Notable release body number 3
40+
[[notable_release_highlight_number_789]]
41+
=== Notable release highlight number 789
42+
Notable release body number 789
43+
44+
{es-pull}789[#789]
3945

‎docs/changelog/83345.yaml

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,8 @@ highlight:
1414
As an example, the following ILM policy would roll an index over if it is at least 7 days old or
1515
at least 100 gigabytes, but only as long as the index is not empty.
1616
17-
```
17+
[source,console]
18+
----
1819
PUT _ilm/policy/my_policy
1920
{
2021
"policy": {
@@ -31,5 +32,5 @@ highlight:
3132
}
3233
}
3334
}
34-
```
35+
----
3536
notable: true

0 commit comments

Comments
 (0)