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

fixes spec dependency between rss_feed_data_spec.rb and update_https_spec.rb #120

Closed
wants to merge 1 commit into from
Closed

fixes spec dependency between rss_feed_data_spec.rb and update_https_spec.rb #120

wants to merge 1 commit into from

Conversation

HalahAb
Copy link
Contributor

@HalahAb HalahAb commented Jul 31, 2018

Resets RssFeedUrl's readonly attributes which are modified after invoking the rake task in update_https_spec.rb, and ensures 'url' is not read only.

…rb by reseting RssFeedUrl readonly_attributes
@HalahAb
Copy link
Contributor Author

HalahAb commented Jul 31, 2018

Resolves #59

@MothOnMars
Copy link
Contributor

@HalaH, this is an interesting case. You’ve actually found a bug in the code, rather than a test issue.

The history of the bug you’ve found is that at the time the update_https rake task was written (ffa833f), url was a read-only attribute for the RssFeedUrl class. So that task removed and then added the url attribute to the readonly_attributes list (ffa833f#diff-49d7b19871b49d356286d27431031aedR76).

A subsequent commit made RssFeedUrl#url not read-only (e0c5ea0#diff-3725379569576e16e22f4f689378dab1L7). So that bug you found is that the update_https task is setting a read-only attribute that should not be read-only, which is breaking rss_feed_data_spec.rb. That’s actually a dangerous bug, because it means that the rake task is erroneously setting the readonly_attributes set for RssFeedUrl.

The fix is two-fold:

  1. update the update_https rake task to stop toggling the read-only attributes for RssFeedUrl (because url is no longer read-only for that class
  2. prevent any similar problems for FlickrProfile or any other classes. Instead of setting a static array of read-only attributes for the FlickrProfile and RssFeedUrl classes, we should make the code more flexible so that it does not depend on any knowledge of the classes it’s updating. I’m going to close this PR and push a fix for that, because I want to revisit that code and make sure there’s nothing I’m missing in that approach.

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