Skip to content

Commit ebbd69b

Browse files
committed
Allow selecting exactly n patches during roulette
The package local implementation of the selection strategy works well for large updates as single packages offer a great "unit" of patches to work on. When updating paper to specific snapshots on a weekly basis, generally the amount of non-applicable patches is a lot lower. Being restricted to a package in this case is bad dx, as most time is spent selecting more patches and finishing them, when 10 patches would be a much better "unit" of patches than a package with a single failed patch.
1 parent 1a04379 commit ebbd69b

File tree

4 files changed

+150
-5
lines changed

4 files changed

+150
-5
lines changed

buildSrc/src/main/kotlin/config-kotlin.gradle.kts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,7 @@ testing {
6868
useKotlinTest(embeddedKotlinVersion)
6969
dependencies {
7070
implementation("org.junit.jupiter:junit-jupiter-engine:5.10.1")
71+
implementation("org.junit.jupiter:junit-jupiter-params:5.10.1")
7172
implementation("org.junit.platform:junit-platform-launcher:1.10.1")
7273
}
7374

paperweight-core/src/main/kotlin/io/papermc/paperweight/core/tasks/patchroulette/AbstractPatchRouletteTask.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ abstract class AbstractPatchRouletteTask : BaseTask() {
5454

5555
override fun init() {
5656
super.init()
57-
endpoint.convention("https://patch-roulette.papermc.io/api")
57+
endpoint.convention("http://localhost:8080/api")
5858
authToken.convention(providers.gradleProperty("paperweight.patch-roulette-token"))
5959
doNotTrackState("Run when requested")
6060
}

paperweight-core/src/main/kotlin/io/papermc/paperweight/core/tasks/patchroulette/PatchRouletteApply.kt

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ abstract class PatchRouletteApply : AbstractPatchRouletteTask() {
8787
}
8888

8989
var tries = 5
90-
var patches = listOf<Path>()
90+
var patches: List<Path>
9191
val patchSelectionStrategy = patchSelectionStrategy
9292
.map { PatchSelectionStrategy.parse(it) }
9393
.getOrElse(PatchSelectionStrategy.NumericInPackage(5))
@@ -166,11 +166,25 @@ abstract class PatchRouletteApply : AbstractPatchRouletteTask() {
166166
data class Config(val skip: List<Path>, val suggestedPackage: Path?, val currentPatches: List<Path>)
167167

168168
sealed interface PatchSelectionStrategy {
169-
data class NumericInPackage(val count: Int) : PatchSelectionStrategy {
169+
data class NumericInPackage(val count: Int, val enforceCount: Boolean = false) : PatchSelectionStrategy {
170170
override fun select(config: Config, available: List<Path>): Pair<Config, List<Path>> {
171171
if (config.suggestedPackage != null) {
172172
val possiblePatches = available.filter { it.parent.equals(config.suggestedPackage) }.take(count)
173-
if (possiblePatches.isNotEmpty()) return config to possiblePatches
173+
if (possiblePatches.isNotEmpty()) {
174+
if (!enforceCount) return config to possiblePatches
175+
// The patches we found satisfy the count param or the entire available set simply does not offer enough patches.
176+
if (possiblePatches.size >= count || possiblePatches.size == available.size) return config to possiblePatches
177+
178+
// The patches found in the package do not satisfy the requested count *and* the strategy was configured to enforce the
179+
// count. Re-select from a new package and different patch set, add them to our already fetched patches and update the
180+
// config, as the last suggested package is the one to suggest in potentially next runs.
181+
val additionalPatches = select(
182+
config.copy(suggestedPackage = null),
183+
available.filter { !possiblePatches.contains(it) }
184+
)
185+
186+
return additionalPatches.first to possiblePatches + additionalPatches.second
187+
}
174188
}
175189

176190
return select(config.copy(suggestedPackage = available.first().parent), available)
@@ -182,7 +196,10 @@ abstract class PatchRouletteApply : AbstractPatchRouletteTask() {
182196
companion object {
183197
fun parse(input: String): PatchSelectionStrategy {
184198
try {
185-
return NumericInPackage(input.toInt())
199+
return when {
200+
input.endsWith("!") -> NumericInPackage(input.substring(0, input.length - 1).toInt(), true)
201+
else -> NumericInPackage(input.toInt())
202+
}
186203
} catch (e: Exception) {
187204
throw PaperweightException("Failed to parse patch selection strategy $input", e)
188205
}
Lines changed: 127 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,127 @@
1+
/*
2+
* paperweight is a Gradle plugin for the PaperMC project.
3+
*
4+
* Copyright (c) 2023 Kyle Wood (DenWav)
5+
* Contributors
6+
*
7+
* This library is free software; you can redistribute it and/or
8+
* modify it under the terms of the GNU Lesser General Public
9+
* License as published by the Free Software Foundation;
10+
* version 2.1 only, no later versions.
11+
*
12+
* This library is distributed in the hope that it will be useful,
13+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
14+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
15+
* Lesser General Public License for more details.
16+
*
17+
* You should have received a copy of the GNU Lesser General Public
18+
* License along with this library; if not, write to the Free Software
19+
* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301
20+
* USA
21+
*/
22+
23+
package io.papermc.paperweight.core.tasks.patchroulette
24+
25+
import kotlin.io.path.*
26+
import kotlin.test.Test
27+
import kotlin.test.assertEquals
28+
import org.junit.jupiter.api.Assertions.assertFalse
29+
import org.junit.jupiter.api.Assertions.assertTrue
30+
import org.junit.jupiter.params.ParameterizedTest
31+
import org.junit.jupiter.params.provider.Arguments
32+
import org.junit.jupiter.params.provider.MethodSource
33+
34+
class PatchRouletteApplyTest {
35+
36+
@Test
37+
fun `test patch strategy parsing non enforced`() {
38+
val strategy = PatchRouletteApply.PatchSelectionStrategy.parse("10")
39+
when (strategy) {
40+
is PatchRouletteApply.PatchSelectionStrategy.NumericInPackage -> {
41+
assertEquals(10, strategy.count)
42+
assertFalse(strategy.enforceCount)
43+
}
44+
}
45+
}
46+
47+
@Test
48+
fun `test patch strategy parsing enforced`() {
49+
val strategy = PatchRouletteApply.PatchSelectionStrategy.parse("20!")
50+
when (strategy) {
51+
is PatchRouletteApply.PatchSelectionStrategy.NumericInPackage -> {
52+
assertEquals(20, strategy.count)
53+
assertTrue(strategy.enforceCount)
54+
}
55+
}
56+
}
57+
58+
@ParameterizedTest
59+
@MethodSource("testPatchSelectionSource")
60+
fun `test patch selection`(
61+
strategy: PatchRouletteApply.PatchSelectionStrategy,
62+
allPatches: List<String>,
63+
expectedPatchBatches: List<List<String>>
64+
) {
65+
var config = PatchRouletteApply.Config(listOf(), null, listOf())
66+
var availablePatches = allPatches.map { Path(it) }
67+
for (batch in expectedPatchBatches) {
68+
val selectionResult = strategy.select(config, availablePatches)
69+
assertEquals(batch.map { Path(it) }, selectionResult.second)
70+
71+
config = selectionResult.first
72+
availablePatches = availablePatches - selectionResult.second
73+
}
74+
75+
assertTrue(availablePatches.isEmpty(), "Patches remained after exhausting expected batches")
76+
}
77+
78+
companion object {
79+
@JvmStatic
80+
fun testPatchSelectionSource(): Collection<Arguments> = listOf(
81+
Arguments.of(
82+
PatchRouletteApply.PatchSelectionStrategy.NumericInPackage(2),
83+
mockAvailablePatches(),
84+
listOf(
85+
listOf("io/papermc/paper/block/Block.java", "io/papermc/paper/block/BlockData.java"),
86+
listOf("io/papermc/paper/block/BlockState.java"),
87+
listOf("io/papermc/paper/entity/Entity.java")
88+
)
89+
),
90+
Arguments.of(
91+
PatchRouletteApply.PatchSelectionStrategy.NumericInPackage(5),
92+
mockAvailablePatches(),
93+
listOf(
94+
listOf("io/papermc/paper/block/Block.java", "io/papermc/paper/block/BlockData.java", "io/papermc/paper/block/BlockState.java"),
95+
listOf("io/papermc/paper/entity/Entity.java")
96+
)
97+
),
98+
Arguments.of(
99+
PatchRouletteApply.PatchSelectionStrategy.NumericInPackage(2, true),
100+
mockAvailablePatches(),
101+
listOf(
102+
listOf("io/papermc/paper/block/Block.java", "io/papermc/paper/block/BlockData.java"),
103+
listOf("io/papermc/paper/block/BlockState.java", "io/papermc/paper/entity/Entity.java"),
104+
)
105+
),
106+
Arguments.of(
107+
PatchRouletteApply.PatchSelectionStrategy.NumericInPackage(5, true),
108+
mockAvailablePatches(),
109+
listOf(
110+
listOf(
111+
"io/papermc/paper/block/Block.java",
112+
"io/papermc/paper/block/BlockData.java",
113+
"io/papermc/paper/block/BlockState.java",
114+
"io/papermc/paper/entity/Entity.java"
115+
),
116+
)
117+
),
118+
)
119+
120+
fun mockAvailablePatches() = listOf(
121+
"io/papermc/paper/block/Block.java",
122+
"io/papermc/paper/block/BlockData.java",
123+
"io/papermc/paper/block/BlockState.java",
124+
"io/papermc/paper/entity/Entity.java"
125+
)
126+
}
127+
}

0 commit comments

Comments
 (0)