Remove last migration's spec file #3415
Conversation
This migration spec has done its job for users upgrading up to v2.9 and now I think we can safely remove it assuming users are upgrading Solidus one version at time. Also, the migration is quite complex since it uses some ActiveRecord internals that may change with time.
There was a problem hiding this comment.
IMO there's still some value in leaving these tests be as long as they don't become problematic.
I'm thinking they work as a good example for future migration tests, the migration is still there, so having that covered by tests is good as long as these tests work, and a lot of effort was put into writing them and making them work... but maybe it's time to let them go 😄
So, I'm fine with whatever the core team decides to do... thank you @kennyadsl 👍
|
@spaghetticode I'd love to keep it but, while testing a branch with another migration, I've found some issue that I didn't consider adding this spec. For example, as is, it gives for granted that the migration under test is the last one, and if/when we'll add a new migration this spec will need some adjustment. This random experiment showed the weakness of testing migrations. Also, there's a lot of boilerplate code, which I tried to extract into some RSpec migration helpers here, which I leave for reference if we want to add it back for new migrations. Going to merge this since it seems like I have a stronger opinion here. 🙂 |
Description
This migration spec has done its job for users upgrading up to
v2.9and now I think we can safely remove it assuming users are upgrading Solidus one version at the time.Also, the migration is quite complex since it uses some ActiveRecord internals that may change with time.
Checklist: