Do not skip generator on :revoke behavior

[mauriciopasquier]

This PR fixes the rails generators when run in reverse, that is with rails destroy, so they remove the corresponding generated migrations.

I didn't add tests because I'm not sure if this behavior is intented, maybe to be cautious about what you remove? Let me know if you want me to add them. Also, thanks for your amazing work with this gem and globalize ^^

[shioyama]

Thanks very much! I didn't even know about rails destroy, thanks for teaching me something new 😄

I didn't add tests because I'm not sure if this behavior is intented, maybe to be cautious about what you remove?

I think it sounds like a good idea, could you add specs for this? Then I can merge it.

Thanks again!

[mauriciopasquier]

No problem! ^^
I'm having some difficulties reproducing this destroy behavior on the specs, but I'm on it.

[shioyama]

Hmm... it looks like generator_spec doesn't support destroy, which makes this more tricky to test. I think unless you've got something already, it's fine to merge without a spec for this one, and maybe add one later.

[shioyama]

I merged this in abff813, but if you'd like to add a spec please go ahead 😄

[mauriciopasquier]

Thank you!

For what it's worth, these are the specs I have right now.

I still don't understand why, but invoking the generator with behavior: :revoke from the spec does remove the migrations, unlike calling rails destroy mobility:install from a rails app.

[shioyama]

Still not quite sure why that spec isn't working, but the reason it passes even without the change is that in this line:

mobility/lib/rails/generators/mobility/install_generator.rb

Line 38 in fb46c07

if behavior == :invoke && self.class.migration_exists?(migration_dir, template)

The conditional self.class.migration_exists?(migration_dir, template) returns false in any case.

The problem would seem to be that migration_dir is not using the correct (test) root, so I changed it to:

migration_dir = File.expand_path(destination_root, "db/migrate")

which I can check actually has the files, but still migration_exists? returns false... puzzled.