Skip to content
This repository was archived by the owner on Jul 24, 2020. It is now read-only.

#1059 Intermittent Tests #1069

Merged
merged 1 commit into from
Jan 7, 2015
Merged

#1059 Intermittent Tests #1069

merged 1 commit into from
Jan 7, 2015

Conversation

orenyk
Copy link
Contributor

@orenyk orenyk commented Dec 19, 2014

Resolves #1059, currently not doing the trick :-\

@orenyk
Copy link
Contributor Author

orenyk commented Dec 19, 2014

Ok, the last commit passed on its first run. I think we can merge this in ASAP and see if it helps across other branches / builds.

@orenyk
Copy link
Contributor Author

orenyk commented Dec 19, 2014

Spoke too soon, I guess :-\

@mnquintana
Copy link
Contributor

It looks like it's passing now

@orenyk
Copy link
Contributor Author

orenyk commented Dec 19, 2014

Yea, after I re-ran the build five or six times. Clearly not the fix :-\

@orenyk
Copy link
Contributor Author

orenyk commented Jan 7, 2015

I'm going to try to deal with this so we can get accurate builds again.

@orenyk
Copy link
Contributor Author

orenyk commented Jan 7, 2015

Ok, when I run just that file the test passes, but not when I run the whole test suite. The plot thickens...

@mnquintana
Copy link
Contributor

@orenyk If it comes to it, Travis did offer last summer to send us a copy of the VM our tests are running on so we could test for system config issues that might be affecting the build.

@orenyk
Copy link
Contributor Author

orenyk commented Jan 7, 2015

Alright, binding.pry confirms that subject.invoke does actually change Reservation.count as expected. I really can't figure out why this is failing.

@orenyk
Copy link
Contributor Author

orenyk commented Jan 7, 2015

It's not only a Travis issue, it also happens locally. Running rspec spec/lib/tasks/delete_missed_reservations_rake_spec.rb passes but just running rspec fails on that test. Still debugging...

@orenyk
Copy link
Contributor Author

orenyk commented Jan 7, 2015

Ok, this is weird. It appears as though the order of the tests matters; when it runs at the start of testing, it passes, so we must have some weird db thing going. I'm going to re-run and see what happens.

@mnquintana
Copy link
Contributor

I was actually gonna ask about that – I was wondering if one of our tests was making some lasting db change that were affecting this test.

@orenyk
Copy link
Contributor Author

orenyk commented Jan 7, 2015

Ok, this is even weirder. I've sandwiched the subject.invoke call between two binding.pry calls to verify that Reservation.count actually changed, and it passed even when it was later in the test suite.

RSpec shouldn's allow for that to happen, lasting DB changes should be impossible.

@orenyk
Copy link
Contributor Author

orenyk commented Jan 7, 2015

On the other hand, sandwiching the call between two puts Reservation.count calls shows that, indeed, the reservation count isn't actually changing. I don't know what the difference is between that and binding.pry, binding.pry shouldn't make any difference...

@orenyk
Copy link
Contributor Author

orenyk commented Jan 7, 2015

Huh, if I simply exit out of the binding.pry breakpoint without calling anything, the tests fail. WTF.

@orenyk
Copy link
Contributor Author

orenyk commented Jan 7, 2015

Oooooooooook, so it's definitely order-dependent, b/c now I can't replicate my 'passing when I call Reservation.count in a breakpoint' solution. So weird.

@orenyk orenyk force-pushed the 1059_intermittent_tests branch from bc2b2da to 32dfe77 Compare January 7, 2015 18:14
@orenyk
Copy link
Contributor Author

orenyk commented Jan 7, 2015

WOOOOOOOOO! Figured it out. For some reason we were creating multiple App Configs and therefore when the rake task checked AppConfig.first it wasn't necessarily getting the right one. Whew. Cleaning this up and then ready for review.

@orenyk
Copy link
Contributor Author

orenyk commented Jan 7, 2015

DONE (build 2001, btw)!

@mnquintana
Copy link
Contributor

@orenyk ZOMG

@mnquintana
Copy link
Contributor

@orenyk @squidgetx That reminds me though – AppConfig should really be a singleton class... thoughts? (ie. there should never be more than one AppConfig instance)

@orenyk
Copy link
Contributor Author

orenyk commented Jan 7, 2015

Yes please :-). That totally makes sense, we're always checking for AppConfig.first anyway, so if we can make that more rigorous that would be awesome. I don't know much about singleton classes but I'll look into it (and open an issue).

@squidgetx
Copy link
Contributor

yesssssssssssssss

@squidgetx
Copy link
Contributor

code looks solid

@orenyk
Copy link
Contributor Author

orenyk commented Jan 7, 2015

great squashing / merging

commented out funky tests

putting the sleep where it counts

comment out the right test

fixed broken spec
@orenyk orenyk force-pushed the 1059_intermittent_tests branch from e1e3d92 to ce38dae Compare January 7, 2015 23:37
@orenyk
Copy link
Contributor Author

orenyk commented Jan 7, 2015

woot green build!

orenyk added a commit that referenced this pull request Jan 7, 2015
@orenyk orenyk merged commit 26fb80e into master Jan 7, 2015
@shippy shippy mentioned this pull request Jan 22, 2015
4 tasks
@mnquintana mnquintana deleted the 1059_intermittent_tests branch February 20, 2015 13:50
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Intermittently failing tests
3 participants