Fixing the test for pushstate #652 #690

Merged
merged 1 commit into from Jan 22, 2014

Conversation

Projects
None yet
2 participants
@andykant
Contributor

andykant commented Jan 22, 2014

The test for pushstate #652 would occasionally fail due to timing issues, this should ensure that it doesn't run into the issue of the window not being ready.

@stevenvachon

This comment has been minimized.

Show comment
Hide comment
@stevenvachon

stevenvachon Jan 22, 2014

Contributor

I originally had it at 200ms, but it would randomly fail when the browser took longer to load the page. Good idea on the multiple setTimeout's!

Contributor

stevenvachon commented on 208b632 Jan 22, 2014

I originally had it at 200ms, but it would randomly fail when the browser took longer to load the page. Good idea on the multiple setTimeout's!

@andykant

This comment has been minimized.

Show comment
Hide comment
@andykant

andykant Jan 22, 2014

Contributor

@stevenvachon Yeah, it was still randomly failing even at 1500ms (not often, but it happens). Difficult to replicate unfortunately, so I'm hoping this fixes it.

Contributor

andykant commented Jan 22, 2014

@stevenvachon Yeah, it was still randomly failing even at 1500ms (not often, but it happens). Difficult to replicate unfortunately, so I'm hoping this fixes it.

@stevenvachon

This comment has been minimized.

Show comment
Hide comment
@stevenvachon

stevenvachon Jan 22, 2014

Contributor

Sorry about that. Wish I'd thought of that multiple timeout idea. I think it will fix it.

Contributor

stevenvachon commented Jan 22, 2014

Sorry about that. Wish I'd thought of that multiple timeout idea. I think it will fix it.

@andykant

This comment has been minimized.

Show comment
Hide comment
@andykant

andykant Jan 22, 2014

Contributor

No worries, honest mistake. That method I used is a good general hack for those types of tests. Not pretty but it works. :)

Contributor

andykant commented Jan 22, 2014

No worries, honest mistake. That method I used is a good general hack for those types of tests. Not pretty but it works. :)

andykant added a commit that referenced this pull request Jan 22, 2014

@andykant andykant merged commit eb75293 into master Jan 22, 2014

1 check passed

default The Travis CI build passed
Details

@andykant andykant deleted the pushstate_652_test_fix branch Jan 22, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment