Skip to content

Commit 420e974

Browse files
committed
fix: Address PR #186 code quality issues
Issue #5: System property restoration in TestApplication - Added previousSkipEffectivePomValue field to store previous property value - Modified useEffectivePomGeneration() to save previous value before clearing - Added cleanup() method to restore system property after tests - Prevents test pollution where one test's effective POM setting affects others Issue #6: Silent fallback on Maven failure in MuleApplication - Changed getEffectivePomFile() to throw RuntimeException instead of silently falling back - Provides clear error message with Maven exit code and troubleshooting hint - Fail-fast approach prevents rules from silently operating on incomplete POM data Issue #8: Temp file leak on Maven exception - Moved deleteOnExit() registration immediately after temp file creation - Wrapped Maven invocation in try-catch to ensure temp file cleanup on exception - Prevents effective-pom temp files from accumulating in /tmp on Maven failures All changes verified with successful build.
1 parent 7ba1f6f commit 420e974

2 files changed

Lines changed: 47 additions & 22 deletions

File tree

mule-linter-core/src/main/groovy/com/avioconsulting/mule/linter/model/MuleApplication.groovy

Lines changed: 28 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -77,30 +77,37 @@ class MuleApplication implements Application {
7777
if (mavenHome == null)
7878
throw new MavenInvocationException( MAVEN_HOME_DOES_NOT_EXIST)
7979

80-
File effectivePomFile = File.createTempFile("effective-pom", ".xml");
81-
def mavenInvokeRequest = new DefaultInvocationRequest().with {
82-
String mvnGoals = 'help:effective-pom -Doutput='+effectivePomFile.getAbsolutePath()
83-
setGoals([mvnGoals])
84-
setPomFile(pFile)
85-
setShowErrors(true)
86-
// Add timeout to prevent hanging
87-
setTimeoutInSeconds(60)
88-
it
89-
}
90-
def mavenInvoker = new DefaultInvoker()
91-
mavenInvoker.setMavenHome(new File(mavenHome))
92-
def result = mavenInvoker.execute(mavenInvokeRequest)
80+
File effectivePomFile = File.createTempFile("effective-pom", ".xml")
81+
// Register for deletion immediately to prevent temp file leak on exception
82+
effectivePomFile.deleteOnExit()
9383

94-
// Check if Maven invocation succeeded
95-
if (result == null || result.getExitCode() != 0) {
84+
try {
85+
def mavenInvokeRequest = new DefaultInvocationRequest().with {
86+
String mvnGoals = 'help:effective-pom -Doutput='+effectivePomFile.getAbsolutePath()
87+
setGoals([mvnGoals])
88+
setPomFile(pFile)
89+
setShowErrors(true)
90+
// Add timeout to prevent hanging
91+
setTimeoutInSeconds(60)
92+
it
93+
}
94+
def mavenInvoker = new DefaultInvoker()
95+
mavenInvoker.setMavenHome(new File(mavenHome))
96+
def result = mavenInvoker.execute(mavenInvokeRequest)
97+
98+
// Check if Maven invocation succeeded
99+
if (result == null || result.getExitCode() != 0) {
100+
effectivePomFile.delete()
101+
throw new RuntimeException("Failed to generate effective POM for ${pFile.absolutePath}. " +
102+
"Maven exit code: ${result?.exitCode ?: 'null'}. " +
103+
"Check that Maven can resolve all parent POMs and dependencies.")
104+
}
105+
106+
return effectivePomFile
107+
} catch (MavenInvocationException e) {
96108
effectivePomFile.delete()
97-
// Fall back to original pom file if effective pom generation fails
98-
println "Warning: Failed to generate effective POM, using original pom.xml"
99-
return pFile
109+
throw new RuntimeException("Failed to invoke Maven for effective POM generation: ${e.message}", e)
100110
}
101-
102-
effectivePomFile.deleteOnExit();
103-
return effectivePomFile
104111
}
105112

106113
void loadPropertyFiles() {

mule-linter-core/src/test/groovy/com/avioconsulting/mule/linter/TestApplication.groovy

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,9 @@ import com.avioconsulting.mule.linter.rule.cicd.JenkinsFileExistsRule
1212
class TestApplication {
1313

1414
static final String SAMPLE_APP_NAME = 'SampleMuleApp'
15+
16+
// Store previous value of system property for restoration
17+
private String previousSkipEffectivePomValue
1518
static final List<String> CONFIGS = ['src/main/mule/business-logic.xml',
1619
'src/main/mule/global-config.xml',
1720
'src/main/mule/sample-mule-app-api.xml']
@@ -53,9 +56,11 @@ class TestApplication {
5356

5457
/**
5558
* Enable effective POM generation for tests.
56-
* Clears the skip flag so MuleApplication uses effective POM
59+
* Clears the skip flag so MuleApplication uses effective POM.
60+
* Remember to call cleanup() in test cleanup to restore the previous value.
5761
*/
5862
void useEffectivePomGeneration() {
63+
previousSkipEffectivePomValue = System.getProperty('mule.linter.skipEffectivePom')
5964
System.clearProperty('mule.linter.skipEffectivePom')
6065
}
6166

@@ -139,6 +144,19 @@ class TestApplication {
139144
void remove() {
140145
appDir.deleteDir()
141146
}
147+
148+
/**
149+
* Restore system properties that were modified during test setup.
150+
* Call this in test cleanup() to avoid polluting other tests.
151+
*/
152+
void cleanup() {
153+
// Restore the mule.linter.skipEffectivePom property
154+
if (previousSkipEffectivePomValue != null) {
155+
System.setProperty('mule.linter.skipEffectivePom', previousSkipEffectivePomValue)
156+
} else {
157+
System.clearProperty('mule.linter.skipEffectivePom')
158+
}
159+
}
142160

143161
void removeFile(String fileName) {
144162
File fileToRemove = new File(appDir, fileName)

0 commit comments

Comments
 (0)