pat / combustion

Simple, elegant testing for Rails Engines
MIT License
707 stars 51 forks source link

Q: modern rails, engine testing, and combustion #93

Closed jrochkind closed 5 years ago

jrochkind commented 5 years ago

The "default Rails" installation/infrastructure for engines has changed a lot since Rails 3.0 when combustion was first written.

I forget if Rails 3.0 even installed a "dummy" app. But the dummy app installed now is relatively light weight already (although still not nearly as light-weight as combustion's).

But additionally, the standard Rails engine setup gives you some features that are dependent upon the 'dummy', including: The ability to run generators rails g model whatever, which are generated into your engine, using generator configuration in your engine.rb. The ability to run rake tasks (now rails tasks, sigh) like db:reset or db:migrate, when the current working directory is your engine (no need to move into dummy dir) (through a really weird engine.rake implementation I don't completely understand, but I think makes the same tasks available as eg app:db:migrate and db:migrate). And the default generated testing tasks call things on your "dummy" db similarly to in an ordinary rails app. The default dummy app is generated with some odd code to make these things possible (it's not really a full/standard rails app in "dummy" already).

The standard documented combustion install breaks these rails g and ability to run rake db:migrate things. I was trying to hack it to see if I could make them work again. I got rails g to work, but not the db-related rake tasks (as integrated by default in the generated testing tasks), yet.

But then I thought: Why am I doing this? What benefits is combustion actually giving me? (And on the other hand new versions of Rails can break combustion as in #92. I won't say they might not break Rails default generated engine setup too, since that tends to be a somewhat neglected part of Rails, but it's probably less likely)

Thinking about what value Combustion adds... I am not sure! (I too have been doing engines since Rails 3.0 where the Rails-generated setup was not good enough, I'm not sure that's still true).

I think hypothetically it might make it easier to have proper non-deprecated default defaults when testing different rails versions, since some of that logic for what is default or what config keys are available in what version is embedded in combustion.

It may make it a little bit easier to have DB setup which is not in Rails migrations, which can be convenient. Not totally sure how you'd do that with stock rails-generated engine dummy setup.

But I'm curious your opinion as Combustion author what the benefits of combustion are over the stock generated rails engine setup. I get the feeling you aren't creating too many new engines yourself anymore where it comes up, and Combustion's maintenance status is somewhat tenuous (although it's still far better maintained than lots of rails-related open source, don't get me wrong :) ). But as I try to figure out if I want to be fighting to make combustion work the way I want, or fighting with default generated Rails setup to make it work how I want instead... I'm curious your thoughts.

Thanks for any!

pat commented 5 years ago

Firstly, some thoughts on Combustion's maintenance status: I consider it to be actively maintained. I don't feel there are new features to add, but I respond to bug reports and aim to keep it working with released Rails versions. Testing against Rails' master branch, however, is not something I actively do - in Combustion, or in any of my libraries.

I use Combustion in other gems I maintain - of note, gutentag and thinking-sphinx. As I maintain those libraries against new versions of Rails, Combustion itself gets updated accordingly if required (alongside pull requests from other contributors).


As for the purpose of Combustion? I built it because I was not at all thrilled by the standard engine testing approach that you've mentioned - the dummy app within the spec or test folder, which meant a ton of generated files which were unrelated to the tests.

Hence the goal of having only the files that are needed in spec/internal. This also allows for (mostly) easier testing across Rails versions, as you're not having to muck around with those boilerplate files that change based on how a Rails app is generated.

Your points around Rails' improvements in this area sound balanced to me. I've not taken the approach of generating an engine from Rails in a very long time (I use bundle gem foo instead and then extend the standard gem to be an engine), so it's far more likely you know that side of things better than I. But yeah, I think you're right that Rails will mostly let you run rake and migrate and such (within the engine's context), and Combustion does not give you that ability at all.

I still feel the goals of Combustion remain pretty valid though - but maybe I should try generating a test app within a Rails-generated engine and see if that scenario's improved? I would still be concerned about how testing across Rails versions is handled though.

… so, is Combustion as useful as it once was? Perhaps not. Do I still find it useful? Yes :)

jrochkind commented 5 years ago

Thank you, this is super helpful! Sometimes one just doesn't know what's going on with an anonymous gem on the internet, right? Thanks for the update!

The Combustion approach of course still has a tiny dummy app, it's just a lot smaller/fewer files. The Rails 5.2 dummy app is already way fewer files than it was in 3.0, but still more than combustion.

I guess the question would be... why do we care how many files there are? Aesthetics or "disk space" probably aren't worth the extra dependency and work. I think the reason I cared is because the combustion approach potentially makes it a lot easier to test under multiple Rails versions, or upgrade to new Rails versions in testing. Fewer files means fewer files that might be "wrong" for a different Rails version. I think this is maybe less of an issue than it used to be -- Rails 'core' app/boot files change less than they used to.

And then there's the fact that combustion's db setup could be considered more convenient than the stock Rails one. Which isn't really about fewer/smaller files, it's a separate thing.

I would still be concerned about how testing across Rails versions is handled though.

Yep. I think it should actually be possible to test accross multiple rails versions (say, using appraisal) even with the stock rails-generated dummy app. The stock generated dummy app just doesn't differ that much from version to version anymore, especially in ways that matter for validity/runnability of the tests. But I'm not totally sure.

Mostly I just hate spending time on setting up tooling, like test infrastructure. I am not certain at this point which approach will make me spend less time fighting with it; neither approach is entirely smooth for me, I've tried both, and with both end up fighting to get it set up smoothly. I hate giving up the generator and rake task integration that is now included in stock dummy approach (it didn't exist back in rails 3), which is why I was investigating if I could make it work with combustion, but didn't have complete success there.

If you ever wanted to spend the time to investigate the stock rails-generated test app in modern rails a bit, I would be very curious to hear your thoughts! Compare and contrast with combustion, maybe ideas to make the combustion approach not have to give anything up from the stock approach.

Thanks for sharing your thoughts above, I appreciate it!

pat commented 5 years ago

Just spun up a generated engine via Rails. It still creates a decent amount of files (particularly, everything in config). It does not generate a Gemfile for the app though, which is nice to see - easier to manage testing dependencies purely from the engine's perspective (and thus testing across different Rails versions should be easier).

The amount of generated files is annoying - yes, I'm sure it's possible to clean them up, but that's the kind of stuff that I'm glad that Combustion handles for me. As for using generators: I'll generally opt for writing models and controllers by hand, and my engines don't involve a great many models, so it's not a headache. Certainly, the more complex an engine gets, this could be more frustrating…

… that said, my gut feel is getting Rails' generators to work within a Combustion context is probably more hassle than it's worth. Certainly, you're welcome to investigate further, but sounds like it has already provided a headache or two for you?

jrochkind commented 5 years ago

Actually, I got the generators to work! What I didn't get to work was rake db:reset and other db: tasks, which are also depended upon by the rails-generated test tasks.

To me, just "having fewer files on disk" isn't really an advantage, I don't really care how many files are on disk, especially if I never have to look at them or pay attention to them. I'm kind of surprised that just minimizing files on disks is an important goal for you, but to each their own!

But fighting with testing different rails versions is an issue. That is one of the main draws of combustion for me, indeed. But if appraisal works just fine to test different rails versions even with default rails-generated dummy app... on the other hand, it might work just fine now, but stop when some future version like Rails 6 is released with a different setup. I dunno!

I just don't want to have to re-figure out how to do useful convenient things that 'just work' with the default rails-generated engine setup. But if I can figure em out once and contribute the setup back to combustion (as code or docs), that could be worth it. If combustion is actually doing significant useful things for me, which I'm not totally sure! (eliminating files on disk that I never have to look at anyway isn't one of em for me) Ah, software.

pat commented 5 years ago

The fewer files thing is less about the number of files generally, but far more about how well they go across Rails versions. And if they need changing to make them cross-version compatible, then those are changes I'd need to make across every engine's test suite. Better to have that logic wrapped up in Combustion and thus the changes only need to be made once, and then the gem update can be shared across all engines.

All that said: the more files there are, the extra maintenance load. The best code is no code, after all! :)

And great to know the generators work! As for the tasks, yeah, that sounds more complicated… can't win 'em all 🤷‍♂️

jrochkind commented 5 years ago

If I can get the rake tasks working too, would you be interested in a PR to have combustion generators generate the right stuff to have it all working? As an option?

jrochkind commented 5 years ago

i guess generate or have internally without generation, which would be better, but might not be able to get it to work like that.

pat commented 5 years ago

I'd be open to a PR for the rake tasks, sure. It feels like only a subset are useful in this case, though? db:create/db:drop are helpful - but db:migrate less so, given that's handled automatically as part of Combustion's initialisation.

As for generators… do they work already? Or changes are needed?

jrochkind commented 5 years ago

Generators don't work already, I had to hack them in, I just succesfully did so. :)

I am choosing to (try to) use combustion with "out of the box" style migrations too, since my engine provides migrations to client apps (which need to be provided as migrations), and needs to test with those migrations too, and the out-of-the-box way of this being all wired up works out nicely.

It may be that my use cases just aren't suited for combustion, I'll continue investigating!

pat commented 5 years ago

Just going to close this issue - the discussion's great, I just treat open issues as things I need to do. Do feel free to add more thoughts in if you want (whether that be @jrochkind or others who come across this).

alexanderfrankel commented 4 years ago

@jrochkind @pat Interesting discussion above. Thanks for posting.

Has there been a recommended solution to getting rails generators working with an engine directory? I frequently use rails g migration <migration_name> inside the engine dir.

@jrochkind You mentioned that you had a workaround. Can you please post?

jrochkind commented 4 years ago

Hmm I've largely stopped using combustion for new Engine projects -- I think the friction points in Rails it addresses have become much less friction-full lately. (I DO use Appraisal still for testing under multiple Rails versions). But let's see if I can find a project the project I was talking about there, and see if I can find what I did....

Hm, I think this might be the project (it is not actually an Engine, but wants to use a Rails app in testing -- I think I might still use combustion in that situation!), and it still uses combustion... but I can't find/remember what I did in it for generators, or if that code is still there. Sorry!

alexanderfrankel commented 4 years ago

@jrochkind Ok thanks for looking!

Can you elaborate on the "friction-points" that have been solved in more recent versions of Rails? Are you specifically referring to Rails 6 as the version where most of these are solved?

jrochkind commented 4 years ago

I've been using Rails since 2.0.

Previously, what a Rails app looked like on disk could change a lot between versions, so it was really hard to test with multiple Rails versions. But lately (I'm not sure exactly when but I think at least 5.0), that isn't really true, it works out pretty well for me to test with different Rails versions using the dummy app on disk created by the "engine generator" without having to change it on disk, just changing the Rails version in Gemfile (using Appraisal). (OR to deal with upgrading Rails versions even if you are only testing with one -- I find the previously generated dummy app generally keeps working fine).

Also, the generator that generates a "dummy" app in an "Engine" for rails does a lot better job of making a dummy app that works -- so I find less need for an alternative. And there are features that come with a template-generated "Engine" (like the rails generators), that end up breaking without the full 'dummy' app. Those features didn't used to exist, so weren't there to break. I am not sure exactly what Rails version that was either.

Other people may have other reasons for finding combustion useful though! My own reasons, the motivation has lessened with more recent Rails versions, I can't say exactly which ones. Also the appraisal gem works well with the dummy app generated by Rails engine generator.

(Although if I want to run tests against a Rails app in a gem that isn't actually an "Engine"... I think I'd still reach for combustion!)