Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

AO3-4818 Complete coverage of prompt meme controller #3166

Open
wants to merge 2 commits into
from

Conversation

Projects
None yet
2 participants
Contributor

hatal175 commented Nov 17, 2017

Issue

https://otwarchive.atlassian.net/browse/AO3-4818

Purpose

Complete coverage of prompt meme controller

Testing

Run tests.

Credit

Tal Hayon

Please use he.

+ post :new, params: { collection_id: @collection.name }
+ end
+
+ it "should set flash notice" do
@sarken

sarken Nov 19, 2017

Owner

Because it's something we test a lot, we've set up a bunch of redirect expectation helpers that we prefer to use for testing redirects with notice or error messages. For example, we might condense this down to:

context "when the collection already has a challenge" do
  [...]

  it "redirects to challenge edit page with notice"
    it_redirects_to_with_notice(edit_collection_prompt_meme_path(@collection), "There is already a challenge set up for this collection.")
  end

Generally, we prefer to leave off the "should" in our descriptions, per these guidelines.

+ end
+
+ describe "update" do
+ context "when it fails to udpate parameters" do
@sarken

sarken Nov 19, 2017

Owner

Little typo here -- should be "update."

+ describe "update" do
+ context "when it fails to udpate parameters" do
+ before do
+ challenge = PromptMeme.new
@sarken

sarken Nov 19, 2017

Owner

It looks like the only reference to the challenge variable here is in @collection = FactoryGirl.create(:collection, challenge: challenge). Would it be possible to avoid creating the variable and do FactoryGirl.create(:collection, challenge: PromptMeme.new) here like in the test above?

You can leave off FactoryGirl for brevity if you like and it should still work.

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