Skip to content

Commit adb0049

Browse files
fix: use workspace root as cwd for ECS
* Set cwd for ECS process * use workspace root as default cwd for ECS * add unit tests for DefaultProcessRunner --------- Co-authored-by: Cezary Marcinik <cezary.marcinik@o2.pl>
1 parent 7a7f6e2 commit adb0049

File tree

5 files changed

+170
-26
lines changed

5 files changed

+170
-26
lines changed

src/core/ConfigLoader.ts

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ export interface EcsConfig {
55
ecsPath: string;
66
configPath: string;
77
onSave: boolean;
8+
workspaceRoot: string;
89
}
910

1011
export class ConfigLoader {
@@ -14,9 +15,12 @@ export class ConfigLoader {
1415

1516
const ecsPath = ConfigLoader.resolveEcsPath(workspaceFolder, config);
1617
const configPath = ConfigLoader.resolveConfigPath(workspaceFolder, config);
18+
1719
const onSave = config.get<boolean>("onsave", false);
1820

19-
return { ecsPath, configPath, onSave };
21+
const workspaceRoot = workspaceFolder;
22+
23+
return { ecsPath, configPath, onSave, workspaceRoot };
2024
}
2125

2226
public static loadConfigForFile(resource: vscode.Uri): EcsConfig {
@@ -27,7 +31,9 @@ export class ConfigLoader {
2731
const configPath = ConfigLoader.resolveConfigPath(workspaceFolder, config);
2832
const onSave = config.get<boolean>("onsave", false);
2933

30-
return { ecsPath, configPath, onSave };
34+
const workspaceRoot = workspaceFolder;
35+
36+
return { ecsPath, configPath, onSave, workspaceRoot };
3137
}
3238

3339
private static resolvePath(
@@ -44,7 +50,6 @@ export class ConfigLoader {
4450
} else {
4551
filePath = path.join(workspaceFolder, defaultRelativePath);
4652
}
47-
4853
return filePath;
4954
}
5055

@@ -84,24 +89,23 @@ export class ConfigLoader {
8489

8590
const activeEditor = vscode.window.activeTextEditor;
8691
if (activeEditor) {
87-
const workspaceFolder = vscode.workspace.getWorkspaceFolder(
92+
const folder = vscode.workspace.getWorkspaceFolder(
8893
activeEditor.document.uri
8994
);
90-
if (workspaceFolder) {
91-
return workspaceFolder.uri.fsPath;
95+
if (folder) {
96+
return folder.uri.fsPath;
9297
}
9398
}
9499

95100
return workspaceFolders[0].uri.fsPath;
96101
}
97102

98103
private static getWorkspaceFolderForFile(resource: vscode.Uri): string {
99-
const workspaceFolder = vscode.workspace.getWorkspaceFolder(resource);
100-
if (workspaceFolder) {
101-
return workspaceFolder.uri.fsPath;
102-
} else {
104+
const folder = vscode.workspace.getWorkspaceFolder(resource);
105+
if (!folder) {
103106
vscode.window.showErrorMessage("Workspace folder not found");
104107
throw new Error("Workspace folder not found");
105108
}
109+
return folder.uri.fsPath;
106110
}
107111
}

src/core/EcsCommandRunner.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,8 @@ export class EcsCommandRunner {
1616
await this.processRunner.runEcsCheck(
1717
this.ecsConfig.ecsPath,
1818
this.ecsConfig.configPath,
19-
tempFilePath
19+
tempFilePath,
20+
this.ecsConfig.workspaceRoot
2021
);
2122
const newContent = await this.fileHandler.readTempFile(tempFilePath);
2223
return newContent;
@@ -29,7 +30,8 @@ export class EcsCommandRunner {
2930
await this.processRunner.runEcsCheck(
3031
this.ecsConfig.ecsPath,
3132
this.ecsConfig.configPath,
32-
filePath
33+
filePath,
34+
this.ecsConfig.workspaceRoot
3335
);
3436
}
3537
}

src/core/ProcessRunner.ts

Lines changed: 20 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -4,38 +4,44 @@ export interface ProcessRunner {
44
runEcsCheck(
55
ecsPath: string,
66
configPath: string,
7-
targetPath: string
7+
targetPath: string,
8+
workspaceRoot: string
89
): Promise<void>;
910
}
1011

1112
export class DefaultProcessRunner implements ProcessRunner {
1213
runEcsCheck(
1314
ecsPath: string,
1415
configPath: string,
15-
targetPath: string
16+
targetPath: string,
17+
workspaceRoot: string
1618
): Promise<void> {
1719
return new Promise<void>((resolve, reject) => {
18-
const process = spawn("php", [
19-
ecsPath,
20-
"check",
21-
targetPath,
22-
"--fix",
23-
`--config=${configPath}`,
24-
"--no-progress-bar",
25-
"--quiet",
26-
]);
20+
const processHandle = spawn(
21+
"php",
22+
[
23+
ecsPath,
24+
"check",
25+
targetPath,
26+
"--fix",
27+
`--config=${configPath}`,
28+
"--no-progress-bar",
29+
"--quiet",
30+
],
31+
{ cwd: workspaceRoot }
32+
);
2733

2834
let stderrData = "";
2935

30-
process.stderr.on("data", (data) => {
36+
processHandle.stderr.on("data", (data) => {
3137
stderrData += data.toString();
3238
});
3339

34-
process.on("error", (err) => {
40+
processHandle.on("error", (err) => {
3541
reject(err);
3642
});
3743

38-
process.on("close", (code) => {
44+
processHandle.on("close", (code) => {
3945
if (code === 0) {
4046
resolve();
4147
} else {

src/test/unit/core/EcsCommandRunner.test.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ suite("EcsCommandRunner", () => {
2424
ecsPath: "/path/to/ecs",
2525
configPath: "/path/to/ecs.php",
2626
onSave: false,
27+
workspaceRoot: "./",
2728
};
2829

2930
tempFileProviderMock = sinon.createStubInstance(DefaultTempFileProvider);
Lines changed: 131 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,131 @@
1+
import * as assert from "assert";
2+
import * as sinon from "sinon";
3+
import { DefaultProcessRunner } from "../../../core/ProcessRunner";
4+
5+
suite("DefaultProcessRunner", () => {
6+
let spawnStub: sinon.SinonStub;
7+
8+
setup(() => {
9+
spawnStub = sinon.stub(require("child_process"), "spawn");
10+
});
11+
12+
teardown(() => {
13+
sinon.restore();
14+
});
15+
16+
test("should spawn php process with correct arguments and cwd", async () => {
17+
const fakeChildProcess = {
18+
stdout: { on: sinon.stub() },
19+
stderr: { on: sinon.stub() },
20+
on: sinon.stub(),
21+
};
22+
spawnStub.returns(fakeChildProcess);
23+
24+
fakeChildProcess.on.withArgs("close").yields(0);
25+
26+
const runner = new DefaultProcessRunner();
27+
28+
await runner.runEcsCheck(
29+
"/path/to/ecs",
30+
"/path/to/ecs.php",
31+
"/some/target.php",
32+
"/my/workspace/root"
33+
);
34+
35+
assert.strictEqual(spawnStub.callCount, 1, "spawn should be called once");
36+
37+
const [command, args, options] = spawnStub.firstCall.args;
38+
39+
assert.strictEqual(command, "php", "Should call php");
40+
41+
assert.deepStrictEqual(
42+
args,
43+
[
44+
"/path/to/ecs",
45+
"check",
46+
"/some/target.php",
47+
"--fix",
48+
"--config=/path/to/ecs.php",
49+
"--no-progress-bar",
50+
"--quiet",
51+
],
52+
"Should pass correct arguments to ECS"
53+
);
54+
55+
assert.strictEqual(
56+
options.cwd,
57+
"/my/workspace/root",
58+
"Should use the workspaceRoot as cwd"
59+
);
60+
});
61+
62+
test("should resolve if exit code is 0", async () => {
63+
const fakeChildProcess = {
64+
stdout: { on: sinon.stub() },
65+
stderr: { on: sinon.stub() },
66+
on: sinon.stub(),
67+
};
68+
spawnStub.returns(fakeChildProcess);
69+
70+
fakeChildProcess.on.withArgs("close").yields(0);
71+
72+
const runner = new DefaultProcessRunner();
73+
await assert.doesNotReject(
74+
runner.runEcsCheck(
75+
"/path/to/ecs",
76+
"/path/to/ecs.php",
77+
"file.php",
78+
"/my/ws"
79+
)
80+
);
81+
});
82+
83+
test("should reject if exit code is non-zero", async () => {
84+
const fakeChildProcess = {
85+
stdout: { on: sinon.stub() },
86+
stderr: { on: sinon.stub() },
87+
on: sinon.stub(),
88+
};
89+
spawnStub.returns(fakeChildProcess);
90+
91+
fakeChildProcess.on.withArgs("close").yields(1);
92+
93+
fakeChildProcess.stderr.on
94+
.withArgs("data")
95+
.callsArgWith(1, "Some ECS error");
96+
97+
const runner = new DefaultProcessRunner();
98+
await assert.rejects(
99+
runner.runEcsCheck(
100+
"/path/to/ecs",
101+
"/path/to/ecs.php",
102+
"file.php",
103+
"/my/ws"
104+
),
105+
/ECS exited with code 1\nSome ECS error/
106+
);
107+
});
108+
109+
test("should reject on process error event", async () => {
110+
const fakeChildProcess = {
111+
stdout: { on: sinon.stub() },
112+
stderr: { on: sinon.stub() },
113+
on: sinon.stub(),
114+
};
115+
spawnStub.returns(fakeChildProcess);
116+
117+
const testError = new Error("Spawn error");
118+
fakeChildProcess.on.withArgs("error").yields(testError);
119+
120+
const runner = new DefaultProcessRunner();
121+
await assert.rejects(
122+
runner.runEcsCheck(
123+
"/path/to/ecs",
124+
"/path/to/ecs.php",
125+
"file.php",
126+
"/my/ws"
127+
),
128+
/Spawn error/
129+
);
130+
});
131+
});

0 commit comments

Comments
 (0)