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

Test failure: "effects: jQuery.easing" #255

Closed
Krinkle opened this issue Mar 30, 2017 · 14 comments
Closed

Test failure: "effects: jQuery.easing" #255

Krinkle opened this issue Mar 30, 2017 · 14 comments

Comments

@Krinkle
Copy link
Member

Krinkle commented Mar 30, 2017

This has been failing for a while now.

Commits in jquery/jquery from 13 March:

screen shot 2017-03-30 at 11 38 47

screen shot 2017-03-30 at 11 38 55


  See also jquery/jquery#3583

@Krinkle
Copy link
Member Author

Krinkle commented Jul 18, 2017

@dmethvin fd324d2 fixed the test in most browsers, except Edge 15. It seems the sixth assertion doesn't happen in Edge 15, causing the test to timeout.

I've already re-ran it a few times to make sure it (probably) isn't an intermittent issue.

http://swarm.jquery.org/job/5790
screen shot

Mozilla/5.0 (Windows NT 10.0; Win64; x64) [..] Edge/15.15063

screen shot

@dmethvin
Copy link
Member

Yeah that's strange because I ran the test locally here (Microsoft Edge 40.15063.0.0) and it worked fine. However, I updated a BUNCH of stuff to get this to work including a move to QUnit 2.0 and found a subtle unit test bug in the process. Maybe this is one of those, I'll take a look.

The strange thing is, the unit tests won't run in PhantomJS 2 for me but they do in CI. Go figure. Probably another Windows thing.

@dmethvin
Copy link
Member

This has me really puzzled because it is running fine on my local MS Edge. I thought maybe it was the fact that we were using expectWarning async and changed that, but no effect on the timeout. I can stumble around and change other things but for now I think I'll leave this as-is. At least all the other broken things are fixed. I still need to get tests running locally with PhantomJS 2 on Windows.

@dmethvin dmethvin reopened this Jul 19, 2017
@Krinkle Krinkle closed this as completed Aug 18, 2017
@Krinkle Krinkle reopened this Aug 18, 2017
@Krinkle
Copy link
Member Author

Krinkle commented Aug 20, 2017

@dmethvin When running the jQuery Migrate tests through BrowserStack Live on the same OS/Edge version, they also pass (see the Aug 14 build at http://swarm.jquery.org/project/jquerymigrate, which I manually re-ran).

Looks like it only fails through the BrowserStack API workers. Unfortunately, no error message is logged per jquery/testswarm#316. The test just ends for some unknown reason and yet uploads the HTML without any error.

@Krinkle
Copy link
Member Author

Krinkle commented Aug 20, 2017

I've fixed jquery/testswarm#316. There is no error because the upload is triggered by a timeout. However, it seems part of the inject.js script was out of sync with the current db schema and wrongly using the "error" status code instead of the "aborted" (timeout) error code. This is now fixed.

The manual run for Edge 15 / jQuery Migrate completes in about 10 seconds.
http://swarm.jquery.org/job/5977 / http://swarm.jquery.org/result/2383293

The BrowserStack worker times out after 70 seconds in QUnit test 42/43 ("42. effects: jQuery.fx.interval - no warning on animations").
http://swarm.jquery.org/job/6014 / http://swarm.jquery.org/result/2383436
screen shot 2017-08-20 at 14 53 43

So presumably whatever completes this async test, it sometimes isn't triggered for over 60 seconds in Edge 15.

QUnit.test( "jQuery.fx.interval - no warning on animations", function( assert ) {
assert.expect( 1 );
var start = assert.async();
// Can't use expectNoWarning since this is async
jQuery.migrateReset();
jQuery( "<div />" )
.appendTo( "#qunit-fixture" )
.animate( { opacity: 0.5 }, 50, function() {
assert.equal( jQuery.migrateWarnings.length, 0, "no warning" );
start();
} );
} );

I'm guessing the animation completion depends on the browser calling requestAnimationFrame - which presumably isn't happening for a while. Might be that the VM doesn't have the browser window in the foreground, which might enable some kind of idle/optimisation mode in Edge, similar to Chrome. Except as far as we can tell, the Chrome VMs do have the browser window in the foreground. (I'll file a browserstack issue)

@mgol
Copy link
Member

mgol commented Aug 21, 2017

@Krinkle BrowserStack provides videos of all test runs. It's a little hard to locate which run represents which TestSwarm test but it's enough to log in to TestSwarm and restart a job while looking at https://www.browserstack.com/automate/.

The browser is not covered by anything but the resolution is low enough that the test iframe doesn't fit in the screen. This might be what's causing animation issues.

I'm not sure what to do about it. Even if resolution was greater, the list of test runs scheduled for this browser displays above the iframe so the iframe does go out of view eventually.

Resolving that may require changes in TestSwarm itself.

screen shot 2017-08-21 at 11 25 11

@Krinkle
Copy link
Member Author

Krinkle commented Aug 22, 2017

@mgol The manual tests I ran also used the TestSwarm runner page. I tried to reproduce it just now and I think you're right. When I did it last week, I had the window maximised in BrowserStack (as is the default) and it was just tall enough to have part of the iframe visible. When I resized the window like in your screenshot, the test started to stall until I scrolled down at which point it finished immediately.

@dmethvin
Copy link
Member

Ugh, that explains why it works fine outside of TestSwarm. The iframe on on the TestSwarm page has focus all during the testing though, right? Maybe this is something we should ask the Edge team to change?

@mgol
Copy link
Member

mgol commented Aug 23, 2017

In my testing for jquery/jquery#3151 I've discovered browsers are getting more & more aggressive at delaying computing stuff for elements out of the visible screen area. This doesn't just apply to iframes, though these optimizations cause different side effects in different browsers.

Even if the Edge team changed that here we'd still have a similar problem in many other browsers. In jquery/jquery#3151 I even had to make sure the tests that create their own iframes are doing it at the top of the QUnit test window as otherwise they might have lost focus as well.

I think there's just no way around that (long term) than changing the TestSwarm layout to make sure the iframe is always visible and perhaps even changing our QUnit setup so that iframes created by tests appear at the top, not at the bottom. Perhaps it may even require changes in QUnit itself (cc @leobalter).

@dmethvin
Copy link
Member

I wonder if we could just override jQuery.fx.start/stop in the unit tests to avoid rAF entirely? For the current things that Migrate does it doesn't matter whether rAF is in use or not. Since there isn't anything browser-specific in it either I suppose we could skip this test in Edge.

Neither solution is ideal, but having the unit tests consistently fail is worse.

@Krinkle
Copy link
Member Author

Krinkle commented Aug 23, 2017

The iframe on on the TestSwarm page has focus all during the testing though, right?
Maybe this is something we should ask the Edge team to change?

Yes, the window has focus, but it seems Edge 15 introduced an optimisation where iframes that are out-of-view iframes within a foreground window or tab, are also considered in the "background", like a tab or window that is not in the foreground.

I might still be worth reaching out to Edge 15 if this was unintentional. Also, it'd be good to verify whether Edge is emitting the proper Page Visibility API or window focus/blur events when they make this decision. As long as they're doing that, I wouldn't consider this a bug.

And yep, agreed, we should try and move the iframe to higher in the viewport. I'll see what I can do.

@mgol
Copy link
Member

mgol commented Aug 23, 2017

In terms of unbreaking the build ASAP I agree with @dmethvin that we can either skip a problematic test in Edge 15 via UA sniffing or mock it to skip rAF. My remarks were more of a general problem that will hit us from time to time and that require changes beyond jQuery or the Migrate prokect.

@Krinkle
Copy link
Member Author

Krinkle commented Aug 23, 2017

After jquery/testswarm@a46a9a6, the build is passing now!

screen shot 2017-08-23 at 20 31 00

@Krinkle Krinkle closed this as completed Aug 23, 2017
@dmethvin
Copy link
Member

YAY! 🎉

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

3 participants