Skip to content

Commit

Permalink
Core:Tests: Add jQuery.migrateDeduplicateWarnings; don't dedupe in te…
Browse files Browse the repository at this point in the history 
…sts (jquery#332)

This commit introduces a new API, `jQuery.migrateDeduplicateWarnings`, set to
`true` by default but set to `false` in tests. This allows a more reliable
testing of bigger code blocks wrapped in a single `expectWarning` helper.

The commit also leverages this new behavior to simplify `jQuery.trim` tests.

Ref jquerygh-331
  • Loading branch information
@mgol
mgol authored Apr 10, 2020
1 parent da094d8 commit a36267d
Show file tree
Hide file tree
Showing 6 changed files with 58 additions and 38 deletions.
2 changes: 2 additions & 0 deletions README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,8 @@ This plugin adds some properties to the `jQuery` object that can be used to prog

`jQuery.migrateVersion`: This string property indicates the version of Migrate in use.

`jQuery.migrateDeduplicateWarnings`: By default, Migrate only gives a specific warning once. If you set this property to `false` it will give a warning for every occurrence each time it happens. Note that this can generate a lot of output, for example when a warning occurs in a loop.

## Reporting problems

Bugs that only occur when the jQuery Migrate plugin is used should be reported in the [jQuery Migrate Issue Tracker](https://github.com/jquery/jquery-migrate/issues) and should be accompanied by an executable test case that demonstrates the bug. The easiest way to do this is via an online test tool such as [jsFiddle.net](https://jsFiddle.net/) or [jsbin.com](https://jsbin.com). Use the development version when you are reporting bugs.
Expand Down
5 changes: 4 additions & 1 deletion src/migrate.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@

var warnedAbout = {};

// By default each warning is only reported once.
jQuery.migrateDeduplicateWarnings = true;

// List of warnings already given; public read only
jQuery.migrateWarnings = [];

Expand All @@ -42,7 +45,7 @@ jQuery.migrateReset = function() {

function migrateWarn( msg ) {
var console = window.console;
if ( !warnedAbout[ msg ] ) {
if ( !jQuery.migrateDeduplicateWarnings || !warnedAbout[ msg ] ) {
warnedAbout[ msg ] = true;
jQuery.migrateWarnings.push( msg );
if ( console && console.warn && !jQuery.migrateMute ) {
Expand Down
6 changes: 3 additions & 3 deletions test/attributes.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,12 +47,12 @@ QUnit.test( ".toggleClass( boolean )", function( assert ) {

var e = jQuery( "<div />" ).appendTo( "#qunit-fixture" );

expectWarning( assert, "toggling initially empty class", function() {
expectWarning( assert, "toggling initially empty class", 1, function() {
e.toggleClass( true );
assert.equal( e[ 0 ].className, "", "Assert class is empty (data was empty)" );
} );

expectNoWarning( assert, ".toggleClass( string ) not full className", function() {
expectNoWarning( assert, ".toggleClass( string ) not full className", 1, function() {
e.attr( "class", "" );
e.toggleClass( "classy" );
assert.equal( e.attr( "class" ), "classy", "class was toggle-set" );
Expand All @@ -74,7 +74,7 @@ QUnit.test( ".toggleClass( boolean )", function( assert ) {
assert.ok( e.is( ".testD.testE" ), "Assert class present (restored from data)" );
} );

expectWarning( assert, ".toggleClass( boolean )", 1, function() {
expectWarning( assert, ".toggleClass( boolean )", 5, function() {
e.toggleClass( false );
assert.ok( !e.is( ".testD.testE" ), "Assert class not present" );
e.toggleClass( true );
Expand Down
61 changes: 29 additions & 32 deletions test/core.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,27 @@ QUnit.test( "compareVersions and jQueryVersionSince", function( assert ) {
assert.equal( jQueryVersionSince( jQuery.fn.jquery ), true, "since - equal" );
} );

QUnit.test( "jQuery.migrateDeduplicateWarnings", function( assert ) {
assert.expect( 3 );

var origValue = jQuery.migrateDeduplicateWarnings;
assert.strictEqual( origValue, true, "true by default" );

jQuery.migrateDeduplicateWarnings = true;
expectWarning( assert, "jQuery.migrateDeduplicateWarnings === true", 1, function() {
jQuery( "#" );
jQuery( "#" );
} );

jQuery.migrateDeduplicateWarnings = false;
expectWarning( assert, "jQuery.migrateDeduplicateWarnings === false", 2, function() {
jQuery( "#" );
jQuery( "#" );
} );

jQuery.migrateDeduplicateWarnings = origValue;
} );

QUnit.test( "jQuery(html, props)", function( assert ) {
assert.expect( 3 );

Expand Down Expand Up @@ -97,21 +118,21 @@ QUnit.test( "Attribute selectors with unquoted hashes", function( assert ) {
} );
} );

expectWarning( assert, "Values with unquoted hashes are quoted", fixables.length, function() {
expectWarning( assert, "Values with unquoted hashes are quoted", fixables.length * 2, function() {
fixables.forEach( function( fixable ) {
assert.equal( jQuery( fixable, markup ).length, 1, fixable );
assert.equal( markup.find( fixable ).length, 1, fixable );
} );
} );

expectWarning( assert, "False positives", positives.length, function() {
expectWarning( assert, "False positives", positives.length * 2, function() {
positives.forEach( function( positive ) {
assert.equal( jQuery( positive, markup ).length, 1, positive );
assert.equal( markup.find( positive ).length, 1, positive );
} );
} );

expectWarning( assert, "Unfixable cases", failures.length, function() {
expectWarning( assert, "Unfixable cases", failures.length * 2, function() {
failures.forEach( function( failure ) {
try {
jQuery( failure, markup );
Expand Down Expand Up @@ -313,7 +334,7 @@ QUnit[ jQueryVersionSince( "3.3.0" ) ? "test" : "skip" ]( ".isNumeric (warn)", f
QUnit[ jQueryVersionSince( "3.3.0" ) ? "test" : "skip" ]( "jQuery.isWindow", function( assert ) {
assert.expect( 3 );

expectWarning( assert, "isWindow", 1, function() {
expectWarning( assert, "isWindow", 2, function() {
assert.equal( jQuery.isWindow( [] ), false, "array" );
assert.equal( jQuery.isWindow( window ), true, "window" );
} );
Expand Down Expand Up @@ -364,56 +385,32 @@ QUnit.test( "jQuery.expr.pseudos aliases", function( assert ) {
QUnit.test( "jQuery.holdReady (warn only)", function( assert ) {
assert.expect( 1 );

expectWarning( assert, "jQuery.holdReady", 1, function() {
expectWarning( assert, "jQuery.holdReady", 2, function() {
jQuery.holdReady( true );
jQuery.holdReady( false );
} );
} );

QUnit[ jQueryVersionSince( "3.1.1" ) ? "test" : "skip" ]( "jQuery.trim", function( assert ) {
assert.expect( 26 );
assert.expect( 14 );

var nbsp = String.fromCharCode( 160 );

expectWarning( assert, "jQuery.trim", 1, function() {
expectWarning( assert, "jQuery.trim", 13, function() {
assert.equal( jQuery.trim( "hello " ), "hello", "trailing space" );
} );
expectWarning( assert, "jQuery.trim", 1, function() {
assert.equal( jQuery.trim( " hello" ), "hello", "leading space" );
} );
expectWarning( assert, "jQuery.trim", 1, function() {
assert.equal( jQuery.trim( " hello " ), "hello", "space on both sides" );
} );
expectWarning( assert, "jQuery.trim", 1, function() {
assert.equal( jQuery.trim( " " + nbsp + "hello " + nbsp + " " ), "hello", "&nbsp;" );
} );

expectWarning( assert, "jQuery.trim", 1, function() {
assert.equal( jQuery.trim(), "", "Nothing in." );
} );
expectWarning( assert, "jQuery.trim", 1, function() {
assert.equal( jQuery.trim( undefined ), "", "Undefined" );
} );
expectWarning( assert, "jQuery.trim", 1, function() {
assert.equal( jQuery.trim( null ), "", "Null" );
} );
expectWarning( assert, "jQuery.trim", 1, function() {
assert.equal( jQuery.trim( 5 ), "5", "Number" );
} );
expectWarning( assert, "jQuery.trim", 1, function() {
assert.equal( jQuery.trim( false ), "false", "Boolean" );
} );

expectWarning( assert, "jQuery.trim", 1, function() {
assert.equal( jQuery.trim( " " ), "", "space should be trimmed" );
} );
expectWarning( assert, "jQuery.trim", 1, function() {
assert.equal( jQuery.trim( "ipad\xA0" ), "ipad", "nbsp should be trimmed" );
} );
expectWarning( assert, "jQuery.trim", 1, function() {
assert.equal( jQuery.trim( "\uFEFF" ), "", "zwsp should be trimmed" );
} );
expectWarning( assert, "jQuery.trim", 1, function() {
assert.equal( jQuery.trim( "\uFEFF \xA0! | \uFEFF" ), "! |", "leading/trailing should be trimmed" );
} );
} );
Expand Down Expand Up @@ -481,7 +478,7 @@ QUnit[ jQueryVersionSince( "3.3.0" ) ? "test" : "skip" ]( "jQuery.type (warn)",
QUnit[ jQueryVersionSince( "3.3.0" ) ? "test" : "skip" ]( "jQuery.isArray", function( assert ) {
assert.expect( 4 );

expectWarning( assert, "isArray", 1, function() {
expectWarning( assert, "isArray", 3, function() {
assert.equal( jQuery.isArray( [] ), true, "empty array" );
assert.equal( jQuery.isArray( "" ), false, "empty string" );
assert.equal( jQuery.isArray( jQuery().toArray() ), true, "toArray" );
Expand Down
2 changes: 1 addition & 1 deletion test/event.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ QUnit.test( "Event aliases", function( assert ) {
var $div = jQuery( "<div />" );

"scroll click submit keydown".split( " " ).forEach( function( name ) {
expectWarning( assert, "." + name + "()", 1, function() {
expectWarning( assert, "." + name + "()", 2, function() {
$div[ name ]( function( event ) {
assert.equal( event.type, name, name );
$div.off( event );
Expand Down
20 changes: 19 additions & 1 deletion test/testinit.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ TestManager = {
iframeCallback: undefined,
baseURL: window.__karma__ ? "base/test/" : "./",
init: function( projects ) {
var p, project;
var p, project, originalDeduplicateWarnings;

this.projects = projects;
this.loaded = [];
Expand All @@ -134,6 +134,24 @@ TestManager = {
value: project.choices.split( "," )
} );
}


QUnit.begin( function( details ) {
originalDeduplicateWarnings = jQuery.migrateDeduplicateWarnings;
} );

// If only the first warning is reported, tests using `expectWarning`
// with multiple function calls would pass even if some of them didn't
// warn. Because of that, by default don't deduplicate warnings in tests.
QUnit.testStart( function( details ) {
if ( details.name !== "jQuery.migrateDeduplicateWarnings" ) {
jQuery.migrateDeduplicateWarnings = false;
} else {

// When testing this API, we want to start with its default value.
jQuery.migrateDeduplicateWarnings = originalDeduplicateWarnings;
}
} );
}
};
TestManager.init( {
Expand Down

0 comments on commit a36267d

Please sign in to comment.