Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Development: Execute tests only for changed modules on non-default branchs #10036

Open
wants to merge 69 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
69 commits
Select commit Hold shift + click to select a range
30a887e
Add dashboard for endpoint monitoring
ole-ve Dec 7, 2024
f584586
Update dashboard
ole-ve Dec 7, 2024
1acbe40
Make number of top endpoints a variable
ole-ve Dec 7, 2024
c9732aa
Debug: Append something to job summary
ole-ve Dec 7, 2024
0bb5333
Append per-module coverage to job summary (hardcoded module)
ole-ve Dec 7, 2024
c273d60
Temporarily add current branch to push triggering branches
ole-ve Dec 7, 2024
c2d0dfc
draft for module coverage
ole-ve Dec 7, 2024
48a4396
Extract jacoco gradle config from build.gradle to jacoco.gradle
ole-ve Dec 10, 2024
2e4d796
Only test changed modules
ole-ve Dec 10, 2024
739f60a
Parse and append per-module coverage results
ole-ve Dec 10, 2024
fd39101
Temporarily remove codacy coverage report upload
ole-ve Dec 10, 2024
5a409be
Upload whole build coverage report directory
ole-ve Dec 10, 2024
9e7d3cf
Temporarily fix tested modules
ole-ve Dec 10, 2024
5083184
temporarily add ArchitectureTest to have at least one test
ole-ve Dec 16, 2024
27b68ba
fix GH variable name of default branch
ole-ve Dec 16, 2024
d2a6f33
clone default branch if not running for the base branch
ole-ve Dec 16, 2024
5cbeac4
prefix branch name to compare with "origin/"
ole-ve Dec 16, 2024
f7e1408
remove null tasks from dependant list
ole-ve Dec 16, 2024
49c151f
correctly type testedModules as ArrayList
ole-ve Dec 16, 2024
e3def21
(temporarily) directly set TEST_MODULE_TAGS instead of evaluating wit…
ole-ve Dec 16, 2024
43b65eb
Add logging on executed test tags
ole-ve Dec 16, 2024
476c792
quote TEST_MODULE_TAGS
ole-ve Dec 16, 2024
786d172
log gradle command
ole-ve Dec 16, 2024
b31f0c8
Revert "log gradle command"
ole-ve Dec 16, 2024
2fd5936
split into includeModules and includeTags
ole-ve Dec 16, 2024
f394c70
change buildagent thresholds to match module-only results
ole-ve Dec 16, 2024
e022592
Move basePath from jacoco.gradle to build.gradle
ole-ve Dec 16, 2024
7dd9730
ensure module-specific jacoco task runs after "rootTask"
ole-ve Dec 16, 2024
98227e8
add comment on including junit tags logic
ole-ve Dec 16, 2024
c860849
create jacoco report for full Artemis bundle also when testing specif…
ole-ve Dec 16, 2024
6453837
remove debugging step from pipeline
ole-ve Dec 16, 2024
9c4348f
adapt expected number of server starts
ole-ve Dec 16, 2024
42b9f88
use finalizedBy instead of mustRunAfter
ole-ve Dec 16, 2024
39f568d
make report paths explicit
ole-ve Dec 16, 2024
d67a9b4
adjust per-module covered instruction threshold for fileupload
ole-ve Dec 16, 2024
e15e05f
remove hardcoded TEST_MODULE_TAGS from test pipeline
ole-ve Dec 16, 2024
78f77a5
move jacoco-parser to .ci-dir and rename to execute.py
ole-ve Dec 16, 2024
ad657cf
move jacoco.gradle to gradle-dir
ole-ve Dec 16, 2024
cfc2f38
move changed-modules.sh to .dir directory
ole-ve Dec 16, 2024
8b6aa0c
fix logic of number of server starts script
ole-ve Dec 16, 2024
1f54c61
update isolated coverage threshold per module
ole-ve Dec 16, 2024
be4da5e
move generate_code_cov_table to subfolder
ole-ve Dec 16, 2024
49ddd27
move get_changed_modules.sh to supporting scripts
ole-ve Dec 16, 2024
46280fe
move parse_module_coverage to supporting scripts
ole-ve Dec 16, 2024
1cad26f
add documentation to get_changed_modules.sh
ole-ve Dec 16, 2024
bfd2c8d
use test filters instead of "include" to ensure test results are crea…
ole-ve Dec 16, 2024
6e0baa5
println for missing coverage message
ole-ve Dec 16, 2024
5ab2996
add info on return type of get_changed_modules.sh
ole-ve Dec 16, 2024
7fd0ad0
refactor: separate concerns in parse_module_coverage.py
ole-ve Dec 16, 2024
2a2b8d7
rename get_changed_modules.sh to test_changed_modules.sh
ole-ve Dec 16, 2024
5de1c8a
Revert "rename get_changed_modules.sh to test_changed_modules.sh"
ole-ve Dec 16, 2024
9de9c3a
fix issues with parse_module_coverage.py
ole-ve Dec 16, 2024
4cdb0fa
remove branch input validation
ole-ve Dec 16, 2024
e1748af
generate full reports when no input modules specified
ole-ve Dec 16, 2024
240aef1
change message to be more explicit about skipping coverage verification
ole-ve Dec 16, 2024
b1042a4
add debugging
ole-ve Dec 17, 2024
e668fb9
Revert "add debugging"
ole-ve Dec 17, 2024
3cdfda7
log command
ole-ve Dec 17, 2024
42d0b82
move test module tags further into the command
ole-ve Dec 17, 2024
53ca8d0
add stacktrace logging
ole-ve Dec 17, 2024
20a43df
explicitly run all or only-specific tests
ole-ve Dec 17, 2024
ce82e2e
remove unused file
ole-ve Dec 17, 2024
4ccaa0a
fix module name in error message
ole-ve Dec 17, 2024
7d3cc92
remove fixed branch
ole-ve Dec 17, 2024
8c58a39
change upload artifact path
ole-ve Dec 17, 2024
dbe638e
add codacy step again
ole-ve Dec 17, 2024
830d2bd
fix jacoco report output location for aggregated report
ole-ve Dec 17, 2024
7e59059
rename jacocoCoverageReport.xml to jacocoTestReport.xml
ole-ve Dec 17, 2024
631e229
include tests into determination of changed modules
ole-ve Dec 17, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/PULL_REQUEST_TEMPLATE.md
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ Prerequisites:
- [ ] Test 2

### Test Coverage
<!-- Please add the test coverages for all changed files modified in this PR here. You can use `supporting_script/generate_code_cov_table/generate_code_cov_table.py` to automatically generate the coverage table from the corresponding artefacts of your branch (follow the ReadMe for setup details). -->
<!-- Please add the test coverages for all changed files modified in this PR here. You can use `supporting_script/code-coverage/generate_code_cov_table/generate_code_cov_table.py` to automatically generate the coverage table from the corresponding artefacts of your branch (follow the ReadMe for setup details). -->
<!-- Alternatively you can execute the tests locally (see build.gradle and package.json) or look into the corresponding artefacts. -->
<!-- The line coverage must be above 90% for changes files, and you must use extensive and useful assertions for server tests and expect statements for client tests. -->
<!-- Note: Confirm in the last column that you have implemented extensive assertions for server tests and expect statements for client tests. -->
Expand Down
2 changes: 1 addition & 1 deletion .github/dependabot.yml
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ updates:

# Check for version updates for Python dependencies (coverage)
- package-ecosystem: "pip"
directory: "/supporting_scripts/generate_code_cov_table"
directory: "/supporting_scripts/code-coverage/generate_code_cov_table"
schedule:
interval: "weekly"
reviewers:
Expand Down
36 changes: 33 additions & 3 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,32 @@ jobs:
${{ env.java }}
cache: 'gradle'
- name: Java Tests
run: set -o pipefail && ./gradlew --console=plain test jacocoTestReport -x webapp jacocoTestCoverageVerification | tee tests.log
run: |
set -o pipefail

DEFAULT_BRANCH="${{ github.event.repository.default_branch }}"
CURRENT_BRANCH="${{ github.ref_name }}"

if [[ "$DEFAULT_BRANCH" != "$CURRENT_BRANCH" ]]; then
# Explicitly fetch as the clone action only clones the current branch
git fetch origin "$DEFAULT_BRANCH"

chmod +x ./supporting_scripts/get_changed_modules.sh
CHANGED_MODULES=$(./supporting_scripts/get_changed_modules.sh "origin/$DEFAULT_BRANCH")

# Restrict executed tests to changed modules if there is diff between this and the base branch
if [ -n "${CHANGED_MODULES}" ]; then
IFS=,
TEST_MODULE_TAGS=$(echo "-DincludeModules=${CHANGED_MODULES[*]}")

echo "Executing tests for modules: $CHANGED_MODULES"
./gradlew --console=plain test jacocoTestReport -x webapp jacocoTestCoverageVerification "$TEST_MODULE_TAGS" | tee tests.log
exit 0
fi
fi

echo "Executing all tests"
./gradlew --console=plain test jacocoTestReport -x webapp jacocoTestCoverageVerification | tee tests.log
- name: Print failed tests
if: failure()
run: grep "Test >.* FAILED\$" tests.log || echo "No failed tests."
Expand Down Expand Up @@ -92,8 +117,13 @@ jobs:
uses: actions/upload-artifact@v4
with:
name: Coverage Report Server Tests
path: build/reports/jacoco/test/html/

path: build/reports/jacoco/test/html
- name: Append Per-Module Coverage to Job Summary
if: success() || failure()
run: |
AGGREGATED_REPORT_FILE=./module_coverage_report.md
python3 ./supporting_scripts/code-coverage/per_module_cov_report/parse_module_coverage.py build/reports/jacoco $AGGREGATED_REPORT_FILE
cat $AGGREGATED_REPORT_FILE > $GITHUB_STEP_SUMMARY

server-tests-mysql:
needs: [ server-tests ]
Expand Down
81 changes: 28 additions & 53 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -118,16 +118,38 @@ modernizer {
exclusions = ["java/util/Optional.get:()Ljava/lang/Object;"]
}

// Allow using in jacoco.gradle
ext {
includedTestTags = System.getProperty("includeTags")
includedTags = !includedTestTags ? new String[]{} : includedTestTags.split(",") as String[]
includedModulesTag = System.getProperty("includeModules")
includedModules = !includedModulesTag ? new String[]{} : includedModulesTag.split(",") as String[]

runAllTests = includedTags.size() == 0 && includedModules.size() == 0
BasePath = "de/tum/cit/aet/artemis"
}

// Execute the test cases: ./gradlew test
// Execute only architecture tests: ./gradlew test -DincludeTags="ArchitectureTest"
// Execute tests only for specific modules: ./gradlew test -DincludeModules="atlas". Using this flag, "includeTags" will be ignored.
test {
if (System.getProperty("includeTags")) {
useJUnitPlatform {
includeTags System.getProperty("includeTags")
if (runAllTests) {
useJUnitPlatform()
exclude "**/*IT*", "**/*IntTest*"
} else if (includedModules.size() == 0) {
// not running all tests, but not module-specific ones -> use tags
useJUnitPlatform() {
includeTags includedTags
}
} else {
useJUnitPlatform()
exclude "**/*IT*", "**/*IntTest*"
// Always execute "shared"-folder when executing module-specifc tests
includedModules += "shared"
filter { testFilter ->
includedModules.each { val ->
testFilter.includeTestsMatching("de.tum.cit.aet.artemis.$val.*")
}
}
}

testLogging {
Expand All @@ -144,59 +166,12 @@ tasks.register("testReport", TestReport) {
testResults.from(test)
}

jacoco {
toolVersion = "0.8.12"
}

jar {
enabled = false
}

private excludedClassFilesForReport(classDirectories) {
classDirectories.setFrom(files(classDirectories.files.collect {
fileTree(dir: it,
exclude: [
"**/de/tum/cit/aet/artemis/**/domain/**/*_*",
"**/de/tum/cit/aet/artemis/core/config/migration/entries/**",
"**/gradle-wrapper.jar/**"
]
)
}))
}

jacocoTestReport {
reports {
xml.required = true
}
// we want to ignore some generated files in the domain folders
afterEvaluate {
excludedClassFilesForReport(classDirectories)
}
}

jacocoTestCoverageVerification {
violationRules {
rule {
limit {
counter = "INSTRUCTION"
value = "COVEREDRATIO"
// TODO: in the future the following value should become higher than 0.92
minimum = 0.892
}
limit {
counter = "CLASS"
value = "MISSEDCOUNT"
// TODO: in the future the following value should become less than 10
maximum = 65
}
}
}
// we want to ignore some generated files in the domain folders
afterEvaluate {
excludedClassFilesForReport(classDirectories)
}
}
check.dependsOn jacocoTestCoverageVerification
// Dynamic generation of jacoco test report generation-/coverage verification-tasks (per-module)
apply from: "gradle/jacoco.gradle"

configurations {
providedRuntime
Expand Down
217 changes: 217 additions & 0 deletions gradle/jacoco.gradle
Original file line number Diff line number Diff line change
@@ -0,0 +1,217 @@
ext {
AggregatedCoverageThresholds = [
"INSTRUCTION": 0.900,
"CLASS": 10
];
// (Isolated) thresholds when executing each module on its own
ModuleCoverageThresholds = [
"assessment" : [
"INSTRUCTION": 0.774,
"CLASS": 9
],
"athena" : [
"INSTRUCTION": 0.863,
"CLASS": 2
],
"atlas" : [
"INSTRUCTION": 0.846,
"CLASS": 13
],
"buildagent" : [
"INSTRUCTION": 0.304,
"CLASS": 13
],
"communication": [
"INSTRUCTION": 0.893,
"CLASS": 7
],
"core" : [
"INSTRUCTION": 0.658,
"CLASS": 69
],
"exam" : [
"INSTRUCTION": 0.914,
"CLASS": 1
],
"exercise" : [
"INSTRUCTION": 0.649,
"CLASS": 9
],
"fileupload" : [
"INSTRUCTION": 0.944,
"CLASS": 0
],
"iris" : [
"INSTRUCTION": 0.706,
"CLASS": 22
],
"lecture" : [
"INSTRUCTION": 0.867,
"CLASS": 0
],
"lti" : [
"INSTRUCTION": 0.770,
"CLASS": 3
],
"modeling" : [
"INSTRUCTION": 0.892,
"CLASS": 2
],
"plagiarism" : [
"INSTRUCTION": 0.770,
"CLASS": 1
],
"programming" : [
"INSTRUCTION": 0.861,
"CLASS": 13
],
"quiz" : [
"INSTRUCTION": 0.785,
"CLASS": 6
],
"text" : [
"INSTRUCTION": 0.847,
"CLASS": 0
],
"tutorialgroup": [
"INSTRUCTION": 0.915,
"CLASS": 0
],
]
// If no explicit modules defined -> generate reports and validate for each module
reportedModules = includedModules.size() == 0
? ModuleCoverageThresholds.collect {element -> element.key}
: includedModules as ArrayList

// we want to ignore some generated files in the domain folders
ignoredDirectories = [
"**/$BasePath/**/domain/**/*_*",
"**/$BasePath/core/config/migration/entries/**",
"**/gradle-wrapper.jar/**"
]
}

jacoco {
toolVersion = "0.8.12"
}

jacocoTestReport {
// For the aggregated report
reports {
xml.required = true
xml.outputLocation = file("build/reports/jacoco/test/jacocoTestReport.xml")
html.required = true
html.outputLocation = file("build/reports/jacoco/test/html")
}

finalizedBy reportedModules
.collect { module -> registerJacocoReportTask(module as String, jacocoTestReport) }
.findAll { task -> task != null}
}

jacocoTestCoverageVerification {
// Only run full coverage when no specific modules set
enabled = reportedModules.size() == 0

def minInstructionCoveredRatio = AggregatedCoverageThresholds["INSTRUCTION"] as double
def maxNumberUncoveredClasses = AggregatedCoverageThresholds["CLASS"] as int
applyVerificationRule(jacocoTestCoverageVerification, minInstructionCoveredRatio, maxNumberUncoveredClasses)

finalizedBy reportedModules
.collect { module -> registerJacocoTestCoverageVerification(module as String, jacocoTestCoverageVerification) }
.findAll { task -> task != null}
}
check.dependsOn jacocoTestCoverageVerification

/**
* Registers a JacocoReport task based on the provided parameters.
*
* @param moduleName The module name to include in the report.
* @param rootTask The root JacocoReport root task.
* @return The configured JacocoReport task.
*/
private JacocoReport registerJacocoReportTask(String moduleName, JacocoReport rootTask) {
def taskName = "jacocoCoverageReport-$moduleName"

JacocoReport task = project.tasks.register(taskName, JacocoReport).get()
task.description = "Generates JaCoCo coverage report for $moduleName"

prepareJacocoReportTask(task, moduleName, rootTask)

task.reports {
xml.required = true
xml.outputLocation = file("build/reports/jacoco/$moduleName/jacocoTestReport.xml")
html.required = true
html.outputLocation = file("build/reports/jacoco/$moduleName/html")
}

return task
}

/**
* Registers a JacocoCoverageVerification task based on the provided parameters.
*
* @param moduleName The module name to validate rules for.
* @param rootTask The root JacocoCoverageVerification task.
* @return The configured JacocoCoverageVerification task.
*/
private JacocoCoverageVerification registerJacocoTestCoverageVerification(String moduleName, JacocoCoverageVerification rootTask) {
def taskName = "jacocoTestCoverageVerification-$moduleName"

def thresholds = ModuleCoverageThresholds[moduleName]
if (thresholds == null) {
println "No coverage thresholds defined for module '$moduleName'. Skipping verification..."
return null
}
def minInstructionCoveredRatio = thresholds["INSTRUCTION"] as double
def maxNumberUncoveredClasses = thresholds["CLASS"] as int

JacocoCoverageVerification task = project.tasks.register(taskName, JacocoCoverageVerification).get()
task.description = "Validates JaCoCo coverage for vioalations for $moduleName"

prepareJacocoReportTask(task, moduleName, rootTask)
applyVerificationRule(task, minInstructionCoveredRatio, maxNumberUncoveredClasses)

return task
}

/**
* Prepares a Jacoco report task (report & verification) to match a specific
* @param task that is modified
* @param moduleName of the module.
* @param rootTask the JacocoReportBase root task
*/
private void prepareJacocoReportTask(JacocoReportBase task, String moduleName, JacocoReportBase rootTask) {
task.group = "Reporting"
task.executionData = project.fileTree("${project.layout.buildDirectory.get()}/jacoco") {
include "test.exec"
}

def modulePath = "$BasePath/$moduleName/**/*.class"
task.sourceDirectories.setFrom(project.files("src/main/java/$modulePath"))
Comment on lines +190 to +191
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Prevent path traversal vulnerability

The module path construction should validate the module name to prevent path traversal.

+    if (!moduleName.matches("^[a-zA-Z0-9-_]+$")) {
+        throw new GradleException("Invalid module name: $moduleName")
+    }
     def modulePath = "$BasePath/$moduleName/**/*.class"
     task.sourceDirectories.setFrom(project.files("src/main/java/$modulePath"))
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
def modulePath = "$BasePath/$moduleName/**/*.class"
task.sourceDirectories.setFrom(project.files("src/main/java/$modulePath"))
if (!moduleName.matches("^[a-zA-Z0-9-_]+$")) {
throw new GradleException("Invalid module name: $moduleName")
}
def modulePath = "$BasePath/$moduleName/**/*.class"
task.sourceDirectories.setFrom(project.files("src/main/java/$modulePath"))

task.classDirectories.setFrom(
files(rootTask.classDirectories.files.collect { classDir ->
project.fileTree(classDir) {
includes=[modulePath]
excludes=ignoredDirectories
}
})
)
}

private static void applyVerificationRule(JacocoCoverageVerification task, double minInstructionCoveredRatio, int maxNumberUncoveredClasses) {
task.violationRules {
rule {
limit {
counter = "INSTRUCTION"
value = "COVEREDRATIO"
minimum = minInstructionCoveredRatio
}
limit {
counter = "CLASS"
value = "MISSEDCOUNT"
maximum = maxNumberUncoveredClasses
}
}
}
}
Loading
Loading