Skip to content

Commit eb959c7

Browse files
rasharabrasharab
committed
Address comments
1. Add abiility for tests to override environment behaviour 2. Fix tests.
1 parent f951da8 commit eb959c7

File tree

8 files changed

+159
-26
lines changed

8 files changed

+159
-26
lines changed

‎src/main/kotlin/com/github/gradle/node/NodeExtension.kt‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import com.github.gradle.node.util.Platform
55
import org.gradle.api.Project
66
import org.gradle.kotlin.dsl.create
77
import org.gradle.kotlin.dsl.getByType
8+
import org.gradle.kotlin.dsl.mapProperty
89
import org.gradle.kotlin.dsl.property
910

1011
open class NodeExtension(project: Project) {
@@ -181,6 +182,9 @@ open class NodeExtension(project: Project) {
181182
*/
182183
val resolvedPlatform = project.objects.property<Platform>()
183184

185+
186+
val environment = project.objects.mapProperty<String, String>().convention(System.getenv())
187+
184188
init {
185189
distBaseUrl.set("https://nodejs.org/dist")
186190
}

‎src/main/kotlin/com/github/gradle/node/exec/ExecRunner.kt‎

Lines changed: 4 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,10 @@ import java.io.File
1313
*
1414
* @param execConfiguration configuration to get environment variables from
1515
*/
16-
fun computeEnvironment(execConfiguration: ExecConfiguration): Map<String, String> {
16+
fun computeEnvironment(extension: NodeExtension, execConfiguration: ExecConfiguration): Map<String, String> {
1717
val execEnvironment = mutableMapOf<String, String>()
1818
execEnvironment += System.getenv()
19+
execEnvironment += extension.environment.getOrElse(emptyMap())
1920
execEnvironment += execConfiguration.environment
2021
if (execConfiguration.additionalBinPaths.isNotEmpty()) {
2122
// Take care of Windows environments that may contain "Path" OR "PATH" - both existing
@@ -34,25 +35,6 @@ fun computeWorkingDir(nodeProjectDir: DirectoryProperty, execConfiguration: Exec
3435
workingDir.mkdirs()
3536
return workingDir
3637
}
37-
38-
/**
39-
* Helper function to find the best matching executable in the system PATH.
40-
*
41-
* @param executableName The name of the executable to search for.
42-
* @return The best matching executable path as a String.
43-
*/
44-
fun findBestExecutableMatch(executableName: String): String {
45-
val pathVariable = System.getenv("PATH") ?: return executableName
46-
val paths = pathVariable.split(File.pathSeparator)
47-
for (path in paths) {
48-
val executableFile = File(path, executableName)
49-
if (executableFile.exists() && executableFile.canExecute()) {
50-
return executableFile.absolutePath
51-
}
52-
}
53-
return executableName // Return the original executable if no match is found
54-
}
55-
5638
/**
5739
* Basic execution runner that runs a given ExecConfiguration.
5840
*
@@ -61,11 +43,10 @@ fun findBestExecutableMatch(executableName: String): String {
6143
*/
6244
class ExecRunner {
6345
fun execute(projectHelper: ProjectApiHelper, extension: NodeExtension, execConfiguration: ExecConfiguration): ExecResult {
64-
val executablePath = findBestExecutableMatch(execConfiguration.executable)
6546
return projectHelper.exec {
66-
executable = executablePath
47+
executable = execConfiguration.executable
6748
args = execConfiguration.args
68-
environment = computeEnvironment(execConfiguration)
49+
environment = computeEnvironment(extension, execConfiguration)
6950
isIgnoreExitValue = execConfiguration.ignoreExitValue
7051
workingDir = computeWorkingDir(extension.nodeProjectDir, execConfiguration)
7152
execConfiguration.execOverrides?.execute(this)

‎src/main/kotlin/com/github/gradle/node/variant/VariantComputer.kt‎

Lines changed: 31 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,19 +8,47 @@ import org.gradle.api.file.Directory
88
import org.gradle.api.file.DirectoryProperty
99
import org.gradle.api.provider.Property
1010
import org.gradle.api.provider.Provider
11+
import java.io.File
12+
13+
/**
14+
* Helper function to find the best matching executable in the system PATH.
15+
*
16+
* @param executableName The name of the executable to search for.
17+
* @return The best matching executable path as a String.
18+
*/
19+
fun findBestExecutableMatch(
20+
executableName: String,
21+
environmentProvider: Provider<Map<String, String>>,
22+
): String {
23+
val environment = environmentProvider.orNull ?: return executableName
24+
val pathEnvironmentVariableName = if (environment["Path"] != null) "Path" else "PATH"
25+
val pathVariable = environment[pathEnvironmentVariableName] ?: return executableName
26+
val paths = pathVariable.split(File.pathSeparator)
27+
for (path in paths) {
28+
val executableFile = File(path, executableName)
29+
if (executableFile.exists() && executableFile.canExecute()) {
30+
return executableFile.absolutePath
31+
}
32+
}
33+
return executableName // Return the original executable if no match is found
34+
}
1135

1236
/**
1337
* Get the expected node binary name, node.exe on Windows and node everywhere else.
1438
*/
15-
fun computeNodeExec(nodeExtension: NodeExtension, nodeBinDirProvider: Provider<Directory>): Provider<String> {
39+
fun computeNodeExec(
40+
nodeExtension: NodeExtension,
41+
nodeBinDirProvider: Provider<Directory>,
42+
): Provider<String> {
1643
return zip(nodeExtension.download, nodeBinDirProvider).map {
1744
val (download, nodeBinDir) = it
1845
if (download) {
1946
val nodeCommand = if (nodeExtension.resolvedPlatform.get().isWindows()) "node.exe" else "node"
2047
nodeBinDir.dir(nodeCommand).asFile.absolutePath
21-
} else "node"
48+
} else findBestExecutableMatch("node", nodeExtension.environment)
2249
}
2350
}
51+
2452
fun computeNpmScriptFile(nodeDirProvider: Provider<Directory>, command: String, isWindows: Boolean): Provider<String> {
2553
return nodeDirProvider.map { nodeDir ->
2654
if (isWindows) nodeDir.dir("node_modules/npm/bin/$command-cli.js").asFile.path
@@ -51,7 +79,7 @@ internal fun computeExec(nodeExtension: NodeExtension, binDirProvider: Provider<
5179
val command = if (nodeExtension.resolvedPlatform.get().isWindows()) {
5280
cfgCommand.mapIf({ it == unixCommand }) { windowsCommand }
5381
} else cfgCommand
54-
if (download) binDir.dir(command).asFile.absolutePath else command
82+
if (download) binDir.dir(command).asFile.absolutePath else findBestExecutableMatch(command, nodeExtension.environment)
5583
}
5684
}
5785

‎src/test/groovy/com/github/gradle/node/npm/task/NpmInstallTaskTest.groovy‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ class NpmInstallTaskTest extends AbstractTaskTest {
1313
def "exec npm install task with configured proxy"() {
1414
given:
1515
nodeExtension.resolvedPlatform.set(PlatformHelperKt.parsePlatform("Linux", "x86_64", {}))
16+
nodeExtension.environment.set([:])
1617
GradleProxyHelper.setHttpsProxyHost("my-super-proxy.net")
1718
GradleProxyHelper.setHttpsProxyPort(11235)
1819

@@ -32,6 +33,7 @@ class NpmInstallTaskTest extends AbstractTaskTest {
3233
def "exec npm install task with configured proxy but disabled"() {
3334
given:
3435
nodeExtension.resolvedPlatform.set(PlatformHelperKt.parsePlatform("Linux", "x86_64", {}))
36+
nodeExtension.environment.set([:])
3537
GradleProxyHelper.setHttpsProxyHost("my-super-proxy.net")
3638
GradleProxyHelper.setHttpsProxyPort(11235)
3739
nodeExtension.nodeProxySettings.set(ProxySettings.OFF)

‎src/test/groovy/com/github/gradle/node/npm/task/NpmTaskTest.groovy‎

Lines changed: 113 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,16 +6,92 @@ import com.github.gradle.node.npm.proxy.ProxySettings
66
import com.github.gradle.node.task.AbstractTaskTest
77
import com.github.gradle.node.util.PlatformHelperKt
88

9+
import java.nio.file.Files
10+
911
import static com.github.gradle.node.NodeExtension.DEFAULT_NODE_VERSION
1012

1113
class NpmTaskTest extends AbstractTaskTest {
1214
def cleanup() {
1315
GradleProxyHelper.resetProxy()
1416
}
1517

18+
/**
19+
* Creates a temporary directory with a dummy executable file.
20+
*
21+
* @param fileName The name of the dummy executable (e.g., "npm").
22+
* @param scriptContent (Optional) The content inside the executable. Defaults to a simple echo script.
23+
* @return File object pointing to the dummy executable.
24+
*/
25+
File createTempDummyExecutable(String fileName, String scriptContent = "#!/bin/bash\necho 'Dummy executable ran with args: \$@'") {
26+
// Create a temporary directory
27+
File tempDir = Files.createTempDirectory("dummy-bin").toFile()
28+
29+
// Create the dummy executable inside the temp directory
30+
File executableFile = new File(tempDir, fileName)
31+
32+
// Write the script content
33+
executableFile.text = scriptContent
34+
35+
// Set executable permissions
36+
executableFile.setExecutable(true)
37+
38+
return executableFile
39+
}
40+
41+
def "exec npm task with environment variable"() {
42+
given:
43+
def npmFile = createTempDummyExecutable("npm")
44+
45+
nodeExtension.resolvedPlatform.set(PlatformHelperKt.parsePlatform("Linux", "x86_64", {}))
46+
nodeExtension.environment.set(["PATH": npmFile.parent])
47+
48+
def task = project.tasks.create('simple', NpmTask)
49+
mockProjectApiHelperExec(task)
50+
task.args.set(['a', 'b'])
51+
task.environment.set(['a': '1'])
52+
task.ignoreExitValue.set(true)
53+
task.workingDir.set(projectDir)
54+
55+
when:
56+
project.evaluate()
57+
task.exec()
58+
59+
then:
60+
1 * execSpec.setIgnoreExitValue(true)
61+
1 * execSpec.setEnvironment({ it['a'] == '1' && containsPath(it) })
62+
1 * execSpec.setExecutable(npmFile.absolutePath)
63+
1 * execSpec.setArgs(['a', 'b'])
64+
}
65+
66+
def "exec npm task with environment variable (windows)"() {
67+
given:
68+
def npmFile = createTempDummyExecutable("npm.cmd")
69+
70+
nodeExtension.resolvedPlatform.set(PlatformHelperKt.parsePlatform("Windows", "x86_64", {}))
71+
nodeExtension.environment.set(["Path": npmFile.parent])
72+
73+
def task = project.tasks.create('simple', NpmTask)
74+
mockProjectApiHelperExec(task)
75+
task.args.set(['a', 'b'])
76+
task.environment.set(['a': '1'])
77+
task.ignoreExitValue.set(true)
78+
task.workingDir.set(projectDir)
79+
80+
when:
81+
project.evaluate()
82+
task.exec()
83+
84+
then:
85+
1 * execSpec.setIgnoreExitValue(true)
86+
1 * execSpec.setEnvironment({ it['a'] == '1' && containsPath(it) })
87+
1 * execSpec.setExecutable(npmFile.absolutePath)
88+
1 * execSpec.setArgs(['a', 'b'])
89+
}
90+
1691
def "exec npm task"() {
1792
given:
1893
nodeExtension.resolvedPlatform.set(PlatformHelperKt.parsePlatform("Linux", "x86_64", {}))
94+
nodeExtension.environment.set([:])
1995
2096
def task = project.tasks.create('simple', NpmTask)
2197
mockProjectApiHelperExec(task)
@@ -57,6 +133,39 @@ class NpmTaskTest extends AbstractTaskTest {
57133
1 * execSpec.setArgs(['a', 'b'])
58134
}
59135
136+
def "exec npm task with path respects download"() {
137+
given:
138+
def npmFile = createTempDummyExecutable("npm")
139+
140+
nodeExtension.resolvedPlatform.set(PlatformHelperKt.parsePlatform("Linux", "x86_64", {}))
141+
nodeExtension.environment.set(["PATH": npmFile.parent])
142+
nodeExtension.download.set(true)
143+
144+
def nodeDir = projectDir.toPath().resolve(".gradle").resolve("nodejs")
145+
.resolve("node-v${DEFAULT_NODE_VERSION}-linux-x64")
146+
147+
def task = project.tasks.create('simple', NpmTask)
148+
mockProjectApiHelperExec(task)
149+
task.args.set(["run", "command"])
150+
151+
when:
152+
project.evaluate()
153+
task.exec()
154+
155+
then:
156+
1 * execSpec.setIgnoreExitValue(false)
157+
1 * execSpec.setExecutable({ executable ->
158+
def expectedNodePath = nodeDir.resolve("bin").resolve("node")
159+
return fixAbsolutePath(executable) == expectedNodePath.toAbsolutePath().toString()
160+
})
161+
1 * execSpec.setArgs({ args ->
162+
def command = nodeDir
163+
.resolve("lib").resolve("node_modules").resolve("npm").resolve("bin")
164+
.resolve("npm-cli.js").toAbsolutePath().toString()
165+
return fixAbsolutePaths(args) == [command, "run", "command"]
166+
})
167+
}
168+
60169
def "exec npm task (download)"() {
61170
given:
62171
nodeExtension.resolvedPlatform.set(PlatformHelperKt.parsePlatform("Linux", "x86_64", {}))
@@ -121,6 +230,8 @@ class NpmTaskTest extends AbstractTaskTest {
121230
def "exec npm task with configured proxy"() {
122231
given:
123232
nodeExtension.resolvedPlatform.set(PlatformHelperKt.parsePlatform("Linux", "x86_64", {}))
233+
nodeExtension.environment.set([:])
234+
124235
GradleProxyHelper.setHttpsProxyHost("my-super-proxy.net")
125236
GradleProxyHelper.setHttpsProxyPort(11235)
126237

@@ -141,6 +252,8 @@ class NpmTaskTest extends AbstractTaskTest {
141252
def "exec npm task with configured proxy but disabled"() {
142253
given:
143254
nodeExtension.resolvedPlatform.set(PlatformHelperKt.parsePlatform("Linux", "x86_64", {}))
255+
nodeExtension.environment.set([:])
256+
144257
GradleProxyHelper.setHttpsProxyHost("my-super-proxy.net")
145258
GradleProxyHelper.setHttpsProxyPort(11235)
146259
nodeExtension.nodeProxySettings.set(ProxySettings.OFF)

‎src/test/groovy/com/github/gradle/node/npm/task/NpxTaskTest.groovy‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ class NpxTaskTest extends AbstractTaskTest {
1010
def "exec npx task"() {
1111
given:
1212
nodeExtension.resolvedPlatform.set(PlatformHelperKt.parsePlatform("Linux", "x86_64", {}))
13+
nodeExtension.environment.set([:])
1314

1415
def task = project.tasks.create('simple', NpxTask)
1516
mockProjectApiHelperExec(task)

‎src/test/groovy/com/github/gradle/node/task/NodeTaskTest.groovy‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ class NodeTaskTest extends AbstractTaskTest {
1818
def "exec node task"() {
1919
given:
2020
nodeExtension.resolvedPlatform.set(PlatformHelperKt.parsePlatform("Linux", "x86_64", {}))
21+
nodeExtension.environment.set([:])
2122
nodeExtension.download.set(false)
2223

2324
def task = project.tasks.create('simple', NodeTask)
@@ -93,6 +94,7 @@ class NodeTaskTest extends AbstractTaskTest {
9394
given:
9495
nodeExtension.resolvedPlatform.set(PlatformHelperKt.parsePlatform("Windows", "x86_64", {}))
9596
nodeExtension.download.set(false)
97+
nodeExtension.environment.set([:])
9698
9799
def task = project.tasks.create('simple', NodeTask)
98100
mockProjectApiHelperExec(task)

‎src/test/groovy/com/github/gradle/node/variant/VariantComputerTest.groovy‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,7 @@ class VariantComputerTest extends Specification {
157157
nodeExtension.resolvedPlatform.set(platform)
158158
nodeExtension.download.set(download)
159159
nodeExtension.npmVersion.set(npmVersion)
160+
nodeExtension.environment.set([:])
160161
161162
def variantComputer = new VariantComputer()
162163
@@ -208,6 +209,7 @@ class VariantComputerTest extends Specification {
208209
nodeExtension.resolvedPlatform.set(platform)
209210
nodeExtension.download.set(download)
210211
nodeExtension.npmVersion.set(npmVersion)
212+
nodeExtension.environment.set([:])
211213
212214
def variantComputer = new VariantComputer()
213215

0 commit comments

Comments
 (0)