Conversation
Swiftda01
commented
Feb 11, 2020
Swiftda01
commented
- Controller action
- Route
- Logout button
- Manual testing
| expect(route.verb).to eq('DELETE') | ||
| end | ||
| end | ||
| end |
There was a problem hiding this comment.
If you do end up adding destroy to the resources array then presumably this spec is no longer needed? I've actually never used routing specs before but I'm guessing they're only needed for custom routes?
There was a problem hiding this comment.
I don't think the fact that it's a custom route makes it any more or less deserving of tests. These are just unit tests to test a piece of functionality - the functionality here being that the expected route has been generated. Currently our other routes are being tested indirectly through feature specs, but since we don't have one of those for logging out yet I didn't want to add an untested piece of code to the codebase.
I followed the Hanami guides for this: https://guides.hanamirb.org/routing/testing/
There was a problem hiding this comment.
Interesting.. there's a part of me that just thinks this is excessive. It seems to be basically testing that the Hanami routes library is working correctly, rather than testing what we've been doing. If we write a spec for one of these routes then there's no argument really against writing one for all of them. Does that seem worthwhile to you? That's not rhetorical, genuinely interested to hear your thoughts.
There was a problem hiding this comment.
Hmm to be honest I don't see this as testing the Hanami routes library, I see it more as checking that the delete sessions route exists for our application and is working as expected.
The reason I added this test is because I was adding functionality to our application which is not covered by an integration-style feature test like our other routes are (as there is no feature that uses this route yet). So I added these specs to ensure that this new unit of code is tested. If I hadn't, then it would be an untested piece of functionality, meaning that for all I know it could be broken or not behaving as expected - or if it was changed or removed then there would be no failing tests to reflect this.
Having individual unit tests for routes also means that if you change or remove the feature test, then you can rest assured that this unit of code is still properly tested in isolation. We already follow this principle in other areas of the codebase such as having a test for web/controllers/users/new.rb when strictly speaking it is already covered by the create user feature specs.
I personally I like the belt and braces approach of having both unit tests and overall feature tests to ensure the test coverage is robust, but if you feel it's excessive in this instance then I'm happy to leave these tests out for routes.
This was my thinking behind adding these tests, but obviously I'm still very new to Hanami so interested to hear your thoughts
2 participants
