Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tests fail in mobile browers #908

Closed
akuckartz opened this issue Dec 18, 2012 · 18 comments
Closed

Tests fail in mobile browers #908

akuckartz opened this issue Dec 18, 2012 · 18 comments

Comments

@akuckartz
Copy link

The following tests fail when using the Android Brower which is part of Android 2.2.1 on a Samsung Galaxy S:

    1. Function: throttle (1,1,2)
    1. Function: throttle repeatedly with results (3,6,9)
    1. Function: throttle triggers trailing call after repeatedly invoked (1,1,2)
    1. Function: debounce (1,0,1)
    1. Objects: functions (1,1,2)

The following tests fail when using Firefox 18 Beta on the same device:
51, 52, 53

The following tests fail when using Opera Mobile 12.10 on the same device:
51, 52

No failed tests are reported by the Windows desktop versions of Firefox 17.0.1 and Chrome 25.0.

@jdalton
Copy link
Contributor

jdalton commented Dec 18, 2012

The tests 47, 51, 52, 53, use setTimeout which can suffer some drift and cause false fails. If you rerun the individual tests (qunit has a rerun link next to each test) you may see that they actually pass.

As for 64, which of the 2 tests failed?

@akuckartz
Copy link
Author

When I separately rerun 47, 51, 52, 53 they succeed, but 51 does not.

64.1 ("can grab the function names of any passed -in object") fails, 64.2 does not fail.

In any case false fails are irritating because something which is not working on those mobile devices for unknown reasons uses underscore and other libraries. (http://szabyg.github.com/annotate.js/)

@jashkenas
Copy link
Owner

I'd love to bulletproof the setTimeout based tests a bit more -- perhaps just adding more time would do it (although it would slow down the suite).

@jdalton
Copy link
Contributor

jdalton commented Dec 18, 2012

@jashkenas

I'd love to bulletproof the setTimeout based tests a bit more -- perhaps just adding more time would do it (although it would slow down the suite).

I've done some work on this in Lo-Dash, adding time helps a bit (though I still run into false fails from time to time), but avoiding exact counts is the biggest bet. For example if a test should just execute more than once, checking that counter > 1 will do.

@jashkenas
Copy link
Owner

For example if a test should just execute more than once, checking that counter > 1 will do.

I'd be wary -- given that some of our debounce-ish bugs have to do with the exact number of times it should be triggering within a given time window.

Very annoying though...

@jdalton
Copy link
Contributor

jdalton commented Dec 18, 2012

@akuckartz Can you run this page in Andriod 2.2.1:
http://jsbin.com/onogad/1

@akuckartz
Copy link
Author

@jdalton

I get this using the Android browser which is part of Android 2.2.1:

typeof /x/ is. Expected: object; Got: function;

_.isFunction(/x/) is. Expected: false; Got: true

(Firefox 18 Beta got the expected results)

@caseywebdev
Copy link
Contributor

I wonder what /x/() does since android thinks RegExps are functions...

@ghost
Copy link

ghost commented Dec 18, 2012

@caseywebdev It's equivalent to RegExp#exec. [[Call]]-able RegExps (typeof /x/ == 'function') are an ancient (Netscape 4, IIRC) behavior inherited by older versions of Gecko, JSC, and V8.

@caseywebdev
Copy link
Contributor

Kinky!

@jdalton
Copy link
Contributor

jdalton commented Dec 18, 2012

@akuckartz Ok, thanks, can you run this too http://jsbin.com/onogad/2

@jdalton
Copy link
Contributor

jdalton commented Dec 18, 2012

@jashkenas Found it. The minified Underscore has the isFunction fallback check, if (typeof (/./) !== 'function') {, removed and so assigns

w.isFunction = function (n) {return "function" == typeof n}

@akuckartz
Copy link
Author

@jdalton The result is:

typeof /./ is. Expected: object; Got: function;

typeof /x/ is. Expected: object; Got: function;

_.isFunction(/./) is. Expected: false; Got: true

lodash.isFunction(/./) is. Expected: false; Got: false

@jashkenas
Copy link
Owner

The minified version has the isFunction check removed? That's horrifying. Bug report for Uglifier?

@jdalton
Copy link
Contributor

jdalton commented Dec 19, 2012

@jashkenas
Ok, I checked and it's not an Uglify bug.
Uglify just needs to be passed the correct options (it prints a warning when minifying).
Passing -c "evaluate=false" will do the trick with the npm package.
The same thing will be required (accounting differences between v1 and v2) for the gem too.

Lo-Dash intentionally uses dead-code removal in its build, but just happened to avoid complications with a check like:

if (isFunction(/x/)) {

@jashkenas
Copy link
Owner

Thanks. Sounds like it's time to bump a release.

jashkenas added a commit that referenced this issue Dec 28, 2012
@jdalton
Copy link
Contributor

jdalton commented Jan 24, 2013

This issue is resolved.

@jdalton jdalton closed this as completed Jan 24, 2013
@jdalton
Copy link
Contributor

jdalton commented Jan 28, 2013

@jashkenas please update the site with the rebuilt minified file.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants