wikimedia/mediawiki-extensions-MultimediaViewer
1
0
Fork 0
mirror of https://gerrit.wikimedia.org/r/mediawiki/extensions/MultimediaViewer synced 2024-11-23 15:56:47 +00:00
Code Issues Packages Projects Releases Wiki Activity

tests: Remove pointless "Hash handling" test that leaves black overlay

Browse source 
This test was leaving behind the black overlay, obscuring the test
results interface. Upon closer inspection, it appears not one part
of this test is doing anything that actually works.

* Assertion 1: `viewer.isOpen === false`
  This is the initial and default state. Not essential for the test,
  but so far so good.

* Prep for Assertion 2 & 3:

  We call loadImageByTitle. One might think that this will set isOpen
  to true, but, this isn't asserted. If one did, the test would fail,
  because loadImageByTitle returns early from its first branch
  in `!this.thumbs.length` and is thus a no-op, and adds nothing to
  the test.

  We set `location.hash = 'Foo'`.

* Assertion 2: `location.hash === '#Foo'`
  Assertion 3: `viewer.isOpen === false`

  Assertion 2 is meaningless because handling of hash changes appears
  to be asynchronous. Perhaps not always, but at least the indirection
  used in this codebase makes it async. Hence, the fact that it is
  correctly normalised to '#Foo' is meaningless as the popstate
  and hashchange event handler haven't been processed yet. If they
  were, it would still be meaningless since viewer.isOpen was never
  true in the first place and MMV's event handlers for popstate/hashchange
  are guarded by `isOpen`.

* Prep for Assertion 4: location.hash = Bar
* Assertion 4: `location.hash === '#Bar'`

  Idem.

* Prep for Assertion 5: Replace the viewer.loadImageByTitle function
  with a function that performs a nested assertion.

  This function is never called and its assertion never reached.
  This is a textbook example why assertions should never be
  nested. If you rewrite this in following with the advice from
  <https://api.qunitjs.com/assert/verifySteps/>, we find that
  the array is in fact empty by the end of the test() function.

  ```js
  var seen = [];
  viewer.loadImageByTitle = function ( title ) {
    // assert.strictEqual( … );
    seen.push( title.getPrefixedText() );
  };
  location.hash = …;
  location.hash = …;
  location.hash = …;
  location.hash = …;
  assert.deepEqual( seen, [ 'File:' + imageSrc' } );
  // Actual: []
  ```

== Misc notes ==

The test ends with "#/media/File:Foo bar.jpg" set on location.hash,
and the black overlay covering the page. It seems that the hashchange
are async, and that the first in a given event loop tick "wins", with
later ones being ignored. Observing this with debugger/breakpoints is
hard given that each pause results in the effect not being observable.
console.log(location.href) at the end of the test shows hash='#',
but placing a debugger at the end of the test results in the hash
being "#/media/…" instead.

Exprienced in Firefox 117 on macOS.

Change-Id: Ib37bec1b3e67fcab1da89d23381a3adbb6312827
This commit is contained in:
Timo Tijhof 2023-10-05 19:41:22 -07:00 committed by Krinkle
parent 1260d98c78
commit 44b15a2c2c
1 changed files with 1 additions and 76 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

77
tests/qunit/mmv/mmv.test.js
View file

@ -1,4 +1,4 @@
const { LightboxInterface, MultimediaViewer, Thumbnail } = require( 'mmv' );
const { MultimediaViewer, Thumbnail } = require( 'mmv' );
const { getMultimediaViewer } = require( './mmv.testhelpers.js' );
const { MultimediaViewerBootstrap } = require( 'mmv.bootstrap' );
@ -38,81 +38,6 @@ const { MultimediaViewerBootstrap } = require( 'mmv.bootstrap' );
} );
} );
QUnit.test( 'Hash handling', function ( assert ) {
var oldUnattach,
viewer = getMultimediaViewer(),
ui = new LightboxInterface(),
imageSrc = 'Foo bar.jpg',
image = { filePageTitle: new mw.Title( 'File:' + imageSrc ) };
// animation would keep running, conflict with other tests
this.sandbox.stub( $.fn, 'animate' ).returnsThis();
location.hash = '';
viewer.setupEventHandlers();
oldUnattach = ui.unattach;
ui.unattach = function () {
assert.true( true, 'Lightbox was unattached' );
oldUnattach.call( this );
};
viewer.ui = ui;
viewer.close();
assert.strictEqual( viewer.isOpen, false, 'Viewer is closed' );
viewer.loadImageByTitle( image.filePageTitle );
// Verify that passing an invalid mmv hash when the mmv is open triggers unattach()
location.hash = 'Foo';
// Verify that mmv doesn't reset a foreign hash
assert.strictEqual( location.hash, '#Foo', 'Foreign hash remains intact' );
assert.strictEqual( viewer.isOpen, false, 'Viewer is closed' );
ui.unattach = function () {
assert.true( false, 'Lightbox was not unattached' );
oldUnattach.call( this );
};
// Verify that passing an invalid mmv hash when the mmv is closed doesn't trigger unattach()
location.hash = 'Bar';
// Verify that mmv doesn't reset a foreign hash
assert.strictEqual( location.hash, '#Bar', 'Foreign hash remains intact' );
viewer.ui = { images: [ image ], disconnect: function () {} };
$( '#qunit-fixture' ).append( '<a class="image"><img src="' + imageSrc + '"></a>' );
viewer.loadImageByTitle = function ( title ) {
assert.strictEqual( title.getPrefixedText(), 'File:' + imageSrc, 'The title matches' );
};
// Open a valid mmv hash link and check that the right image is requested.
// imageSrc contains a space without any encoding on purpose
location.hash = '/media/File:' + imageSrc;
// Reset the hash, because for some browsers switching from the non-URI-encoded to
// the non-URI-encoded version of the same text with a space will not trigger a hash change
location.hash = '';
// Try again with an URI-encoded imageSrc containing a space
location.hash = '/media/File:' + encodeURIComponent( imageSrc );
// Reset the hash
location.hash = '';
// Try again with a legacy hash
location.hash = 'mediaviewer/File:' + imageSrc;
viewer.cleanupEventHandlers();
location.hash = '';
} );
QUnit.test( 'Progress', function ( assert ) {
var imageDeferred = $.Deferred(),
viewer = getMultimediaViewer(),