Skip to content

Commit 87633a9

Browse files
marchermansmarchermans
authored
[Feature]: Improved testing (#194)
1 parent e8c6d92 commit 87633a9

File tree

29 files changed

+330
-139
lines changed

29 files changed

+330
-139
lines changed

‎.github/gradle/gradle.gradle‎

Lines changed: 95 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,41 +1,119 @@
1+
import groovy.json.JsonBuilder
2+
import groovy.transform.CompileStatic
3+
14
if (!project.hasProperty('githubCiTesting') && System.env['GITHUB_OUTPUT'] == null) {
25
// If the property is not set and we are not running in the CI environment then we will do nothing
36
return
47
}
58

6-
static def outputUnitTest(Project project, Task test) {
7-
project.getLogger().lifecycle("<< TEST >>${test.path}")
9+
static Map<String, String> removeCommonPrefix(List<String> list) {
10+
if (list.size() == 0) {
11+
return [:]
12+
}
13+
14+
def result = new HashMap<String, String>()
15+
16+
if (list.unique(false).size() == 1) {
17+
def sections = list[0].split("\\.")
18+
result.put(list[0], sections.last())
19+
return result
20+
}
21+
22+
Collections.sort(list, { String a, String b -> a.length() - b.length() })
23+
def max = list[0].length()
24+
def prefix = ""
25+
26+
for(int index = 0; index < max; index++) {
27+
def currentChar = list[0][index]
28+
for (String item : list) {
29+
if (item[index] != currentChar) {
30+
for (final def entry in list) {
31+
result.put(entry, entry.substring(prefix.length()))
32+
}
33+
return result
34+
}
35+
}
36+
37+
prefix += currentChar
38+
}
39+
40+
throw new IllegalArgumentException("Could not remove common prefix: ${list}")
841
}
942

10-
static def outputClassTest(Project project, Task test, String className) {
11-
project.getLogger().lifecycle("<< TEST >>${test.path} --tests \"${className}\"")
43+
static String createDisplayNameSuffix(String path, String filter) {
44+
//The path starts with a : and then the project name:
45+
path = path.substring(path.indexOf(":") + 1)
46+
47+
//Now split the path into parts
48+
def parts = path.split(":")
49+
def capitalizedParts = parts.collect { it.capitalize() }
50+
51+
//Combine the parts with ->
52+
path = capitalizedParts.join(" -> ")
53+
54+
if (filter === null) {
55+
return path
56+
}
57+
58+
//Next the filter has its common prefix stripped, is however still in domain name form
59+
def filterParts = filter.split("\\.")
60+
def capitalizedFilterParts = filterParts.collect { it.capitalize() }
61+
filter = capitalizedFilterParts.join("/")
62+
63+
return "${path} (${filter})"
64+
}
65+
66+
static List<TestRun> createTestRuns(Task it, List<String> testClasses) {
67+
def testRuns = []
68+
if (testClasses.size() == 0) {
69+
testRuns.add(new TestRun(displayName: "Test - ${createDisplayNameSuffix(it.path, null)}", path: it.path, filter: null))
70+
return testRuns
71+
}
72+
73+
def testClassesWithoutCommonPrefix = removeCommonPrefix(testClasses)
74+
testClassesWithoutCommonPrefix.forEach { className, displayName ->
75+
testRuns.add(new TestRun(displayName: "Test - ${createDisplayNameSuffix(it.path, displayName)}", path: it.path, filter: className))
76+
}
77+
78+
return testRuns
79+
}
80+
81+
@CompileStatic
82+
class TestRun {
83+
String displayName
84+
String path
85+
String filter
1286
}
1387

1488
subprojects.forEach { Project subProject ->
1589
subProject.tasks.register('determineTests') { Task it ->
90+
def tests = []
91+
1692
it.group = 'infrastructure'
1793
it.doLast {
1894
subProject.tasks.withType(Test).forEach { Task test ->
1995
def testSourceSetCandidate = test.extensions.findByName('test-source-set')
2096
if (testSourceSetCandidate != null) {
2197
SourceSet testSourceSet = testSourceSetCandidate as SourceSet
2298

99+
def testClasses = []
100+
23101
testSourceSet.java.srcDirs
24102
.collect { File file ->
25103
subProject.fileTree(file.parentFile)
26104
}
27105
.collect { FileTree fileTree ->
28-
return fileTree.matching {
29-
include '**/*Test.java'
30-
include '**/*Tests.java'
31-
include '**/*Test.groovy'
32-
include '**/*Tests.groovy'
106+
return fileTree.matching { PatternFilterable pattern ->
107+
pattern.include '**/*Test.java'
108+
pattern.include '**/*Tests.java'
109+
pattern.include '**/*Test.groovy'
110+
pattern.include '**/*Tests.groovy'
33111
}
34112
}
35113
.forEach {
36114
it.visit { FileVisitDetails details ->
37115
if (details.isDirectory())
38-
return;
116+
return
39117

40118
String className = details.relativePath.pathString
41119
.replace("groovy/", "")
@@ -44,14 +122,19 @@ subprojects.forEach { Project subProject ->
44122
.replace(".java", "")
45123
.replace("/", ".")
46124

47-
outputClassTest(subProject, test, className)
125+
testClasses.add(className)
48126
}
49127
}
50128

129+
tests.addAll(createTestRuns(test, testClasses))
51130
} else {
52-
outputUnitTest(subProject, test)
131+
tests.addAll(createTestRuns(test, new ArrayList<String>()))
53132
}
54133
}
134+
135+
if (!tests.isEmpty()) {
136+
project.getLogger().lifecycle("<< TEST >>${new JsonBuilder(tests)}")
137+
}
55138
}
56139
}
57140
}

‎.github/scripts/collect-tests.sh‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
#!/usr/bin/env bash
22

3-
./gradlew determineTests | grep -e '<< TEST >>' | sed -e 's/<< TEST >>//g' > ./tasks
4-
TESTS=$(cat tasks | jq --raw-input . | jq --compact-output --slurp .)
3+
TESTS=$(./gradlew determineTests | grep -e '<< TEST >>' | sed -e 's/<< TEST >>//g' | jq -s 'add' | jq -c .)
54
# Check if the GITHUB_OUTPUT is set
65
if [ -z "$GITHUB_OUTPUT" ]; then
76
# We do not have github output, then use the set output command

‎.github/workflows/test-prs.yml‎

Lines changed: 68 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,10 @@ on:
99
- ready_for_review
1010
- reopened
1111

12+
concurrency:
13+
group: ci-${{ github.head_ref || github.run_id }}
14+
cancel-in-progress: true
15+
1216
jobs:
1317
setup:
1418
name: Setup
@@ -62,13 +66,15 @@ jobs:
6266
run: ./gradlew --info -s -x assemble
6367

6468
test:
65-
name: Test
66-
runs-on: ubuntu-latest
69+
name: "${{ matrix.test.displayName }} (${{ matrix.os }})"
70+
runs-on: "${{ matrix.os }}-latest"
6771
needs: setup
6872
strategy:
73+
max-parallel: 15
6974
fail-fast: false
7075
matrix:
7176
test: ${{ fromJSON(needs.setup.outputs.tests-to-run) }}
77+
os: [ubuntu, windows, macos]
7278
steps:
7379
- name: Checkout repository
7480
uses: actions/checkout@v4
@@ -83,45 +89,77 @@ jobs:
8389
distribution: 'microsoft'
8490

8591
- name: Setup Gradle
86-
uses: gradle/gradle-build-action@v3
92+
uses: gradle/actions/setup-gradle@v3
8793

8894
- name: Test
89-
run: ./gradlew --info -s ${{ matrix.test }}
95+
if: ${{ matrix.test.filter != null }}
96+
run: ./gradlew --info -s ${{ matrix.test.path }} --tests "${{ matrix.test.filter }}"
97+
98+
- name: Test
99+
if: ${{ matrix.test.filter == null }}
100+
run: ./gradlew --info -s ${{ matrix.test.path }}
90101

91102
# Always upload test results
92103
- name: Merge Test Reports
93104
if: success() || failure()
94105
run: npx junit-report-merger junit.xml "**/TEST-*.xml"
95106

96107
- name: Format test run name as artifact name
97-
id: format-artifact-name
108+
if: (success() || failure()) && runner.os == 'Windows'
109+
id: format-artifact-name-windows
110+
# Use the GITHUB_OUTPUT mechanic to set the output variable
111+
run: |
112+
# We need to remove all invalid characters from the test name:
113+
# Invalid characters include: Double quote ", Colon :, Less than <, Greater than >, Vertical bar |, Asterisk *, Question mark ?, Carriage return \r, Line feed \n, Backslash \, Forward slash /
114+
$NAME = "${{ matrix.test.displayName }}" -replace '[":<>|*?\\/]', '-' -replace ' ', ''
115+
116+
# Check if the GITHUB_OUTPUT is set
117+
Write-Output "Determined name to be $NAME-${{ matrix.os }}"
118+
if ([string]::IsNullOrEmpty($env:GITHUB_OUTPUT)) {
119+
# We do not have github output, then use the set output command
120+
Write-Output "::set-output name=artifact-name::$NAME-${{ matrix.os }}"
121+
exit
122+
}
123+
Add-Content -Path $env:GITHUB_OUTPUT -Value "artifact-name=$NAME-${{ matrix.os }}"
124+
125+
- name: Format test run name as artifact name
126+
if: (success() || failure()) && runner.os != 'Windows'
127+
id: format-artifact-name-unix
98128
# Use the GITHUB_OUTPUT mechanic to set the output variable
99129
run: |
100-
# We have two cases here, one with a gradle task path, there we replace the : with a - and strip the leading -
101-
# The other case is complexer, again we replace the : with a - and strip the leading - but now we also remove the ' --tests ' part
102-
# Remove the '"' from the string and replace the '.' with a '-'
103-
NAME=$(echo "${{ matrix.test }}" | sed 's/:/-/g' | sed 's/^-//' | sed 's/ --tests /-/g' | sed 's/"//g' | sed 's/\./-/g')
130+
# We need to remove all invalid characters from the test name:
131+
# Invalid characters include: Double quote ", Colon :, Less than <, Greater than >, Vertical bar |, Asterisk *, Question mark ?, Carriage return \r, Line feed \n, Backslash \, Forward slash /
132+
NAME=$(echo "${{ matrix.test.displayName }}" | tr '":<>|*?\\/' '-' | tr -d ' ')
104133
# Check if the GITHUB_OUTPUT is set
134+
echo "Determined name to be $NAME-${{ matrix.os }}"
105135
if [ -z "$GITHUB_OUTPUT" ]; then
106136
# We do not have github output, then use the set output command
107-
echo "::set-output name=artifact-name::$NAME"
137+
echo "::set-output name=artifact-name::$NAME-${{ matrix.os }}"
108138
exit 0
109139
fi
110-
echo "artifact-name=$NAME" >> "$GITHUB_OUTPUT"
140+
echo "artifact-name=$NAME-${{ matrix.os }}" >> "$GITHUB_OUTPUT"
111141
142+
- uses: actions/upload-artifact@v4
143+
if: (success() || failure()) && runner.os != 'Windows'
144+
with:
145+
if-no-files-found: ignore
146+
name: test-results-${{ steps.format-artifact-name-unix.outputs.artifact-name }}
147+
path: junit.xml
148+
retention-days: 1
112149

113150
- uses: actions/upload-artifact@v4
114-
if: success() || failure()
151+
if: (success() || failure()) && runner.os == 'Windows'
115152
with:
116153
if-no-files-found: ignore
117-
name: test-results-${{ steps.format-artifact-name.outputs.artifact-name }}
154+
name: test-results-${{ steps.format-artifact-name-windows.outputs.artifact-name }}
118155
path: junit.xml
119156
retention-days: 1
120157

121158
process-test-data:
159+
name: Process Test Data
122160
runs-on: ubuntu-latest
123161
needs: test
124-
if: success() || failure()
162+
if: always()
125163
steps:
126164
- uses: actions/checkout@v3
127165

@@ -133,16 +171,29 @@ jobs:
133171

134172
- name: Publish Test Report
135173
uses: mikepenz/action-junit-report@v4
136-
if: success() || failure() # always run even if the previous step fails
174+
if: always() # always run even if the previous step fails
137175
with:
138176
report_paths: '**/*.xml'
139177

140178
- name: Merge Test Reports
141-
if: success() || failure()
179+
if: always()
142180
run: npx junit-report-merger junit.xml "**/*.xml"
143181

144182
- uses: actions/upload-artifact@v4
145-
if: success() || failure()
183+
if: always()
146184
with:
147185
name: test-results
148186
path: junit.xml
187+
188+
- name: Failed build detection
189+
uses: actions/github-script@v7
190+
if: >-
191+
${{
192+
contains(needs.*.result, 'failure')
193+
|| contains(needs.*.result, 'cancelled')
194+
|| contains(needs.*.result, 'skipped')
195+
}}
196+
with:
197+
script: |
198+
core.setFailed('Test build failure!')
199+

‎build.gradle‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,7 @@ subprojects.forEach { subProject ->
163163
spec.exclude group: 'org.codehaus.groovy'
164164
}
165165
evalSubProject.dependencies.functionalTestImplementation "net.neoforged.trainingwheels:gradle-functional:${project.trainingwheels_version}"
166+
evalSubProject.dependencies.functionalTestImplementation project(':test-utils')
166167

167168
//Configure the plugin metadata, so we can publish it.
168169
evalSubProject.gradlePlugin.plugins { NamedDomainObjectContainer<PluginDeclaration> plugins ->

‎common/src/main/java/net/neoforged/gradle/common/CommonPlugin.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,12 @@
22

33
import org.gradle.api.Plugin;
44
import org.gradle.api.Project;
5+
import org.jetbrains.annotations.NotNull;
56

67
public class CommonPlugin implements Plugin<Object> {
78
@Override
8-
public void apply(Object o) {
9-
if (o instanceof Project) {
10-
final Project project = (Project) o;
9+
public void apply(@NotNull Object o) {
10+
if (o instanceof Project project) {
1111
project.getPluginManager().apply(CommonProjectPlugin.class);
1212
} else {
1313
throw new IllegalArgumentException("CommonPlugin can only be applied to a project");

‎common/src/main/java/net/neoforged/gradle/common/CommonProjectPlugin.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,7 @@ public void apply(Project project) {
116116

117117
project.getRepositories().maven(e -> {
118118
e.setUrl(UrlConstants.MOJANG_MAVEN);
119-
e.metadataSources(MavenArtifactRepository.MetadataSources::artifact);
119+
e.metadataSources(MavenArtifactRepository.MetadataSources::mavenPom);
120120
});
121121

122122
project.getExtensions().getByType(SourceSetContainer.class).configureEach(sourceSet -> {

‎common/src/main/java/net/neoforged/gradle/common/util/ToolUtilities.java‎

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,10 @@
11
package net.neoforged.gradle.common.util;
22

3-
import net.neoforged.gradle.dsl.common.extensions.repository.Repository;
43
import net.neoforged.gradle.dsl.common.util.ConfigurationUtils;
54
import net.neoforged.gradle.util.ModuleDependencyUtils;
65
import org.gradle.api.Project;
76
import org.gradle.api.artifacts.Dependency;
87
import org.gradle.api.artifacts.ResolvedArtifact;
9-
import org.gradle.api.artifacts.repositories.ArtifactRepository;
108

119
import java.io.File;
1210
import java.util.function.Supplier;
@@ -18,33 +16,30 @@ private ToolUtilities() {
1816
}
1917

2018
public static File resolveTool(final Project project, final String tool) {
21-
return resolveTool(project, () -> ConfigurationUtils.temporaryUnhandledConfiguration(
19+
return resolveTool(() -> ConfigurationUtils.temporaryUnhandledConfiguration(
2220
project.getConfigurations(),
2321
"ToolLookupFor" + ModuleDependencyUtils.toConfigurationName(tool),
2422
project.getDependencies().create(tool)
2523
).getFiles().iterator().next());
2624
}
2725

2826
public static ResolvedArtifact resolveToolArtifact(final Project project, final String tool) {
29-
return resolveTool(project, () -> ConfigurationUtils.temporaryUnhandledConfiguration(
27+
return resolveTool(() -> ConfigurationUtils.temporaryUnhandledConfiguration(
3028
project.getConfigurations(),
3129
"ToolLookupFor" + ModuleDependencyUtils.toConfigurationName(tool),
3230
project.getDependencies().create(tool)
3331
).getResolvedConfiguration().getResolvedArtifacts().iterator().next());
3432
}
3533

3634
public static ResolvedArtifact resolveToolArtifact(final Project project, final Dependency tool) {
37-
return resolveTool(project, () -> ConfigurationUtils.temporaryUnhandledConfiguration(
35+
return resolveTool(() -> ConfigurationUtils.temporaryUnhandledConfiguration(
3836
project.getConfigurations(),
3937
"ToolLookupFor" + ModuleDependencyUtils.toConfigurationName(tool),
4038
tool
4139
).getResolvedConfiguration().getResolvedArtifacts().iterator().next());
4240
}
4341

44-
private static <T> T resolveTool(final Project project, final Supplier<T> searcher) {
45-
//Grab the dynamic repository
46-
final Repository repository = project.getExtensions().getByType(Repository.class);
47-
42+
private static <T> T resolveTool(final Supplier<T> searcher) {
4843
//Return the resolved artifact
4944
return searcher.get();
5045
}

0 commit comments

Comments
 (0)