Skip to content
This repository was archived by the owner on Feb 26, 2024. It is now read-only.

Commit e535671

Browse files
jeffsmale90jeffsmale90
committed
tidy up implementation, fix tests, don't accept filehandle as logging.file because then it's impossible for us to now whether we should close it
1 parent f8381cc commit e535671

File tree

8 files changed

+32
-86
lines changed

8 files changed

+32
-86
lines changed

‎src/chains/ethereum/console.log/tests/index.test.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ import {
2121
CONTRACT_NAME
2222
} from "./helpers";
2323

24-
describe.skip("@ganache/console.log", () => {
24+
describe("@ganache/console.log", () => {
2525
const logger = {
2626
log: () => {}
2727
};

‎src/chains/ethereum/ethereum/src/provider.ts‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -432,12 +432,10 @@ export class EthereumProvider
432432
this.#executor.stop();
433433
await this.#blockchain.stop();
434434

435-
// only call close on the logger if it's an instance of AsyncronousLogger
435+
// only need to do this if it's an `AsyncronousLogger`
436436
if ("getCompletionHandle" in this.#options.logging.logger) {
437-
// todo: maybe need to stop the logger from accepting new logs. This should work as is, because we wait for
438-
// any logs created before we call getCompletionHandle().
439437
await this.#options.logging.logger.getCompletionHandle();
440-
closeSync(+this.#options.logging.file);
438+
closeSync(this.#options.logging.file);
441439
}
442440

443441
this.#executor.end();

‎src/chains/ethereum/options/src/logging-options.ts‎

Lines changed: 10 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -75,11 +75,11 @@ export type LoggingConfig = {
7575

7676
/**
7777
* If you set this option, Ganache will write logs to a file located at the
78-
* specified path. You can provide a path, or numerical file descriptor.
78+
* specified path.
7979
*/
8080
readonly file: {
8181
type: number;
82-
rawType: number | PathLike;
82+
rawType: PathLike;
8383
};
8484
};
8585
};
@@ -108,18 +108,15 @@ export const LoggingOptions: Definitions<LoggingConfig> = {
108108
cliType: "boolean"
109109
},
110110
file: {
111-
normalize: (raw: number | PathLike): number => {
111+
normalize: (raw: PathLike): number => {
112112
let descriptor: number;
113-
if (typeof raw === "number") {
114-
descriptor = raw as number;
115-
} else {
116-
try {
117-
descriptor = openSync(raw as PathLike, "a");
118-
} catch (err) {
119-
throw new Error(
120-
`Failed to open log file ${raw}. Please check if the file path is valid and if the process has write permissions to the directory.`
121-
);
122-
}
113+
114+
try {
115+
descriptor = openSync(raw, "a");
116+
} catch (err) {
117+
throw new Error(
118+
`Failed to open log file ${raw}. Please check if the file path is valid and if the process has write permissions to the directory.`
119+
);
123120
}
124121
return descriptor;
125122
},

‎src/chains/ethereum/options/tests/logging-options.test.ts‎

Lines changed: 4 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -66,9 +66,7 @@ describe("EthereumOptionsConfig", () => {
6666
});
6767
try {
6868
assert(typeof options.logging.file === "number");
69-
assert.doesNotThrow(() =>
70-
closeSync(options.logging.file as number)
71-
);
69+
assert.doesNotThrow(() => closeSync(options.logging.file));
7270
} finally {
7371
await unlink(validFilePath);
7472
}
@@ -80,9 +78,7 @@ describe("EthereumOptionsConfig", () => {
8078
});
8179
try {
8280
assert(typeof options.logging.file === "number");
83-
assert.doesNotThrow(() =>
84-
closeSync(options.logging.file as number)
85-
);
81+
assert.doesNotThrow(() => closeSync(options.logging.file));
8682
} finally {
8783
await unlink(validFilePath);
8884
}
@@ -94,27 +90,7 @@ describe("EthereumOptionsConfig", () => {
9490
});
9591
try {
9692
assert(typeof options.logging.file === "number");
97-
assert.doesNotThrow(() =>
98-
closeSync(options.logging.file as number)
99-
);
100-
} finally {
101-
await unlink(validFilePath);
102-
}
103-
});
104-
105-
it("uses an existing descriptor if passed in", async () => {
106-
const fd = openSync(validFilePath, "a");
107-
108-
const options = EthereumOptionsConfig.normalize({
109-
logging: { file: fd }
110-
});
111-
112-
try {
113-
assert.strictEqual(options.logging.file, fd);
114-
assert(typeof options.logging.file === "number");
115-
assert.doesNotThrow(() =>
116-
closeSync(options.logging.file as number)
117-
);
93+
assert.doesNotThrow(() => closeSync(options.logging.file));
11894
} finally {
11995
await unlink(validFilePath);
12096
}
@@ -140,13 +116,12 @@ describe("EthereumOptionsConfig", () => {
140116
calls.push([message, ...params]);
141117
}
142118
};
143-
const descriptor = openSync(validFilePath, "a");
144119

145120
try {
146121
const options = EthereumOptionsConfig.normalize({
147122
logging: {
148123
logger,
149-
file: descriptor
124+
file: validFilePath
150125
}
151126
});
152127

‎src/chains/filecoin/options/src/logging-options.ts‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
import { normalize } from "./helpers";
21
import { Definitions } from "@ganache/options";
32
import { openSync, PathLike } from "fs";
43
import { Logger, createLogger } from "@ganache/utils";
@@ -26,24 +25,24 @@ export type LoggingConfig = {
2625

2726
/**
2827
* If you set this option, Ganache will write logs to a file located at the
29-
* specified path. You can provide a path, or numerical file descriptor.
28+
* specified path.
3029
*/
3130
readonly file: {
3231
type: number;
33-
rawType: number | PathLike;
32+
rawType: PathLike;
3433
};
3534
};
3635
};
3736

3837
export const LoggingOptions: Definitions<LoggingConfig> = {
3938
file: {
40-
normalize: (raw: number | PathLike): number => {
39+
normalize: (raw: PathLike): number => {
4140
let descriptor: number;
4241
if (typeof raw === "number") {
4342
descriptor = raw as number;
4443
} else {
4544
try {
46-
descriptor = openSync(raw as PathLike, "a");
45+
descriptor = openSync(raw, "a");
4746
} catch (err) {
4847
throw new Error(
4948
`Failed to open log file ${raw}. Please check if the file path is valid and if the process has write permissions to the directory.`

‎src/chains/filecoin/options/tests/logging-options.test.ts‎

Lines changed: 4 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -40,9 +40,7 @@ describe("FilecoinOptionsConfig", () => {
4040
});
4141
try {
4242
assert(typeof options.logging.file === "number");
43-
assert.doesNotThrow(() =>
44-
closeSync(options.logging.file as number)
45-
);
43+
assert.doesNotThrow(() => closeSync(options.logging.file));
4644
} finally {
4745
await unlink(validFilePath);
4846
}
@@ -54,9 +52,7 @@ describe("FilecoinOptionsConfig", () => {
5452
});
5553
try {
5654
assert(typeof options.logging.file === "number");
57-
assert.doesNotThrow(() =>
58-
closeSync(options.logging.file as number)
59-
);
55+
assert.doesNotThrow(() => closeSync(options.logging.file));
6056
} finally {
6157
await unlink(validFilePath);
6258
}
@@ -68,27 +64,7 @@ describe("FilecoinOptionsConfig", () => {
6864
});
6965
try {
7066
assert(typeof options.logging.file === "number");
71-
assert.doesNotThrow(() =>
72-
closeSync(options.logging.file as number)
73-
);
74-
} finally {
75-
await unlink(validFilePath);
76-
}
77-
});
78-
79-
it("uses an existing descriptor if passed in", async () => {
80-
const fd = openSync(validFilePath, "a");
81-
82-
const options = FilecoinOptionsConfig.normalize({
83-
logging: { file: fd }
84-
});
85-
86-
try {
87-
assert.strictEqual(options.logging.file, fd);
88-
assert(typeof options.logging.file === "number");
89-
assert.doesNotThrow(() =>
90-
closeSync(options.logging.file as number)
91-
);
67+
assert.doesNotThrow(() => closeSync(options.logging.file));
9268
} finally {
9369
await unlink(validFilePath);
9470
}
@@ -114,13 +90,12 @@ describe("FilecoinOptionsConfig", () => {
11490
calls.push([message, ...params]);
11591
}
11692
};
117-
const descriptor = openSync(validFilePath, "a");
11893

11994
try {
12095
const options = FilecoinOptionsConfig.normalize({
12196
logging: {
12297
logger,
123-
file: descriptor
98+
file: validFilePath
12499
}
125100
});
126101

‎src/packages/options/src/create.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ function fill(defaults: any, options: any, target: any, namespace: any) {
5151
const propDefinition = def[key];
5252
let value = namespaceOptions[key];
5353
if (value !== undefined) {
54-
const normalized = propDefinition.normalize(value, namespaceOptions);
54+
const normalized = propDefinition.normalize(value, config);
5555
if (normalized !== undefined) {
5656
checkForConflicts(
5757
key,
@@ -66,7 +66,7 @@ function fill(defaults: any, options: any, target: any, namespace: any) {
6666
const legacyName = propDefinition.legacyName || key;
6767
value = options[legacyName];
6868
if (value !== undefined) {
69-
const normalized = propDefinition.normalize(value, options);
69+
const normalized = propDefinition.normalize(value, config);
7070
if (normalized !== undefined) {
7171
checkForConflicts(
7272
key,
@@ -90,7 +90,7 @@ function fill(defaults: any, options: any, target: any, namespace: any) {
9090
const legacyName = propDefinition.legacyName || key;
9191
const value = options[legacyName];
9292
if (value !== undefined) {
93-
const normalized = propDefinition.normalize(value, options);
93+
const normalized = propDefinition.normalize(value, config);
9494
if (normalized !== undefined) {
9595
checkForConflicts(
9696
key,

‎src/packages/utils/src/things/logger.ts‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,10 +26,12 @@ export function createLogger(config: {
2626
} else {
2727
if (typeof config.file !== "number") {
2828
throw new Error(
29-
"`config.file` was not correctly noramlized to a file descriptor. This should not happen."
29+
`'config.file' was not correctly noramlized to a file descriptor. This should not happen. ${
30+
config.file
31+
}: ${typeof config.file}`
3032
);
3133
}
32-
const descriptor = config.file as number;
34+
const descriptor = config.file;
3335

3436
const diskLogFormatter = (message: any) => {
3537
const linePrefix = `${new Date().toISOString()} `;

0 commit comments

Comments
 (0)