Adding feature specs to project

[bpurinton]

These are just some basic feature tests mostly focused on visiting pages and checking content. There are a couple of more advanced tests relevant to this project to test for the presence of authenticity token fields and for proper DELETE requests.

[jelaniwoods]

@bpurinton nice work! I realize, I've been assuming these tests are supposed follow best practices and not do the unconventional readable appdev 1 style specs, is that correct?

If not, there are more recent notes about the hacks I did for the AD1 specs you can reference.

[jelaniwoods]

Same feedback as the previous PR. Best practices suggest that you should prefer using let when creating variables that are used across multiple examples.

[bpurinton]

@jelaniwoods I understand using let for instance variables / database objects and can make that change. But my use of before here and elsewhere was to DRY up the visit requests in each describe block. Is there a "correct" way to do that? Or should I just put the visit in each it block?

Update
I did some research, and looked at specs for other projects. Looks like I'm best off just re-using visit in each it block, so going to do that now.

[jelaniwoods]

Is there a "correct" way to do that? Or should I just put the visit in each it block?

From what I understand the only issue with before is that you shouldn't define variables in that block. I think the preferred approach is something like this:

  let(:movie) do
    Movie.create(title: "My title", description: "My description")
  end

  before do
    visit "/movies/#{movie.id}"
  end

See shared examples.

But again, if these tests don't have to follow "best practices", no need to worry about this too much.

[bpurinton]

I'm just following your style guide in this case and putting visit inside each block.

[bpurinton]

@bpurinton nice work! I realize, I've been assuming these tests are supposed follow best practices and not do the unconventional readable appdev 1 style specs, is that correct?

If not, there are more recent notes about the hacks I did for the AD1 specs you can reference.

@jelaniwoods I have looked through the notes already, but it's a lot to process when I'm just trying to get some simple, functional tests. These Phase II specs are a rush job at the moment, @raghubetina just wants something to track progress and these tests might all change in the near future. I think it's better to leave any inconsistencies compared to Phase I tests for a refactoring later on. In theory, since the students have become very familiar with rails grade up to this point, some changes in wording / structure of the tests won't be too jarring?

[jelaniwoods]

@bpurinton

I have looked through the notes already, but it's a lot to process when I'm just trying to get some simple, functional tests.

Yes it's definitely a large guide. It was very recently updated, so I wasn't sure if you knew about it.

raghubetina just wants something to track progress and these tests might all change in the near future.

Understood. Then you can largely ignore my style comments then. The exception is the authenticity_token test, doesn't pass for me when I update the form to use form_with. Was that passing for you?

[bpurinton]

Understood. Then you can largely ignore my style comments then. The exception is the authenticity_token test, doesn't pass for me when I update the form to use form_with. Was that passing for you?

Oddly no, and I spent too long trying to dig into it, I'm working on a draft PR here and have removed the authenticity_token test in that case. Not going to spend all day trying to figure that out, just going to drop it and move on for now.

I know that sounds a little sketchy, but the point of this specific project is that they hand-write the authenticity_token code. So it's good to check for that, and actually good that this doesn't pass with form_with.

[jelaniwoods]

I know that sounds a little sketchy, but the point of this specific project is that they hand-write the authenticity_token code.

Understood. It should be fine for this project then. LGTM 🚀