Core: guards performance.measure to not throw and cause tests to fail

qunitjs · Oct 12, 2018 · 7b8fc7a · 7b8fc7a
1 parent 2b30582
commit 7b8fc7a
Show file tree

Hide file tree

Showing 5 changed files with 38 additions and 4 deletions.
diff --git a/src/core/utilities.js b/src/core/utilities.js
@@ -1,4 +1,5 @@
 import { window, setTimeout } from "../globals";
+import Logger from "../logger";
 
 export const toString = Object.prototype.toString;
 export const hasOwn = Object.prototype.hasOwnProperty;
@@ -19,6 +20,17 @@ function detectPerformanceApi() {
 		typeof window.performance.measure === "function";
 }
 
+export function measure( comment, startMark, endMark ) {
+
+	// `performance.measure` may fail if the mark could not be found.
+	// reasons a specific mark could not be found include: outside code invoking `performance.clearMarks()`
+	try {
+		performance.measure( comment, startMark, endMark );
+	} catch ( ex ) {
+		Logger.warn( "performance.measure could not be executed because of ", ex.message );
+	}
+}
+
 export const defined = {
 	document: window && window.document !== undefined,
 	setTimeout: setTimeout !== undefined

diff --git a/src/reports/suite.js b/src/reports/suite.js
@@ -1,4 +1,4 @@
-import { performance, performanceNow } from "../core/utilities";
+import { measure, performance, performanceNow } from "../core/utilities";
 
 export default class SuiteReport {
 	constructor( name, parentSuite ) {
@@ -43,7 +43,8 @@ export default class SuiteReport {
 				performance.mark( `qunit_suite_${suiteLevel}_end` );
 
 				const suiteName = this.fullName.join( " – " );
-				performance.measure(
+
+				measure(
 					suiteLevel === 0 ? "QUnit Test Run" : `QUnit Test Suite: ${suiteName}`,
 					`qunit_suite_${suiteLevel}_start`,
 					`qunit_suite_${suiteLevel}_end`

diff --git a/src/reports/test.js b/src/reports/test.js
@@ -1,4 +1,4 @@
-import { extend, performance, performanceNow } from "../core/utilities";
+import { extend, measure, performance, performanceNow } from "../core/utilities";
 
 export default class TestReport {
 	constructor( name, suite, options ) {
@@ -41,7 +41,8 @@ export default class TestReport {
 				performance.mark( "qunit_test_end" );
 
 				const testName = this.fullName.join( " – " );
-				performance.measure(
+
+				measure(
 					`QUnit Test: ${testName}`,
 					"qunit_test_start",
 					"qunit_test_end"

diff --git a/test/performance-mark.html b/test/performance-mark.html
@@ -0,0 +1,13 @@
+<!DOCTYPE html>
+<html>
+	<head>
+		<meta charset="UTF-8">
+		<title>QUnit Performance Test Suite</title>
+		<link rel="stylesheet" href="../dist/qunit.css">
+		<script src="../dist/qunit.js"></script>
+		<script src="performance-mark.js"></script>
+	</head>
+	<body>
+		<div id="qunit"></div>
+	</body>
+</html>
diff --git a/test/performance-mark.js b/test/performance-mark.js
@@ -0,0 +1,7 @@
+QUnit.module( "urlParams performance mark module", function() {
+	QUnit.test( "shouldn't fail if performance marks are cleared ", function( assert ) {
+		performance.clearMarks();
+
+		assert.ok( true );
+	} );
+} );