Wikimedia Bugzilla is closed!

We had a similar discussion recently at e3-team@lists.wikimedia.org. (I could not find the list at https://lists.wikimedia.org/mailman/listinfo. Subject was "testing extension JS code").

That should be easy to do. I see three ways how to do it:

1) Create a separate smoke test suite that will just open a lot of pages in a lot of browsers. Every time the page is opened the test checks for ResourceLoader errors.

2) If that proves useful, we add the check to regular tests, as it is requested in this bug.

3) We implement the check in a framework like TestSwarm. When we were in Netherlands late last year I remember Timo (Krinke) mentioned he is working on TestSwarm (http://swarm.jquery.org/) tests for MediaWiki. Timo, is the project sill alive? Is TestSwarm the way to go, or should we use Selenium?

I will implement 1) for now until I hear back from Timo.

S, could you please create a page that should fail a test? Or just let me know what I need to do on a page to make it fail the test. I do not trust a test that I did not see fail first.

Asserting that modules load without errors is trivial to implement in QUnit. That's the cheapest, most reliable and appropiate way to test it. No need to do this from Selenium (which would do it only from qa-browsertests right now, instead of in the unit tests).

See also:

https://www.mediawiki.org/wiki/Continuous_integration/Workflow_specification#High_level_overview

You might want to focus on running the QUnit test suite occasionally in browsers, aside from the Selenium integration tests.

Also, to clarify, there is no such thing as a TestSwarm test. TestSwarm, with all its glory, is just a dum tool that distributes a QUnit suite as html file to a bunch of browsers and distributes the tests.

Having said that, TestSwarm is on the roadmap, but it's not really relevant here, nor is Selenium imho.

Lowering priority to normal because module test suites already naturally result in a test failure if the module is not loaded (0 tests is considered a failure). And if the tests load but the core module does not then all tests will fail naturally.

Is there anything that needs to be done about this bug, or should be close it?

Created attachment 11696 [details]
some JavaScript that has errors; copy into extensions/E3Experiments/lib

(In reply to comment #2)
> S, could you please create a page that should fail a test? ...
> I do not trust a test that I did not see fail first.

You have to craft an extension with a ResourceLoader module that fails, and then visit a page that loads that module.

I could check in a version of an extension that fails, or you could roll back to a version of AFT, postEdit, etc. that had a JavaScript error.  Instead, I took advantage of a feature of Extension:E3Experiments: it loads all files matching lib/*.js.  So if you copy the attached file into extensions/E3Experiments/lib/, then any wiki page that loads ext.Experiments.lib or another module that depends on it will encounter
   mw.loader.getState( 'ext.Experiments.lib' ) === "error"

Currently E3Experiments' own ext.Experiments.experiments and ext.Experiments.acux modules depend on this, and E3Experiments adds the former to every page in a BeforePageDisplayHook.  So, on any wiki page at least twomodules will be in state "error" so long as RL is not in debug mode. In debug mode (with ?debug=1 or the resourceLoaderDebug cookie set to true) then the modules are in state "ready" despite the runtime error in one of the module's scripts.

If you give me some pointers I can try adding an assertion to Cucumber.


(In reply to comment #4)
> module test suites already naturally
> result in a test failure if the module is not loaded (0 tests is considered a
> failure). And if the tests load but the core module does not then all tests
> will fail naturally.

This is not my experience, running fairly recent http://localhost/wiki/index.php/Special:JavaScriptTest/qunit in non-debug mode with this bad .js file hack to cause errors:
* There's no report from qunit that any modules are in "error" state.
* The E3Experiments qunit test is silently omitted from the list of tested modules and the dropdown, but there's no warning.
* If I force the E3Experiments test to run with http://localhost/wiki/index.php/Special:JavaScriptTest/qunit?module=ext.E3Experiments.standalone , it reports "0 tests of 0 passed, 0 failed", but otherwise looks OK.

The E3Experiments qunit test is very incomplete, it can run standalone and barely tests any functionality.  But mw.loader.getState( 'ext.Experiments.tests' ) is "error" also, so I would expect some notification from qunit that there was a problem.

I entered bug 44488 about Special:JavaScriptTest/qunit  not reporting errors (the second half of my comment #7).

S, I have added the check when opening login page[1].

Next steps:

1) We could add the check to every place where we check a page. That should not be a lot of work.

2) If we monkey patch PageObject::PageFactory#visit_page[2] the check would automatically run every time a test opens a page. That should be trivial to implement, but I am not sure if there is a better way.

Chris, what do you think?

[1] https://gerrit.wikimedia.org/r/#/c/47027/
[2] http://stackoverflow.com/q/14635290/17469

I like the idea of monkey-patching #visit_page for this.  If we don't like it we could easily revert it.

So why did you add it as a selenium test in Iadda6170?

You could've added in mediawiki core's qunit test instead.

I don't maintain selenium tests so do as you like, but an explanation or counter argument would help me understand it your decision.

If you don't want to run qunit tests before triggering selenium, how about writing selenium code to go to Special:JavaScriptTest/qunit and running all the qunit tests before anything else?

I still stand (though I'd like to be convinced other wise) on this assertion being a unit test and belonging in and only in the qunit test suite (not in selenium integration tests). The last thing we'd want is two different test phases asserting the same thing.

Just like we don't do linting checks in qunit, we shouldn't be doing unit tests in selenium.

See also comment 3 and bug 44488. Only run selenium tests after making sure qunit tests pass. If you don't then you'll only get caught up in trying to catch everything that can go wrong, and that's a whole lot of things too many.

One big reason to do this with Selenium is that the qunit tests don't run according to https://integration.mediawiki.org/ and I can't find them in https://integration.mediawiki.org/ci/

What Zeljko proposes is easy, answers a need, and costs very little.  It's been done before:  http://watirmelon.com/2012/12/19/using-webdriver-to-automatically-check-for-javascript-errors-on-every-page/

Well, Selenium doesn't run from the standard Jenkins jobs either, so that doesn't justify anything.

I meant that a human (or Selenium driven script) can easily "run" the QUnit test before proceeding to the integration tests.

Also, Antoine and I are getting really close to getting QUnit tests to run on every commit. It would be useful to have this unit test be part of the unit test suite.

What has been done before doesn't justify anything. Generally if something involves only javascript, and involves javascript calling a function and directing asserting the outcome, that's best done as a unit test.

I'm not saying Selenium can't do it. It is very good as reaching out to the javascript engine and executing arbitrary strings of code.

What I mean is, when a developer is working developing, there's a limit to what is reasonable to run periodically when testing and working with things before committing.

That generally involves linting and unit testing. Not cross-browser integration tests (not yet anyway). So aside from what is semantically correct (though I think this is semantically a unit test), it is currently more useful for it to be a unit test so that more people naturally run into it more often in the current state of reality.

I'd be happy to add you to the account at https://wmf.ci.cloudbees.com/ if it would be useful.  

I agree that it would be better if qunit could identify these kinds of failures. 

However, I suspect that qunit is not going to be able to support the multiple versions of IE that we get for free in the Selenium framework. 

So until such time as qunit suite can support this sort of test, I'd like to put it in the selenium tests.  As I mentioned earlier, this would serve an immediate need, and if we can in fact do this sort of test in qunit eventually, we can easily remove it from the selenium test suite later.

By design it works better in QUnit because:
* It is in javascript (as opposed to passing strings of code to an execution engine from Selenium)
* It works in equal or more browsers (but not less, since all it requires it javascript, as opposed to javascript + web driver API)


Can you explain what you mean by "able to support multiple IE versions" and "until [it] can support this kind of test"? That makes no sense, as those seem simply untrue.

I think the assertion is worth making in both test environments.  Browser behavior testing is going to load a different subset of ResourceLoader modules on each page and in a different order than Special:JavaScriptTest/qunit.  It would be great if a test in the latter would load every registered module (currently 391 on enwiki!) and see if any end up in an error state.

(In reply to comment #12)
The test you link to for any JS errors is different from this bug, but would also be great to run on every Selenium page! It would catch, e.g. EventLogging's run-time warnings.

As Voltaire famously remarked, don't let the ideal be the enemy of the somewhat improved testing situation.

@spage: the javascript that you have provided causes TypeError on Internet Explorer 6-8. See this Sauce Labs job for more details (scroll to the bottom of the page for screenshot):

https://saucelabs.com/tests/b203cb5adf664843ba69e9a64e4faa17



@Krinkle: I have added it as a Selenium test as an experiment. I am reverting the change, see above for the reason.

Oh, array filter unsupported in IE versions prior to 9,
https://developer.mozilla.org/en-US/docs/JavaScript/Reference/Global_Objects/Array/filter

There are lots of ways to do it.  The following should produce an empty string, otherwise it produces a string identifying problematic modules and their state. 

var modules = mw.loader.getModuleNames(),
    len = modules.length,
    name, state,
    bad = [],
    err = '';

for (var i = 0; i < len; i++) {
    state = mw.loader.getState( modules[i] );
    if ( state !== 'ready' && state !== 'registered' ) {
        bad.push ( modules[i] + ' state is ' + state );
    }
}
if ( bad.length ) {
    err = "troubled modules: " + bad.join( '; ') + '.';
}
err;
// Something like assertEmptyString( err );

(In reply to comment #18)
> Oh, array filter unsupported in IE versions prior to 9,
> https://developer.mozilla.org/en-US/docs/JavaScript/Reference/Global_Objects/
> Array/filter
> 
> There are lots of ways to do it.  The following should produce an empty
> string,
> otherwise it produces a string identifying problematic modules and their
> state. 
> 
> var modules = mw.loader.getModuleNames(),
>     len = modules.length,
>     name, state,
>     bad = [],
>     err = '';
> 
> for (var i = 0; i < len; i++) {
>     state = mw.loader.getState( modules[i] );
>     if ( state !== 'ready' && state !== 'registered' ) {
>         bad.push ( modules[i] + ' state is ' + state );
>     }
> }
> if ( bad.length ) {
>     err = "troubled modules: " + bad.join( '; ') + '.';
> }
> err;
> // Something like assertEmptyString( err );

Unnecessarily complex, and produces output that is hard to use.

Probably more like (untested):

var i, len, state,
 modules = mw.loader.getModuleNames(),
 error = [],
 missing = [];

for ( i = 0, len = modules.length; i < len; i++ ) {
    state = mw.loader.getState( modules[i] );
    if ( state === 'error' ) {
        error.push( modules[i] );
    }
    if ( state === 'missing' ) {
        missing.push( modules[i] );
    }
}

assert.deepEqual( error, [], 'Modules in error state' );
assert.deepEqual( missing, [], 'Modules in missing state' );


Anyway, patch something like this up for mediawiki core's qunit test.

S, would it make more sense now to include a test like this in QUnit, now that "we automatically run our QUnit test suite in MediaWiki core from Jenkins"[1].

Is there still a reason to run the test from Selenium?

[1] http://lists.wikimedia.org/pipermail/wikitech-l/2013-March/067208.html

(In reply to comment #20)
> S, would it make more sense now to include a test like this in QUnit, now
> that
> "we automatically run our QUnit test suite in MediaWiki core from

As I remarked in comment #16, run in both.  "Browser behavior testing is going to load a different subset of ResourceLoader modules on each page and in a different order than Special:JavaScriptTest/qunit."

> It would be great if a test in the latter would load every registered module
> (currently 391 on enwiki!) and see if any end up in an error state.
I tried this and it doesn't work, some of the modules are conflicting skins and others fail by design if they're loaded on an inappropriate page. The page looks bizarre after you try it :)

Moving this bug back to MediaWiki>Unit tests.

(In reply to comment #20)
> Is there still a reason to run the test from Selenium?

I agree with S, it is useful to run them both from QUnit (on Special:JavaScriptTest) and from Selenium (on actual pages as part of the wider integration test).

(In reply to comment #21)
> > It would be great if a test in the latter would load every registered module
> > (currently 391 on enwiki!) and see if any end up in an error state.
> I tried this and it doesn't work, some of the modules are conflicting skins
> and  others fail by design if they're loaded on an inappropriate page. The page
> looks bizarre after you try it :)

Indeed, they're not supposed to be loaded unconditionally. Failure is by design in that case.

Also, even if they were changed to fail gracefully when loaded in the wrong environment, the test would be moot. It would run the code and load the classes in memory, but it still won't actually execute individual methods until they are interacted with. You'd have to know how to interact with it and trigger every code path.

The only thing achieved by indiscriminately loading all code is catching syntax errors, which we already catch pre-commit with jshint. And if you want to catch it on code that isn't linted for some reason, run jshint on it (checkout all of mediawiki/core and mediawiki/extensions/* and run jshint, if you really want to, though that's probably going to give a lot of false positives since it includes upstream code. Best to just let Jenkins/jshint handle it).

Change-Id: Ib6e2b8d1be3e38fd9f1b948407c62da550fce0b4

Note that the mediawiki_selenium gem also implements this, so you can add
  step "page has no ResourceLoader errors"
to browser tests.

https://github.com/wikimedia/mediawiki-selenium/blob/master/lib/mediawiki_selenium/step_definitions/resource_loader_steps.rb