Skip to content

Commit e04b44f

Browse files
vladlearnsvladlearns
committed
normalize annotations to guarantee type is always a string
1 parent 38b3ba0 commit e04b44f

File tree

4 files changed

+95
-58
lines changed

4 files changed

+95
-58
lines changed

‎packages/html-reporter/src/filter.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -204,7 +204,7 @@ function cacheSearchValues(test: TestCaseSummary & { [searchValuesSymbol]?: Sear
204204
line: String(test.location.line),
205205
column: String(test.location.column),
206206
labels: test.tags.map(tag => tag.toLowerCase()),
207-
annotations: test.annotations.map(a => (a.type || '').toLowerCase() + '=' + a.description?.toLocaleLowerCase())
207+
annotations: test.annotations.map(a => a.type.toLowerCase() + '=' + a.description?.toLocaleLowerCase())
208208
};
209209
test[searchValuesSymbol] = searchValues;
210210
return searchValues;

‎packages/playwright/src/common/testType.ts‎

Lines changed: 33 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ import { expect } from '../matchers/expect';
2323
import { wrapFunctionWithLocation } from '../transform/transform';
2424

2525
import type { FixturesWithLocation } from './config';
26-
import type { Fixtures, TestDetails, TestStepInfo, TestType } from '../../types/test';
26+
import type { Fixtures, TestAnnotation, TestDetails, TestStepInfo, TestType } from '../../types/test';
2727
import type { Location } from '../../types/testReporter';
2828

2929
const testTypeSymbol = Symbol('testType');
@@ -309,24 +309,42 @@ function throwIfRunningInsideJest() {
309309
}
310310
}
311311

312+
export function normalizeAnnotation(annotation: any): TestAnnotation {
313+
if (typeof annotation !== 'object' || annotation === null || Array.isArray(annotation)) {
314+
return {
315+
type: 'invalid-annotation',
316+
description: String(annotation)
317+
};
318+
}
319+
320+
const type = ('type' in annotation && annotation.type !== null && annotation.type !== undefined)
321+
? String(annotation.type)
322+
: 'unknown';
323+
324+
let description = annotation.description;
325+
if (description !== undefined && description !== null) {
326+
const descType = typeof description;
327+
if (descType === 'object' || descType === 'function')
328+
description = String(description);
329+
}
330+
331+
const result: TestAnnotation = { type };
332+
if (description !== undefined)
333+
result.description = description;
334+
if (annotation.location)
335+
result.location = annotation.location;
336+
337+
return result;
338+
}
339+
312340
function validateTestDetails(details: TestDetails, location: Location) {
313341
const originalAnnotations = Array.isArray(details.annotation) ? details.annotation : (details.annotation ? [details.annotation] : []);
314342

315-
for (const annotation of originalAnnotations) {
316-
if (typeof annotation !== 'object' || annotation === null || Array.isArray(annotation))
317-
throw new Error(`Annotation must be an object, got ${typeof annotation} instead.`);
318-
if (!('type' in annotation) || typeof annotation.type !== 'string')
319-
throw new Error(`Annotation must have a "type" property of type string.`);
320-
// Allow description to be omitted, undefined, or any primitive type (string, number, boolean)
321-
// but not objects or arrays which would be invalid
322-
if ('description' in annotation && annotation.description !== undefined && annotation.description !== null) {
323-
const descType = typeof annotation.description;
324-
if (descType === 'object' || descType === 'function')
325-
throw new Error(`Annotation "description" must be a primitive value, got ${descType} instead.`);
326-
}
327-
}
343+
const annotations = originalAnnotations.map(annotation => {
344+
const normalized = normalizeAnnotation(annotation);
345+
return { ...normalized, location };
346+
});
328347

329-
const annotations = originalAnnotations.map(annotation => ({ ...annotation, location }));
330348
const tags = Array.isArray(details.tag) ? details.tag : (details.tag ? [details.tag] : []);
331349
for (const tag of tags) {
332350
if (tag[0] !== '@')

‎packages/playwright/src/worker/testInfo.ts‎

Lines changed: 3 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import { addSuffixToFilePath, filteredStackTrace, getContainedPath, normalizeAnd
2424
import { TestTracing } from './testTracing';
2525
import { testInfoError } from './util';
2626
import { wrapFunctionWithLocation } from '../transform/transform';
27+
import { normalizeAnnotation } from '../common/testType';
2728

2829
import type { RunnableDescription } from './timeoutManager';
2930
import type { FullProject, TestInfo, TestStatus, TestStepInfo, TestAnnotation } from '../../types/test';
@@ -216,20 +217,8 @@ export class TestInfoImpl implements TestInfo {
216217

217218
this._annotationsPush = this.annotations.push.bind(this.annotations);
218219
this.annotations.push = (...annotations: TestAnnotation[]) => {
219-
for (const annotation of annotations) {
220-
if (typeof annotation !== 'object' || annotation === null || Array.isArray(annotation))
221-
throw new Error(`Annotation must be an object, got ${typeof annotation} instead.`);
222-
if (!('type' in annotation) || typeof annotation.type !== 'string')
223-
throw new Error(`Annotation must have a "type" property of type string.`);
224-
// Allow description to be omitted, undefined, or any primitive type (string, number, boolean)
225-
// but not objects or arrays which would be invalid
226-
if ('description' in annotation && annotation.description !== undefined && annotation.description !== null) {
227-
const descType = typeof annotation.description;
228-
if (descType === 'object' || descType === 'function')
229-
throw new Error(`Annotation "description" must be a primitive value, got ${descType} instead.`);
230-
}
231-
}
232-
return this._annotationsPush(...annotations);
220+
const normalized = annotations.map(a => normalizeAnnotation(a));
221+
return this._annotationsPush(...normalized);
233222
};
234223

235224
this._attachmentsPush = this.attachments.push.bind(this.attachments);

‎tests/playwright-test/reporter-html.spec.ts‎

Lines changed: 58 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -942,113 +942,143 @@ for (const useIntermediateMergeReport of [true, false] as const) {
942942
await expect(page.locator('.test-case-annotation')).toHaveText('issue: 0');
943943
});
944944

945-
test('should validate annotation structure at test declaration', async ({ runInlineTest }) => {
945+
test('should normalize invalid annotation at test declaration', async ({ runInlineTest, page, showReport }) => {
946946
const result = await runInlineTest({
947947
'a.test.js': `
948948
import { test, expect } from '@playwright/test';
949949
950-
// Invalid: annotation is a string instead of object
950+
// Invalid: annotation is a string instead of object - will be normalized
951951
test('test with string annotation', { annotation: 'bug' }, async ({}) => {
952952
expect(1).toBe(1);
953953
});
954954
`,
955955
}, { reporter: 'dot,html' }, { PLAYWRIGHT_HTML_OPEN: 'never' });
956-
expect(result.exitCode).toBe(1);
957-
expect(result.output).toContain('Annotation must be an object');
956+
expect(result.exitCode).toBe(0);
957+
expect(result.passed).toBe(1);
958+
959+
await showReport();
960+
await page.getByText('test with string annotation').click();
961+
// String annotation is normalized to { type: 'invalid-annotation', description: 'bug' }
962+
await expect(page.locator('.test-case-annotation')).toHaveText('invalid-annotation: bug');
958963
});
959964

960-
test('should validate annotation has type property', async ({ runInlineTest }) => {
965+
test('should normalize annotation with missing type property', async ({ runInlineTest, page, showReport }) => {
961966
const result = await runInlineTest({
962967
'a.test.js': `
963968
import { test, expect } from '@playwright/test';
964969
965-
// Invalid: annotation missing type property
970+
// Invalid: annotation missing type property - will be normalized to type: 'unknown'
966971
test('test with missing type', { annotation: { description: 'TEST-58xxx' } }, async ({}) => {
967972
expect(1).toBe(1);
968973
});
969974
`,
970975
}, { reporter: 'dot,html' }, { PLAYWRIGHT_HTML_OPEN: 'never' });
971-
expect(result.exitCode).toBe(1);
972-
expect(result.output).toContain('Annotation must have a "type" property of type string');
976+
expect(result.exitCode).toBe(0);
977+
expect(result.passed).toBe(1);
978+
979+
await showReport();
980+
await page.getByText('test with missing type').click();
981+
// Missing type is normalized to 'unknown'
982+
await expect(page.locator('.test-case-annotation')).toHaveText('unknown: TEST-58xxx');
973983
});
974984

975-
test('should validate annotation type is a string', async ({ runInlineTest }) => {
985+
test('should normalize annotation with non-string type', async ({ runInlineTest, page, showReport }) => {
976986
const result = await runInlineTest({
977987
'a.test.js': `
978988
import { test, expect } from '@playwright/test';
979989
980-
// Invalid: annotation type is not a string
990+
// Invalid: annotation type is not a string - will be stringified
981991
test('test with non-string type', { annotation: { type: 123, description: 'desc' } }, async ({}) => {
982992
expect(1).toBe(1);
983993
});
984994
`,
985995
}, { reporter: 'dot,html' }, { PLAYWRIGHT_HTML_OPEN: 'never' });
986-
expect(result.exitCode).toBe(1);
987-
expect(result.output).toContain('Annotation must have a "type" property of type string');
996+
expect(result.exitCode).toBe(0);
997+
expect(result.passed).toBe(1);
998+
999+
await showReport();
1000+
await page.getByText('test with non-string type').click();
1001+
await expect(page.locator('.test-case-annotation')).toHaveText('123: desc');
9881002
});
9891003

990-
test('should validate runtime annotation pushed via test.info() is an object', async ({ runInlineTest }) => {
1004+
test('should normalize runtime annotation that is not an object', async ({ runInlineTest, page, showReport }) => {
9911005
const result = await runInlineTest({
9921006
'a.test.js': `
9931007
import { test, expect } from '@playwright/test';
9941008
9951009
test('test with invalid runtime annotation', async ({}) => {
996-
// Invalid: pushing a string instead of object
1010+
// Invalid: pushing a string instead of object - will be normalized
9971011
test.info().annotations.push('bug');
9981012
expect(1).toBe(1);
9991013
});
10001014
`,
10011015
}, { reporter: 'dot,html' }, { PLAYWRIGHT_HTML_OPEN: 'never' });
1002-
expect(result.exitCode).toBe(1);
1003-
expect(result.output).toContain('Annotation must be an object');
1016+
expect(result.exitCode).toBe(0);
1017+
expect(result.passed).toBe(1);
1018+
1019+
await showReport();
1020+
await page.getByText('test with invalid runtime annotation').click();
1021+
await expect(page.locator('.test-case-annotation')).toHaveText('invalid-annotation: bug');
10041022
});
10051023

1006-
test('should validate runtime annotation has type property', async ({ runInlineTest }) => {
1024+
test('should normalize runtime annotation with missing type property', async ({ runInlineTest, page, showReport }) => {
10071025
const result = await runInlineTest({
10081026
'a.test.js': `
10091027
import { test, expect } from '@playwright/test';
10101028
10111029
test('test with runtime annotation missing type', async ({}) => {
1012-
// Invalid: missing type property
1030+
// Invalid: missing type property - will be normalized to 'unknown'
10131031
test.info().annotations.push({ description: 'TEST-123' });
10141032
expect(1).toBe(1);
10151033
});
10161034
`,
10171035
}, { reporter: 'dot,html' }, { PLAYWRIGHT_HTML_OPEN: 'never' });
1018-
expect(result.exitCode).toBe(1);
1019-
expect(result.output).toContain('Annotation must have a "type" property of type string');
1036+
expect(result.exitCode).toBe(0);
1037+
expect(result.passed).toBe(1);
1038+
1039+
await showReport();
1040+
await page.getByText('test with runtime annotation missing type').click();
1041+
await expect(page.locator('.test-case-annotation')).toHaveText('unknown: TEST-123');
10201042
});
10211043

1022-
test('should validate runtime annotation type is a string', async ({ runInlineTest }) => {
1044+
test('should normalize runtime annotation with non-string type', async ({ runInlineTest, page, showReport }) => {
10231045
const result = await runInlineTest({
10241046
'a.test.js': `
10251047
import { test, expect } from '@playwright/test';
10261048
10271049
test('test with runtime annotation non-string type', async ({}) => {
1028-
// Invalid: type is not a string
1050+
// Invalid: type is not a string - will be stringified
10291051
test.info().annotations.push({ type: 123, description: 'desc' });
10301052
expect(1).toBe(1);
10311053
});
10321054
`,
10331055
}, { reporter: 'dot,html' }, { PLAYWRIGHT_HTML_OPEN: 'never' });
1034-
expect(result.exitCode).toBe(1);
1035-
expect(result.output).toContain('Annotation must have a "type" property of type string');
1056+
expect(result.exitCode).toBe(0);
1057+
expect(result.passed).toBe(1);
1058+
1059+
await showReport();
1060+
await page.getByText('test with runtime annotation non-string type').click();
1061+
await expect(page.locator('.test-case-annotation')).toHaveText('123: desc');
10361062
});
10371063

1038-
test('should reject object/array description in runtime annotations', async ({ runInlineTest }) => {
1064+
test('should normalize object/array description in runtime annotations', async ({ runInlineTest, page, showReport }) => {
10391065
const result = await runInlineTest({
10401066
'a.test.js': `
10411067
import { test, expect } from '@playwright/test';
10421068
10431069
test('test with object description', async ({}) => {
1044-
// Invalid: description is an object
1070+
// Invalid: description is an object - will be stringified
10451071
test.info().annotations.push({ type: 'issue', description: { bug: '123' } });
10461072
expect(1).toBe(1);
10471073
});
10481074
`,
10491075
}, { reporter: 'dot,html' }, { PLAYWRIGHT_HTML_OPEN: 'never' });
1050-
expect(result.exitCode).toBe(1);
1051-
expect(result.output).toContain('Annotation "description" must be a primitive value');
1076+
expect(result.exitCode).toBe(0);
1077+
expect(result.passed).toBe(1);
1078+
1079+
await showReport();
1080+
await page.getByText('test with object description').click();
1081+
await expect(page.locator('.test-case-annotation')).toHaveText('issue: [object Object]');
10521082
});
10531083

10541084
test('should allow valid runtime annotations', async ({ runInlineTest, page, showReport }) => {

0 commit comments

Comments
 (0)