Isolation of components during testing #1296
Conversation
[ci skip]
…ot included. For reasoning, see #1292
This is because, due to isolation, there are no roles for users inside of Core
Why? Well, because Core never has a concept of current_user. This only needed to be stubbed because auth was inadvertently being included when we were running the tests
…ove it into auth's decorator for the same
… object, so no database call is done until necessary
…e own password in admin backend)
This is because core has no concept of authentication/authorization.
…a tax_category or shipping_category object. This is because most product tests don't need these objects. By avoiding creating them, the tests that use these attributes should be faster
…int also having a controller spec for it in admin/products_controller_spec
…analytics_tracker_spec
This is to address the problem in #1292.
… the tests. This is because otherwise the libraries that Bundler load tend to leak. So what we want to do is go into every single directory and run the tests JUST for that component. One by one.
…runs. This is because Travis automatically sets this to be the Gemfile at the beginning of the test run which would cause all components to be used. What we want instead is for each component to use just its own Gemfile.
…by both spec/models/payment_method_spec and spec/requests/admin/payment_method
This is because when the test goes through the checkout process it fails on the address page due to lack of email. We *could* just fill out the email field, but there simply isn't one in this case... therefore we do this small hack.
…d as dependencies in the Gemfile
…d is from core, and so won't be called in core's tests anymore
|
Build is all fixed now: http://travis-ci.org/#!/spree/spree/builds/908921. This can be merged into master. |
|
You're the man Ryan! 🍺 🍺 |
|
Based on a discussion I had with Sean yesterday, these changes probably won't make it into 1-0-stable, but will be applied directly to master -- and therefore contribute to the 1-1-stable release. A patch release, in our opinion, should not introduce so many large changes like this. The patch cmar gave to fix the issue will be released in 1.0.4, meaning your extensions tests should work on that version. We'll be making sure of that before we do a release. We'll be releasing 1-1-stable later this week most likely. |
This commit contains a whole bunch of changes, from pull request #1296 which will fix #1292. To understand the gist of the problem, please see the comment threads in The main bunch of work that was done here was done in two parts: remove all current_user references from Core and to isolate each component during its testing so that it is not loading all of Spree for each component, just the bare-bones components that are needed. For instance, Core now will only load Core. API will now load API, Auth and Core. Dash will load Auth and Core. Promo will load Auth and Core.
|
Merged in @6dce9ba8b25c3a577255f08be37a2d9bc2a30b99. |
|
Thanks so much Ryan, I'm looking forward to the 1.0.4 and 1.1.0 releases! |
|
hi @radar, Is this issue fixed in spree_core 1.0.4 ? my extension's spec are failling when updating to 1.0.4 |
|
I'm still seeing broken tests on 1.0.4 as well... |
|
This would've been nice to know before we released :\ Why weren't you testing on 1-0-stable? Could you please provide more information about the broken tests you are encountering? |
|
The issue I'm still seeing is issue #1292 (current_user method error) I'll test against the 1-0-stable branch in the future, but it would be nice if the spree team tested new versions against some popular extensions before they are released as well.
On another note, 1.1.0 is working great without any breakage. Thanks!! |
|
@radar well, you said
So I was basically waiting for 1.0.4 release..I will also test again 1-0-stable in the future. |
|
So is this fixed in 1.0.4 ? |
|
@ygor: Do you have reason to think that it's not? |
|
Well, I'm trying to run the tests on spree_essential_cms and spree_essential_blog and get the missing current_user method error, which was exactly the cause for this fix.. |
Hi,
There was an issue (#1292) brought up yesterday where Spencer attempted to run tests for an extension only to run into an issue where we were referencing
current_userinside of Core when we should truly never be doing something like that. All references tocurrent_usershould be in theauthcomponent.To ensure that something like this never comes up again, I've taken the time to separate the testing of the individual components of Spree and have run the tests to determine if there were any more places where issues like #1292 would come up. There were, and they have been fixed.
When the build passes for the isolation branch on Travis, it should be good to merge this in... providing that everyone agrees that this is the way forward.