Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code

Delta Between Two Patch Sets: test/_common.js

Issue 29622595: Issue 6090 - Properly use regexp for message matching and proper console override (Closed) Base URL: https://hg.adblockplus.org/adblockpluscore/
Left Patch Set: Properly handle non-string arguments for console.*() Created Nov. 28, 2017, 9:28 p.m.
Right Patch Set: Properly handle args Created Nov. 29, 2017, 3:21 p.m.
Left:
Right:
Use n/p to move between diff chunks; N/P to move between comments.
Jump to:
Left: Side by side diff | Download
Right: Side by side diff | Download
« no previous file with change/comment | « no previous file | no next file » | no next file with change/comment »
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
LEFTRIGHT
1 /* 1 /*
2 * This file is part of Adblock Plus <https://adblockplus.org/>, 2 * This file is part of Adblock Plus <https://adblockplus.org/>,
3 * Copyright (C) 2006-present eyeo GmbH 3 * Copyright (C) 2006-present eyeo GmbH
4 * 4 *
5 * Adblock Plus is free software: you can redistribute it and/or modify 5 * Adblock Plus is free software: you can redistribute it and/or modify
6 * it under the terms of the GNU General Public License version 3 as 6 * it under the terms of the GNU General Public License version 3 as
7 * published by the Free Software Foundation. 7 * published by the Free Software Foundation.
8 * 8 *
9 * Adblock Plus is distributed in the hope that it will be useful, 9 * Adblock Plus is distributed in the hope that it will be useful,
10 * but WITHOUT ANY WARRANTY; without even the implied warranty of 10 * but WITHOUT ANY WARRANTY; without even the implied warranty of
(...skipping 402 matching lines...) Expand 10 before | Expand all | Expand 10 after
413 }; 413 };
414 }; 414 };
415 415
416 console.warn = console.log; 416 console.warn = console.log;
417 417
418 exports.silenceWarnOutput = function(test, msg) 418 exports.silenceWarnOutput = function(test, msg)
419 { 419 {
420 let warnHandler = console.warn; 420 let warnHandler = console.warn;
421 console.warn = (...args) => 421 console.warn = (...args) =>
422 { 422 {
423 let s; 423 let s = (args[0] instanceof Error ? args[0].message : args[0]);
424 if (typeof args[0] === "string")
sergei 2017/11/29 09:31:53 To be consistent with other JS code it should not
425 s = args[0];
426 else
427 s = args[0].message;
sergei 2017/11/29 09:31:54 args[0] can be undefined and before accessing `mes
Wladimir Palant 2017/11/29 10:27:31 Yes, the better approach is: let s = (args[0] i
hub 2017/11/29 15:22:32 Done.
428 424
429 if (s != msg) 425 if (s != msg)
430 warnHandler(args); 426 warnHandler(...args);
sergei 2017/11/29 09:31:53 strictly speaking it should be warnHandler.warn.ap
Wladimir Palant 2017/11/29 10:27:31 This should be `warnHandler(...args)` - pass the o
hub 2017/11/29 15:22:32 Done.
431 }; 427 };
432 try 428 try
433 { 429 {
434 return test(); 430 return test();
435 } 431 }
436 finally 432 finally
437 { 433 {
438 console.warn = warnHandler; 434 console.warn = warnHandler;
439 } 435 }
440 }; 436 };
441 437
442 exports.silenceAssertionOutput = function(test, msg) 438 exports.silenceAssertionOutput = function(test, msg)
443 { 439 {
444 let msgMatch = new RegExp("^Error: (.*)[\r\n]"); 440 let msgMatch = new RegExp("^Error: (.*)[\r\n]");
445 let errorHandler = console.error; 441 let errorHandler = console.error;
446 console.error = (...args) => 442 console.error = (...args) =>
447 { 443 {
448 let s; 444 let s = (args[0] instanceof Error ? args[0].message : args[0]);
449 if (typeof args[0] === "string")
450 s = args[0];
451 else
452 s = args[0].message;
453
454 let match = s && s.match(msgMatch); 445 let match = s && s.match(msgMatch);
455 if (!match || match[1] != msg) 446 if (!match || match[1] != msg)
Wladimir Palant 2017/11/29 10:27:31 So why still use the regular expression if you are
hub 2017/11/29 15:22:32 We want to silence an assert (call to our assert2(
456 errorHandler(args); 447 errorHandler(...args);
457 }; 448 };
458 try 449 try
459 { 450 {
460 return test(); 451 return test();
461 } 452 }
462 finally 453 finally
463 { 454 {
464 console.error = errorHandler; 455 console.error = errorHandler;
465 } 456 }
466 }; 457 };
(...skipping 13 matching lines...) Expand all
480 } 471 }
481 }) 472 })
482 }; 473 };
483 }; 474 };
484 475
485 exports.unexpectedError = function(error) 476 exports.unexpectedError = function(error)
486 { 477 {
487 console.error(error); 478 console.error(error);
488 this.ok(false, "Unexpected error: " + error); 479 this.ok(false, "Unexpected error: " + error);
489 }; 480 };
LEFTRIGHT
« no previous file | no next file » | Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Toggle Comments ('s')

Powered by Google App Engine
This is Rietveld