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

User settings mutation #2270

Merged
merged 2 commits into from
Apr 21, 2017
Merged

Conversation

mjankowski
Copy link
Contributor

@mjankowski mjankowski commented Apr 21, 2017

There was a setting assignment in the settings/preferences controller which had a side effect of mutating the global setting defaults, which then caused an issue in the api notifications spec, but only when the preferences spec ran first, which is why the failure was intermittent.

This should fix what have been intermittent CI failures like https://travis-ci.org/tootsuite/mastodon/jobs/224311789

I left an xit spec in here with a note about whats happening, because I don't know how to resolve this. It seems like that assignment style should not be mutating the setting defaults, but that's probably happening in the rails-settings-cached gem itself, or in our interaction with it.

Steps to replicate the failure, prior to these changes:

rspec ./spec/controllers/api/v1/media_controller_spec.rb[1:1:3:4] ./spec/controllers/api/v1/notifications_controller_spec.rb[1:1:1:2,1:1:1:3,1:1:1:4,1:1:2:2,1:1:2:4] ./spec/controllers/settings/preferences_controller_spec.rb[1:2:2] ./spec/services/fan_out_on_write_service_spec.rb[1:1] --seed 2887

@Gargron Gargron merged commit ee0c897 into mastodon:master Apr 21, 2017
@mjankowski mjankowski deleted the user-settings-mutation branch April 21, 2017 18:52
seefood pushed a commit to Toootim/mastodon that referenced this pull request Apr 28, 2017
* Add user spec for settings, highlight global default mutation issue

* Fix mutation issue caused by settings/preferences spec
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

Successfully merging this pull request may close these issues.

None yet

2 participants