Skip to content

Commit 7c2eabf

Browse files
jdmarshalljdmarshall
authored
Align bars in chart reporter when labels are over-long (#115)
* chore: Use padEnd for text reporter This makes it consistent with pretty.js * chore: Grouping tests. * feat: Keep bars aligned when test names are longer than the requested length.
1 parent 4985b35 commit 7c2eabf

File tree

3 files changed

+154
-114
lines changed

3 files changed

+154
-114
lines changed

‎lib/reporter/chart.js‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,9 @@ function chartReport(results, options = { labelWidth: 45, printHeader: true }) {
116116
process.stdout.write(styleText("bold", "\nSummary (vs. baseline):\n"));
117117
}
118118

119+
const maxNameLength = Math.max(...results.map((r) => r.name.length));
120+
const columnWidth = Math.max(maxNameLength, options.labelWidth ?? 45);
121+
119122
for (const result of results) {
120123
let comment = "";
121124

@@ -129,8 +132,6 @@ function chartReport(results, options = { labelWidth: 45, printHeader: true }) {
129132
}
130133
}
131134

132-
const columnWidth = options.labelWidth ?? 45;
133-
134135
drawBar(
135136
result.name.padEnd(columnWidth),
136137
result[primaryMetric],

‎lib/reporter/text.js‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -85,12 +85,9 @@ function textReport(results, options = {}) {
8585
const maxNameLength = Math.max(...sortedResults.map((r) => r.name.length));
8686

8787
for (const result of sortedResults) {
88-
const namePart = ` ${result.name}`;
88+
const namePart = ` ${result.name.padEnd(maxNameLength)} `;
8989
process.stdout.write(namePart);
9090

91-
const padding = maxNameLength - result.name.length + 2;
92-
process.stdout.write(" ".repeat(padding));
93-
9491
if (result.baseline) {
9592
process.stdout.write(styleText("magenta", "(baseline)"));
9693
} else if (result.comparison !== undefined) {

‎test/reporter.js‎

Lines changed: 150 additions & 108 deletions
Original file line numberDiff line numberDiff line change
@@ -14,143 +14,185 @@ const {
1414

1515
const { analyze } = require("../lib/utils/analyze.js");
1616

17-
describe("chartReport outputs benchmark results as a bar chart", async (t) => {
18-
let output = "";
19-
20-
before(async () => {
21-
const originalStdoutWrite = process.stdout.write;
22-
process.stdout.write = (data) => {
23-
output += data;
24-
};
17+
describe("chartReport", () => {
18+
describe("outputs benchmark results as a bar chart", async (t) => {
19+
let output = "";
2520

26-
const suite = new Suite({
27-
reporter: chartReport,
28-
});
21+
before(async () => {
22+
const originalStdoutWrite = process.stdout.write;
23+
process.stdout.write = (data) => {
24+
output += data;
25+
};
2926

30-
suite
31-
.add("single with matcher", () => {
32-
const pattern = /[123]/g;
33-
const replacements = { 1: "a", 2: "b", 3: "c" };
34-
const subject = "123123123123123123123123123123123123123123123123";
35-
const r = subject.replace(pattern, (m) => replacements[m]);
36-
assert.ok(r);
37-
})
38-
.add("multiple replaces", () => {
39-
const subject = "123123123123123123123123123123123123123123123123";
40-
const r = subject
41-
.replace(/1/g, "a")
42-
.replace(/2/g, "b")
43-
.replace(/3/g, "c");
44-
assert.ok(r);
27+
const suite = new Suite({
28+
reporter: chartReport,
4529
});
46-
await suite.run();
4730

48-
process.stdout.write = originalStdoutWrite;
49-
});
31+
suite
32+
.add("single with matcher", () => {
33+
const pattern = /[123]/g;
34+
const replacements = { 1: "a", 2: "b", 3: "c" };
35+
const subject = "123123123123123123123123123123123123123123123123";
36+
const r = subject.replace(pattern, (m) => replacements[m]);
37+
assert.ok(r);
38+
})
39+
.add("multiple replaces", () => {
40+
const subject = "123123123123123123123123123123123123123123123123";
41+
const r = subject
42+
.replace(/1/g, "a")
43+
.replace(/2/g, "b")
44+
.replace(/3/g, "c");
45+
assert.ok(r);
46+
});
47+
await suite.run();
5048

51-
it("should include bar chart chars", () => {
52-
assert.ok(output.includes("█"));
53-
});
49+
process.stdout.write = originalStdoutWrite;
50+
});
5451

55-
it("should include ops/sec", () => {
56-
assert.ok(output.includes("ops/sec"));
57-
});
52+
it("should include bar chart chars", () => {
53+
assert.ok(output.includes("█"));
54+
});
5855

59-
it("should include benchmark names", () => {
60-
assert.ok(output.includes("single with matcher"));
61-
assert.ok(output.includes("multiple replaces"));
62-
});
56+
it("should include ops/sec", () => {
57+
assert.ok(output.includes("ops/sec"));
58+
});
6359

64-
it("should include sample count", () => {
65-
assert.ok(output.includes("samples"));
66-
});
60+
it("should include benchmark names", () => {
61+
assert.ok(output.includes("single with matcher"));
62+
assert.ok(output.includes("multiple replaces"));
63+
});
6764

68-
it("should include Node.js version", () => {
69-
const regex = /Node\.js version: v\d+\.\d+\.\d+/;
65+
it("should include sample count", () => {
66+
assert.ok(output.includes("samples"));
67+
});
68+
69+
it("should include Node.js version", () => {
70+
const regex = /Node\.js version: v\d+\.\d+\.\d+/;
7071

71-
assert.ok(output.match(regex));
72+
assert.ok(output.match(regex));
73+
});
7274
});
73-
});
7475

75-
describe("chartReport respects reporterOptions.printHeader", async (t) => {
76-
let outputWithHeader = "";
77-
let outputWithoutHeader = "";
76+
describe("with long names", async (t) => {
77+
let output = "";
7878

79-
before(async () => {
80-
const originalStdoutWrite = process.stdout.write;
79+
before(async () => {
80+
const originalStdoutWrite = process.stdout.write;
81+
process.stdout.write = (data) => {
82+
output += data;
83+
};
8184

82-
// Test with default settings (printHeader: true)
83-
process.stdout.write = (data) => {
84-
outputWithHeader += data;
85-
};
85+
const suite = new Suite({
86+
reporter: chartReport,
87+
});
8688

87-
const suiteWithHeader = new Suite({
88-
reporter: chartReport,
89-
reporterOptions: {
90-
printHeader: true,
91-
},
89+
suite
90+
.add("single with matcher looooooooooooooooooooooooooong", () => {
91+
const pattern = /[123]/g;
92+
const replacements = { 1: "a", 2: "b", 3: "c" };
93+
const subject = "123123123123123123123123123123123123123123123123";
94+
const r = subject.replace(pattern, (m) => replacements[m]);
95+
assert.ok(r);
96+
})
97+
.add("multiple replaces", () => {
98+
const subject = "123123123123123123123123123123123123123123123123";
99+
const r = subject
100+
.replace(/1/g, "a")
101+
.replace(/2/g, "b")
102+
.replace(/3/g, "c");
103+
assert.ok(r);
104+
});
105+
await suite.run();
106+
107+
process.stdout.write = originalStdoutWrite;
92108
});
93109

94-
suiteWithHeader.add("test benchmark", () => {
95-
const a = 1 + 1;
96-
assert.strictEqual(a, 2);
110+
it("should pad out benchmark names", () => {
111+
assert.ok(output.includes("oong | "));
112+
assert.ok(output.includes("multiple replaces".padEnd(51)));
97113
});
98-
await suiteWithHeader.run();
114+
});
99115

100-
// Test with printHeader: false
101-
outputWithoutHeader = "";
102-
process.stdout.write = (data) => {
103-
outputWithoutHeader += data;
104-
};
116+
describe("respects reporterOptions.printHeader", async (t) => {
117+
let outputWithHeader = "";
118+
let outputWithoutHeader = "";
105119

106-
const suiteWithoutHeader = new Suite({
107-
reporter: chartReport,
108-
reporterOptions: {
109-
printHeader: false,
110-
},
111-
});
120+
before(async () => {
121+
const originalStdoutWrite = process.stdout.write;
112122

113-
suiteWithoutHeader.add("test benchmark", () => {
114-
const a = 1 + 1;
115-
assert.strictEqual(a, 2);
116-
});
117-
await suiteWithoutHeader.run();
123+
// Test with default settings (printHeader: true)
124+
process.stdout.write = (data) => {
125+
outputWithHeader += data;
126+
};
118127

119-
process.stdout.write = originalStdoutWrite;
120-
});
128+
const suiteWithHeader = new Suite({
129+
reporter: chartReport,
130+
reporterOptions: {
131+
printHeader: true,
132+
},
133+
});
121134

122-
it("should include Node.js version when printHeader is true", () => {
123-
const regex = /Node\.js version: v\d+\.\d+\.\d+/;
124-
assert.ok(outputWithHeader.match(regex));
125-
});
135+
suiteWithHeader.add("test benchmark", () => {
136+
const a = 1 + 1;
137+
assert.strictEqual(a, 2);
138+
});
139+
await suiteWithHeader.run();
126140

127-
it("should include Platform when printHeader is true", () => {
128-
assert.ok(outputWithHeader.includes("Platform:"));
129-
});
141+
// Test with printHeader: false
142+
outputWithoutHeader = "";
143+
process.stdout.write = (data) => {
144+
outputWithoutHeader += data;
145+
};
130146

131-
it("should include CPU Cores when printHeader is true", () => {
132-
assert.ok(outputWithHeader.includes("CPU Cores:"));
133-
});
147+
const suiteWithoutHeader = new Suite({
148+
reporter: chartReport,
149+
reporterOptions: {
150+
printHeader: false,
151+
},
152+
});
134153

135-
it("should NOT include Node.js version when printHeader is false", () => {
136-
const regex = /Node\.js version: v\d+\.\d+\.\d+/;
137-
assert.ok(!outputWithoutHeader.match(regex));
138-
});
154+
suiteWithoutHeader.add("test benchmark", () => {
155+
const a = 1 + 1;
156+
assert.strictEqual(a, 2);
157+
});
158+
await suiteWithoutHeader.run();
139159

140-
it("should NOT include Platform when printHeader is false", () => {
141-
assert.ok(!outputWithoutHeader.includes("Platform:"));
142-
});
160+
process.stdout.write = originalStdoutWrite;
161+
});
143162

144-
it("should NOT include CPU Cores when printHeader is false", () => {
145-
assert.ok(!outputWithoutHeader.includes("CPU Cores:"));
146-
});
163+
it("should include Node.js version when printHeader is true", () => {
164+
const regex = /Node\.js version: v\d+\.\d+\.\d+/;
165+
assert.ok(outputWithHeader.match(regex));
166+
});
167+
168+
it("should include Platform when printHeader is true", () => {
169+
assert.ok(outputWithHeader.includes("Platform:"));
170+
});
171+
172+
it("should include CPU Cores when printHeader is true", () => {
173+
assert.ok(outputWithHeader.includes("CPU Cores:"));
174+
});
175+
176+
it("should NOT include Node.js version when printHeader is false", () => {
177+
const regex = /Node\.js version: v\d+\.\d+\.\d+/;
178+
assert.ok(!outputWithoutHeader.match(regex));
179+
});
180+
181+
it("should NOT include Platform when printHeader is false", () => {
182+
assert.ok(!outputWithoutHeader.includes("Platform:"));
183+
});
147184

148-
it("should still include benchmark data with or without header", () => {
149-
// Both outputs should still have benchmark bars and results
150-
assert.ok(outputWithHeader.includes("█"));
151-
assert.ok(outputWithoutHeader.includes("█"));
152-
assert.ok(outputWithHeader.includes("test benchmark"));
153-
assert.ok(outputWithoutHeader.includes("test benchmark"));
185+
it("should NOT include CPU Cores when printHeader is false", () => {
186+
assert.ok(!outputWithoutHeader.includes("CPU Cores:"));
187+
});
188+
189+
it("should still include benchmark data with or without header", () => {
190+
// Both outputs should still have benchmark bars and results
191+
assert.ok(outputWithHeader.includes("█"));
192+
assert.ok(outputWithoutHeader.includes("█"));
193+
assert.ok(outputWithHeader.includes("test benchmark"));
194+
assert.ok(outputWithoutHeader.includes("test benchmark"));
195+
});
154196
});
155197
});
156198

0 commit comments

Comments
 (0)