qunitjs / qunit

🔮 An easy-to-use JavaScript unit testing framework.
https://qunitjs.com
MIT License
4.01k stars 783 forks source link

Modules can be out of order #391

Closed jaredwy closed 11 years ago

jaredwy commented 11 years ago

I noticed in a project i was working on that sometimes that module names would appear out of order in their drop down.

Can't rely on key ordering in objects. Pull request to follow.

Krinkle commented 11 years ago

I agree with @JamesMGreene, I have yet to see an example where the modules are out of order. I suspect there is a problem with the way you load the test suites?

JamesMGreene commented 11 years ago

I believe that the test.queue(); would also maintain the module order, right?

@jaredwy is definitely not wrong that key order can be unreliable in the older browsers at the ECMAScript spec never guarantees order but I still don't even recall seeing this in, say, IE7.

jzaefferer commented 11 years ago

The only case I know of where a browser didn't maintain insertion order was early Chrome, and after pointing at the spec for a little while, they quickly fixed it.

jaredwy commented 11 years ago

They are currently showing in execution order for me. Everyone on our team complained about it being hard to locate specific modules so i made it alphabetical to aid in finding the executed module. Looking at it, it appears chrome dev tools may sort keys in their console output (strange).

@jzaefferer pointing to the spec? the spec says "The mechanics and order of enumerating the properties (step 6.a in the first algorithm, step 7.a in the second) is not specified." See, http://es5.github.com/#x12.6.4

jaredwy commented 11 years ago

Screenshot_10_01_13_6_12_AM This is a screenshot of part of our dropdown. They are showing as the order executed, not alphabetically we have a lot of modules. It takes a bit of time for us to locate the module we want to run.

Krinkle commented 11 years ago

@jaredwy How are you loading the test suites?

jaredwy commented 11 years ago

Tests are brought in using require.js. Once all the tests are loaded we call QUnit.start(); Note, order is not impacted by the order in which they load either it is execution order.

JamesMGreene commented 11 years ago

Oh! The title of this issue and your initial comment definitely confused me. It seemed like you were implying that the modules themselves were executing in an incorrect order (i.e. not in the order their respective scripts are executed) or that the HTML report was displaying them in an incorrect order.

It seems that neither of those is true here, rather you just want to have the Modules dropdown sorted to make it easier to find the desired module. I think that is reasonable but not what I would expect to see by default (which would be a list that matches the execution order, IMHO). However, I would assume it would also be easy to add a basic sort that can be invoked via the URL configuration... but again, probably not in core...?

As a workaround, your team could each add this a simple bookmarklet to your browsers to quickly sort the list using the following script (and assuming you have jQuery on the page): https://gist.github.com/4496619

jaredwy commented 11 years ago

I am not understanding the reasoning behind wanting them sorted by execution order. That seems completely arbitrary and subject to change, not sure what benefit it adds as it makes it hard to locate modules you want to run quickly?

JamesMGreene commented 11 years ago

For example, if I am looking at the HTML report page and see that my module is the 2nd module executed, it's much quicker for me to select the 2nd item in the dropdown than to find it by name. But I do totally acknowledge your use case: large sets of modules are indeed difficult to manage sometimes. QUnit overlord @jzaefferer may be more in favor than I am, I'm just the new kid on the block. :)

Another option if your report is just too unwieldy is to utilize the QUnit composite addon to break your test run up into sub-pages.

jaredwy commented 11 years ago

Yeah as you said, this will not scale. When you have scrolled down and notice that a module is broken, now you have to count how many were broken, then count that many in the drop down.

Quote i just recieved from a team memmber if it helps sway @jzaefferer ;) i am also not above bribes.

Screenshot_10_01_13_7_55_AM

We are not quiet at the size we would start to split tests out into their own modules. The overhead of managing the extra code is just not worth it yet. Even with 20 modules it became hard to locate the module in the drop down.

Krinkle commented 11 years ago

Okay, lets zoom back out. I belief there is a misunderstanding here that was exposed when @jaredwy answered my question on how their test suite is loaded.

The modules should not be executed in alphabetical order, instead they should executed in the order they arrive. First and formost because QUnit can't know how many and when more modules are going to be loaded.

Secondly, the dropdown menu. The dropdown menu currently matches the order of the modules on the page (so it is like a visually matching table of contents).

Lastly, coming back to the misunderstanding. The problem is that you're loading all test suites separately and asynchronously. Which means you're no longer have control over the order in which they arrive.

I agree with @JamesMGreene that QUnit should not intervene in the module order. If you want to load the modules in a certain order, QUnit should stick with that. For example in MediaWiki we execute mediawiki-prefixed modules first and then various jquery plugins. And in jQuery we want "core" to be first, not "ajax".

Though for the sake of atomic tests, it shouldn't really matter. It's all going to execute anyway. But if you have a logical structure, it makes sense to execute certain tests first. Not because they depend on it, but because those failures are on a lower level (for example in jQuery it makes sense to execute core, than callbacks, than deferred than ajax. Because if there are issues in callbacks, it might make ajax fail but you'd want to see the callback failures on top).

The drop down menu we can sorting alphabetically, that's fine. It won't affect the test execution.It does mean the menu order won't match the order of the tests on the page.

Though I still recommend that you don't load test suites asynchronously. It seems like an optimisation that isn't very useful for a test suite, it can only cause inconsistencies and obscurity. That way you're responsible and fully in control over the order.

jaredwy commented 11 years ago

i am not changing the order of which the tests run. Sorry if that is clear, it is simply the order of the dropdown.

The only thing sorted here it is the order of the dropdown. The code in the pull request should make that clear, i hope.

This change is to simply help to locate modules to run easier (humans can scan lists quicker in alphabetical order rather than some arbitrary run order).

JamesMGreene commented 11 years ago

I'd prefer to see the dropdown's contents still in execution order by default but add a urlConfig to alpha-sort it.

jaredwy commented 11 years ago

If that will get this pull request across the line i will happily add it.

On Sat, Jan 19, 2013 at 12:55 PM, James M. Greene notifications@github.comwrote:

I'd prefer to see the dropdown's contents still in execution order by default but add a urlConfig to alpha-sort it.

— Reply to this email directly or view it on GitHubhttps://github.com/jquery/qunit/issues/391#issuecomment-12460990.

Krinkle commented 11 years ago

@jaredwy As I asked before, why don't you just load the tests in the order you want them in?

@JamesMGreene I find a urlConfig option unacceptable for a trivial thing like this. If we start adding options for UI, it will never end. Make a good UI and deal with it.

jaredwy commented 11 years ago

Putting the onus of maintaining alphabetical order on humans is not feasible. Also, this is a large project we can not easily change the way we load tests.

Could some one please explain how execution order scales for this drop down? With each module possibly containing tens or even hundreds of tests? How can you possibly keep track of what number module you are up to? How can you easily locate the relevant module in the dropdown?

On 19/01/2013, at 6:26 PM, Timo Tijhof notifications@github.com wrote:

@jaredwy As I asked before, why don't you just load the tests in the order you want them in?

— Reply to this email directly or view it on GitHub.

JamesMGreene commented 11 years ago

It depends on what you think the order of your tests is. If they are arbitrarily ordered, then yes, it will likely be impossible to keep track of them in the dropdown. However, as @Krinkle mentioned, if you take a project like jQuery core, then the order of tests is very intentional: it starts with the most "core" code and spirals its way outward to the most high-level code.

While I don't agree with @Krinkle's proposal to just order your tests in advance to be alphabetical, I do agree with his concern over bloating the UI with options... which again brings me back to my previous proposal of utilizing a bookmarklet — or, if clicking a bookmarklet on the occasions when you do need to use the module filter dropdown is too much effort, then just attach the functionality that the bookmarklet provides to a QUnit.done callback handler to sort the dropdown after the full test run has completed on your test page.

Krinkle commented 11 years ago

If you let a loader put them in a random order then yes one would have a scale issues and have trouble locating modules. But since you're asking me/us personally, the answer is no, we don't have an issue with it because we don't have a system that (as a side-effect) shuffles them by design throwing them in a random order.

If the order is logical, it is logical. This can be alphabetical or not, whatever your system does, whatever your team considers convenient (e.g. "main.a - main.Z, extra.a - extra.Z" or "core, utilities, child classes a-z" or "everything a-z").

It isn't about a "random order" vs. "structured order". It is about a structured order controlled by you vs. a forced/uncontrollable order by QUnit.

Maintaining alphabetical order wouldn't be put on humans. Especially in a large project this would never be the case because the tests would be loaded by a system (you're not gonna manually list out each <script> tag). Depending on this 'system' it can load things in a logical order or not.

Anyway, urlConfig is out of the question but QUnit.config seems do-able. So like we have QUnit.config.autostart, QUnit.config.reorder we could have QUnit.config.sortModuleDropdown.

Though this does have one catch. Like any config option, it needs to be set before init after QUnit is loaded.

Even in async loaders, you'll be able to manage dependencies (otherwise you wouldn't even be able to ensure QUnit is loaded before the test suites, and the test suites before your main application). So ensure that your configuration file is loaded right after qunit.

jaredwy commented 11 years ago

I am not proposing to change the order tests are run. The pull request is only changing the order of the dropdown.

This is to facilitate quickly locating a specific module you wish to run.

Something that, having spoken to a few people is something desired by a few.

On 19/01/2013, at 8:35 PM, "James M. Greene" notifications@github.com wrote:

It depends on what you think the order of your tests is. If they are arbitrarily ordered, then yes, it will likely be impossible to keep track of them in the dropdown. However, as @Krinkle mentioned, if you take a project like jQuery core, then the order of tests is very intentional: it starts with the most "core" code and spirals its way outward to the most high-level code.

While I don't agree with @Krinkle's proposal to just order your tests in advance to be alphabetical, I do agree with his concern over bloating the UI with options... which again brings me back to my previous proposal of utilizing a bookmarklet — or, if clicking a bookmarklet on the occasions when you do need to use the module filter dropdown is too much effort, then just attach the functionality that the bookmarklet provides to a QUnit.done callback handler to sort the dropdown after the full test run has completed on your test page.

— Reply to this email directly or view it on GitHub.

jzaefferer commented 11 years ago

Sorting just the options in the selectmenu seems fine to me. If we do that, we should just do it, without adding a configuration option.

Since there seems to be some confusion - @Krinkle @JamesMGreene what do you think about just sorting the <option>s?

jaredwy commented 11 years ago

Thanks Jörn.

On 20/01/2013, at 3:35 PM, Jörn Zaefferer notifications@github.com wrote:

Sorting just the options in the selectmenu seems fine to me. If we do that, we should just do it, without adding a configuration option.

Since there seems to be some confusion - @Krinkle @JamesMGreene what do you think about just sorting the

— Reply to this email directly or view it on GitHub.

JamesMGreene commented 11 years ago

@jzaefferer: I'm not strongly opposed to sorting the dropdown if you aren't. My [perhaps unwarranted] concern was that having the module filter in execution order was the current preference, i.e. preferred by an important QUnit consumer like jQuery core.

Krinkle commented 11 years ago

Yeah, sort the modules before generating the <option> elements. Then if any modules are added dynamically after the QUnit.start event, append to the bottom like we do.

jaredwy commented 11 years ago

Pull request referenced at the top of this issue already does that.

Jared.

On 21/01/2013, at 12:13 AM, Timo Tijhof notifications@github.com wrote:

Yeah, sort the modules before generating the

— Reply to this email directly or view it on GitHub.