Skip to content

Commit 0c5b6e8

Browse files
JVerwolfJVerwolf
committed
iter
1 parent 50cde19 commit 0c5b6e8

File tree

7 files changed

+121
-31
lines changed

7 files changed

+121
-31
lines changed

‎build-tools-internal/src/integTest/groovy/org/elasticsearch/gradle/internal/transport/AbstractTransportVersionFuncTest.groovy‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,10 +55,18 @@ class AbstractTransportVersionFuncTest extends AbstractGradleFuncTest {
5555
}
5656

5757
def definedAndUsedTransportVersion(String name, String ids, String classname) {
58+
referencedTransportVersion(name, classname);
59+
namedTransportVersion(name, ids)
60+
}
61+
62+
def referencedTransportVersion(String name, String classname) {
5863
javaSource("myserver", "org.elasticsearch", classname, "", """
5964
static final TransportVersion usage = TransportVersion.fromName("${name}");
6065
""")
61-
namedTransportVersion(name, ids)
66+
}
67+
68+
def referencedTransportVersion(String name) {
69+
return referencedTransportVersion(name, "Test${name.capitalize()}")
6270
}
6371

6472
def latestTransportVersion(String branch, String name, String id) {

‎build-tools-internal/src/integTest/groovy/org/elasticsearch/gradle/internal/transport/TransportVersionGenerationFuncTest.groovy‎

Lines changed: 101 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -19,60 +19,139 @@ class TransportVersionGenerationFuncTest extends AbstractTransportVersionFuncTes
1919
result.task(":myserver:generateTransportVersionDefinition").outcome == TaskOutcome.SUCCESS
2020
}
2121

22-
def "A definition should be generated for an undefined reference"() {
22+
def "A definition should be generated for the given branches"(List<String> branches) {
2323
given:
24-
String tvName = "potato_tv"
24+
String tvName = "test_tv_patch_ids"
25+
referencedTransportVersion(tvName)
26+
List<LatestFile> latestBranchesToOriginalIds = readLatestFiles(branches)
27+
2528
when:
2629
def result = gradleRunner(
30+
":myserver:validateTransportVersionDefinitions",
2731
"generateTransportVersionDefinition",
2832
"--name=" + tvName,
29-
"--increment=1",
30-
"--branches=main"
33+
"--branches=" + branches.join(",")
34+
).build()
35+
36+
then:
37+
result.task(":myserver:generateTransportVersionDefinition").outcome == TaskOutcome.SUCCESS
38+
result.task(":myserver:validateTransportVersionDefinitions").outcome == TaskOutcome.SUCCESS
39+
validateDefinitionFile(tvName, latestBranchesToOriginalIds)
40+
41+
where:
42+
branches << [
43+
["9.2"],
44+
["9.2", "9.1"]
45+
]
46+
}
47+
48+
def "A definition should be generated when the name isn't specified"() {
49+
given:
50+
String tvName = "test_tv_patch_ids"
51+
referencedTransportVersion(tvName)
52+
List<String> branches = ["9.2"]
53+
List<LatestFile> latestBranchesToOriginalIds = readLatestFiles(branches)
54+
55+
when:
56+
def result = gradleRunner(
57+
":myserver:validateTransportVersionDefinitions",
58+
"generateTransportVersionDefinition",
59+
"--branches=" + branches.join(",")
3160
).build()
61+
3262
then:
3363
result.task(":myserver:generateTransportVersionDefinition").outcome == TaskOutcome.SUCCESS
34-
validateDefinitionFile(tvName, List.of("9.2"))
64+
result.task(":myserver:validateTransportVersionDefinitions").outcome == TaskOutcome.SUCCESS
65+
validateDefinitionFile(tvName, latestBranchesToOriginalIds)
66+
}
67+
68+
def "Should fail if branches are omitted and state should remain unaltered"() {
69+
when:
70+
def generateResult = gradleRunner("generateTransportVersionDefinition", "--name=no_branches").buildAndFail()
71+
def validateResult = gradleRunner("validateTransportVersionDefinitions").build()
72+
73+
then:
74+
generateResult.task(":myserver:generateTransportVersionDefinition").outcome == TaskOutcome.FAILED
75+
validateResult.task(":myserver:validateTransportVersionDefinitions").outcome == TaskOutcome.SUCCESS
76+
}
77+
78+
79+
/*
80+
TODO: Add tests that check that:
81+
- TVs added ontop of main in git, but are no longer referenced, are deleted
82+
- name without branches param should fail
83+
- a latest file without a corresponding definition file should be reverted to main
84+
- a merge conflict should be resolved, resulting in regeneration of the latest file.
85+
-
86+
*/
87+
88+
List<LatestFile> readLatestFiles(List<String> branches) {
89+
return branches.stream()
90+
.map { readLatestFile(it) }
91+
.toList()
3592
}
3693

37-
def "Missing required arguments should fail, state should remain unaltered"() {
38-
// TODO
94+
LatestFile readLatestFile(String branch) {
95+
String latestFileText = file(
96+
"myserver/src/main/resources/transport/latest/${branch}.csv"
97+
).text.strip()
98+
assert latestFileText.isEmpty() == false: "The latest file must not be empty"
99+
List<String> parts = latestFileText.split(",")
100+
assert parts.size() == 2: "The latest file must contain exactly two parts"
101+
return new LatestFile(branch, parts[0], Integer.valueOf(parts[1]))
39102
}
40103

41-
// TODO add a check for the increment, primary and patch
42-
void validateDefinitionFile(String definitionName, List<String> branches) {
104+
void validateDefinitionFile(String definitionName, List<LatestFile> originalLatestFiles, Integer primaryIncrement = 1000) {
43105
String filename = "myserver/src/main/resources/transport/definitions/named/" + definitionName + ".csv"
44106
assert file(filename).exists()
45107

46108
String definitionFileText = file(filename).text.strip()
47109
assert definitionFileText.isEmpty() == false: "The definition file must not be empty"
48110
List<Integer> definitionIDs = Arrays.stream(definitionFileText.split(",")).map(Integer::valueOf).toList()
49-
assert branches.size() == definitionIDs.size(): "The definition file does not have an id for each branch"
111+
assert originalLatestFiles.size() == definitionIDs.size(): "The definition file does not have an id for each latest file"
50112

51-
def latestNamesToIds = branches.stream()
113+
def latestNamesToIds = originalLatestFiles.stream()
114+
.map { it.branch }
52115
.map { file("myserver/src/main/resources/transport/latest/${it}.csv").text }
53116
.map { it.strip().split(",") }
54117
.map { new Tuple2(it.first(), Integer.valueOf(it.last())) }
55118
.toList()
56119

120+
57121
for (int i = 0; i < definitionIDs.size(); i++) {
58122
int definitionID = definitionIDs[i]
59123
String nameInLatest = latestNamesToIds[i].getV1()
60124
def idInLatest = latestNamesToIds[i].getV2()
61125

62126
assert definitionName.equals(nameInLatest): "The latest and definition names must match"
63127
assert definitionID == idInLatest: "The latest and definition ids must match"
128+
129+
int originalID = originalLatestFiles[i].id
130+
if (i == 0) {
131+
assert definitionID == originalID + primaryIncrement:
132+
"The primary version ID should be incremented by ${primaryIncrement} from the main branch latest file"
133+
} else {
134+
assert definitionID == originalID + 1:
135+
"The patch version ID should be incremented by 1 from the primary version latest file"
136+
}
137+
64138
}
65139
}
66140

67-
/*
68-
TODO: Add tests that check that:
69-
- TVs added ontop of main in git, but are no longer referenced, are deleted
70-
- name without branches param should fail
71-
- branches without name param should fail (+ other invalid combos)
72-
- multiple branches should create patch versions
73-
- a single branch value should create only a primary id
74-
- a latest file without a corresponding definition file should be reverted to main
75-
- a merge conflict should be resolved, resulting in regeneration of the latest file.
76-
-
77-
*/
141+
class LatestFile {
142+
String branch
143+
String name
144+
Integer id
145+
146+
LatestFile(String branch, String name, Integer id) {
147+
this.branch = branch
148+
this.name = name
149+
this.id = id
150+
}
151+
152+
@Override
153+
String toString() {
154+
return "LatestFile(branch=${branch}, name=${name}, id=${id})"
155+
}
156+
}
78157
}

‎build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/GenerateTransportVersionDefinitionTask.java‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@
1313
import org.gradle.api.Project;
1414
import org.gradle.api.file.ConfigurableFileCollection;
1515
import org.gradle.api.file.DirectoryProperty;
16-
import org.gradle.api.provider.ListProperty;
1716
import org.gradle.api.provider.Property;
1817
import org.gradle.api.tasks.Input;
1918
import org.gradle.api.tasks.InputDirectory;
@@ -33,6 +32,7 @@
3332
import java.nio.file.Files;
3433
import java.nio.file.Path;
3534
import java.util.ArrayList;
35+
import java.util.Arrays;
3636
import java.util.Collections;
3737
import java.util.HashSet;
3838
import java.util.List;
@@ -86,11 +86,11 @@ public abstract class GenerateTransportVersionDefinitionTask extends DefaultTask
8686
@Optional // In CI we find these from the github PR labels
8787
@Input
8888
@Option(option = "branches", description = "The branches for which to generate IDs, e.g. --branches=\"main,9.1\"")
89-
public abstract ListProperty<String> getBranches();
89+
public abstract Property<String> getBranches();
9090

9191
@Input
9292
@Optional
93-
@Option(option = "increment", description = "TBD")
93+
@Option(option = "increment", description = "The amount to increment the primary id for the main branch")
9494
public abstract Property<Integer> getPrimaryIncrement();
9595

9696
@Input
@@ -161,13 +161,13 @@ private List<TransportVersionId> updateLatestFiles(Path resourcesDir, String nam
161161
}
162162
}
163163

164+
Collections.sort(ids);
164165
return ids;
165166
}
166167

167168
private Set<String> getTargetReleaseBranches() {
168169
if (getBranches().get().isEmpty() == false) {
169-
return getBranches().get()
170-
.stream()
170+
return Arrays.stream(getBranches().get().split(","))
171171
.map(branch -> branch.equals("main") ? getMainReleaseBranch().get() : branch)
172172
.collect(Collectors.toSet());
173173
} else {

‎build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/TransportVersionId.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ public TransportVersionId bumpComponent(Component component) {
6262

6363
@Override
6464
public int compareTo(TransportVersionId o) {
65-
return Integer.compare(complete, o.complete);
65+
return Integer.compare(o.complete, complete);
6666
}
6767

6868
@Override

‎build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/TransportVersionResourcesPlugin.java‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,10 +73,13 @@ public void apply(Project project) {
7373
t.setDescription("(Re)generates a transport version definition file");
7474
t.getResourcesDirectory().set(getResourcesDirectory(project));
7575
t.getReferencesFiles().setFrom(tvReferencesConfig);
76+
t.getPrimaryIncrement().convention(1000);
7677
Version esVersion = VersionProperties.getElasticsearchVersion();
7778
t.getMainReleaseBranch().set(esVersion.getMajor() + "." + esVersion.getMinor());
7879
t.getResourcesProjectDir()
7980
.set(project.getRootProject().getProjectDir().toPath().relativize(project.getProjectDir().toPath()).toString());
8081
});
82+
83+
validateTask.configure(t -> t.mustRunAfter(generateDefinitionsTask));
8184
}
8285
}

‎build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/TransportVersionUtils.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ static TransportVersionDefinition readDefinitionFile(Path file) throws IOExcepti
3636
static void writeDefinitionFile(Path resourcesDir, TransportVersionDefinition definition) throws IOException {
3737
Files.writeString(
3838
resourcesDir.resolve("definitions/named").resolve(definition.name() + ".csv"),
39-
definition.ids().stream().map(id -> String.valueOf(id.complete())).collect(Collectors.joining(",")) + "\n",
39+
definition.ids().stream().map(Object::toString).collect(Collectors.joining(",")) + "\n",
4040
StandardCharsets.UTF_8
4141
);
4242
}

‎build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/ValidateTransportVersionResourcesTask.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -197,7 +197,7 @@ private void recordAndValidateDefinition(TransportVersionDefinition definition)
197197
if (definition.ids().isEmpty()) {
198198
throwDefinitionFailure(definition.name(), "does not contain any ids");
199199
}
200-
if (Comparators.isInOrder(definition.ids(), Comparator.reverseOrder()) == false) {
200+
if (Comparators.isInOrder(definition.ids(), Comparator.naturalOrder()) == false) {
201201
throwDefinitionFailure(definition.name(), "does not have ordered ids");
202202
}
203203
for (int ndx = 0; ndx < definition.ids().size(); ++ndx) {

0 commit comments

Comments
 (0)