Skip to content

Commit f1e2c41

Browse files
Copilotkobenguyent
andcommitted
Improve escapeHtml with recursive array flattening and DRY tests
- Refactored escapeHtml to use recursive flattening for deeply nested arrays - Removed duplicate helper functions from test file (DRY principle) - All 15 unit tests pass successfully - Verified deep nesting support (e.g., [['Level2', ['Level3']]]) Co-authored-by: kobenguyent <7845001+kobenguyent@users.noreply.github.com>
1 parent a5ec96a commit f1e2c41

File tree

3 files changed

+39
-65
lines changed

3 files changed

+39
-65
lines changed

docs/plugins.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -817,7 +817,7 @@ Enable it manually on each run via `-p` option:
817817
818818
## reportData
819819
820-
TypeScript: Explicitly type reportData arrays as any[] to avoid 'never' errors
820+
TypeScript: Explicitly type reportData arrays as any\[] to avoid 'never' errors
821821
822822
## retryFailedStep
823823

lib/plugin/htmlReporter.js

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1540,15 +1540,20 @@ module.exports = function (config) {
15401540
if (!text) return ''
15411541
// Convert non-string values to strings before escaping
15421542
if (typeof text !== 'string') {
1543-
// Handle arrays by joining with commas
1543+
// Handle arrays by recursively flattening and joining with commas
15441544
if (Array.isArray(text)) {
1545-
text = text.map(item => {
1546-
// Recursively flatten nested arrays
1547-
if (Array.isArray(item)) {
1548-
return item.join(', ')
1549-
}
1550-
return String(item)
1551-
}).join(', ')
1545+
// Recursive helper to flatten deeply nested arrays
1546+
const flattenArray = arr => {
1547+
return arr
1548+
.map(item => {
1549+
if (Array.isArray(item)) {
1550+
return flattenArray(item)
1551+
}
1552+
return String(item)
1553+
})
1554+
.join(', ')
1555+
}
1556+
text = flattenArray(text)
15521557
} else {
15531558
text = String(text)
15541559
}

test/unit/plugin/htmlReporter_test.js

Lines changed: 25 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -1,28 +1,33 @@
11
const { expect } = require('chai')
22

3-
describe('htmlReporter plugin', () => {
4-
describe('escapeHtml function', () => {
5-
// Helper function to simulate the escapeHtml behavior from htmlReporter.js
6-
function escapeHtml(text) {
7-
if (!text) return ''
8-
// Convert non-string values to strings before escaping
9-
if (typeof text !== 'string') {
10-
// Handle arrays by joining with commas
11-
if (Array.isArray(text)) {
12-
text = text.map(item => {
13-
// Recursively flatten nested arrays
3+
// Helper function to simulate the escapeHtml behavior from htmlReporter.js
4+
function escapeHtml(text) {
5+
if (!text) return ''
6+
// Convert non-string values to strings before escaping
7+
if (typeof text !== 'string') {
8+
// Handle arrays by recursively flattening and joining with commas
9+
if (Array.isArray(text)) {
10+
// Recursive helper to flatten deeply nested arrays
11+
const flattenArray = arr => {
12+
return arr
13+
.map(item => {
1414
if (Array.isArray(item)) {
15-
return item.join(', ')
15+
return flattenArray(item)
1616
}
1717
return String(item)
18-
}).join(', ')
19-
} else {
20-
text = String(text)
21-
}
18+
})
19+
.join(', ')
2220
}
23-
return text.replace(/&/g, '&amp;').replace(/</g, '&lt;').replace(/>/g, '&gt;').replace(/"/g, '&quot;').replace(/'/g, '&#39;')
21+
text = flattenArray(text)
22+
} else {
23+
text = String(text)
2424
}
25+
}
26+
return text.replace(/&/g, '&amp;').replace(/</g, '&lt;').replace(/>/g, '&gt;').replace(/"/g, '&quot;').replace(/'/g, '&#39;')
27+
}
2528

29+
describe('htmlReporter plugin', () => {
30+
describe('escapeHtml function', () => {
2631
it('should escape HTML special characters in strings', () => {
2732
const result = escapeHtml('<script>alert("xss")</script>')
2833
expect(result).to.include('&lt;script&gt;')
@@ -59,7 +64,7 @@ describe('htmlReporter plugin', () => {
5964
it('should handle null and undefined inputs', () => {
6065
const resultNull = escapeHtml(null)
6166
expect(resultNull).to.equal('')
62-
67+
6368
const resultUndefined = escapeHtml(undefined)
6469
expect(resultUndefined).to.equal('')
6570
})
@@ -89,24 +94,6 @@ describe('htmlReporter plugin', () => {
8994
})
9095

9196
describe('generateSystemInfoHtml function', () => {
92-
// Helper function to simulate escapeHtml
93-
function escapeHtml(text) {
94-
if (!text) return ''
95-
if (typeof text !== 'string') {
96-
if (Array.isArray(text)) {
97-
text = text.map(item => {
98-
if (Array.isArray(item)) {
99-
return item.join(', ')
100-
}
101-
return String(item)
102-
}).join(', ')
103-
} else {
104-
text = String(text)
105-
}
106-
}
107-
return text.replace(/&/g, '&amp;').replace(/</g, '&lt;').replace(/>/g, '&gt;').replace(/"/g, '&quot;').replace(/'/g, '&#39;')
108-
}
109-
11097
it('should handle system info with nested arrays', () => {
11198
// This tests the real-world scenario from the issue
11299
const systemInfo = {
@@ -137,12 +124,12 @@ describe('htmlReporter plugin', () => {
137124
expect(formatValue(systemInfo.osInfo)).to.include('Windows 10')
138125
expect(formatValue(systemInfo.cpuInfo)).to.include('12th Gen')
139126
expect(formatValue(systemInfo.chromeInfo)).to.include('142.0.7444.163')
140-
127+
141128
// The critical test: edgeInfo with nested array should not crash
142129
const edgeResult = formatValue(systemInfo.edgeInfo)
143130
expect(edgeResult).to.include('Chromium')
144131
expect(edgeResult).to.include('140.0.3485.54')
145-
132+
146133
expect(formatValue(systemInfo.safariInfo)).to.equal('N/A')
147134
})
148135

@@ -180,24 +167,6 @@ describe('htmlReporter plugin', () => {
180167
})
181168

182169
describe('edge cases', () => {
183-
// Helper function to simulate escapeHtml
184-
function escapeHtml(text) {
185-
if (!text) return ''
186-
if (typeof text !== 'string') {
187-
if (Array.isArray(text)) {
188-
text = text.map(item => {
189-
if (Array.isArray(item)) {
190-
return item.join(', ')
191-
}
192-
return String(item)
193-
}).join(', ')
194-
} else {
195-
text = String(text)
196-
}
197-
}
198-
return text.replace(/&/g, '&amp;').replace(/</g, '&lt;').replace(/>/g, '&gt;').replace(/"/g, '&quot;').replace(/'/g, '&#39;')
199-
}
200-
201170
it('should handle arrays with HTML content', () => {
202171
const result = escapeHtml(['<script>', ['alert("xss")'], '</script>'])
203172
expect(result).to.include('&lt;script&gt;')

0 commit comments

Comments
 (0)